Re: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker <brobecker at adacore dot com>]

>      > btw, do you have a copyright assignment on file?
>      > This change feels small enough to me to not need one,
>      > but it's not clear. 
> 
>      I am a Chinese guy,and Chinese have not clue about the copyright.
>      (A joke.I don't need copyright.)

It's actually not for your personal benefit, but rather to help the FSF
enforce the GPL license on the code you are contributing, thus helping
it defend the freedom of our collective code. See:
http://www.gnu.org/licenses/why-assign.html

> >>    Please Joel Brobecker <brobecker@adacore.com> helps with the testcases.

Attached is a testcase that causes the debugger to crash on
x86_64-linux. It should work on all ELF targets.

A plea to the dwarf2read.c gurus:

Would it be possible to take a look at this patch, to see if it is
going in the right direction? Otherwise, I'll take a deeper look,
and see if I can solve it better. Intuitively, I think it may work,
but almost as a side-effect. Could the recursion check introduced
here do more than what we'd want to, for instance?

Thanks!

> >>>     The c++ code causing the problem is:
> >>> 
> >>>         // Integer variants of certain metrics, used for HTML rendering.
> >>>         int ascent(FontBaseline baselineType = AlphabeticBaseline) const
> >>>         {
> >>>             if (baselineType == AlphabeticBaseline)
> >>>                 return lroundf(m_ascent);
> >>>             return height() - height() / 2;
> >>>         }
> >>> 
> >>>         int height(FontBaseline baselineType = AlphabeticBaseline) const
> >>>         {
> >>>             return ascent(baselineType) + descent(baselineType);
> >>>         }
> >>> 
> >>>     As you can see,ascent(0x5816d55) calls height(0x5812c1b),and height calls
> >>>     ascent(0x5816d55) recursivly.And the compiler  generates these dwarf code
> >>>     representing this relationship preciously.
> >>> 
> >>>     A dwarf die may have the following relationship:
> >>>     564860c<-----------------------------
> >>>       |                                 |
> >>>       |(abstract origin)                |
> >>>       |                                 |
> >>>       V                                 |
> >>>     5816d55                             | (abstract origin)
> >>>       |                                 |
> >>>       |(child)                          |
> >>>       |                                 |
> >>>       V                                 |
> >>>       ...                               |
> >>>     5812c34------------------------------
> >>>     So inherit_abstract_dies may results in infinite recursive call.
> >>>     A bit field call in_process has been add to struct die_info to fix this problem.
> >>>     process_die would first check if a die is in processing state, if so,just return.
> >>>     Then in_process bit is set.Before process_die returns,this bit field is unset.---
>  ChangeLog        |  4 ++++
>  gdb/dwarf2read.c | 20 ++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 9b1cbfa..0098a72 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2013-01-20  lin zuojian  <manjian2006@gmail.com>
> +	* dwarf2read.c (struct die_info): New member in_process.
> +	(reset_die_in_process): New function.
> +	(process_die): Set it at the start, reset when returning.
>  2013-12-19  Keven Boell  <keven.boell@intel.com>
>  
>  	* cp-namespace.c (cp_lookup_nested_symbol): Enable
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 7ca527d..ffedde5 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1225,6 +1225,9 @@ struct die_info
>         type derived from this DIE.  */
>      unsigned char building_fullname : 1;
>  
> +    /* True if this die is in process.  */
> +    unsigned char in_process : 1;
> +
>      /* Abbrev number */
>      unsigned int abbrev;
>  
> @@ -8008,11 +8011,27 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu)
>      }
>  }
>  
> +/* Reset the in_process bit of a die.  */
> +
> +static void
> +reset_die_in_process (void *arg)
> +{
> +  struct die_info *die = arg;
> +  die->in_process = 0;
> +}
> +
>  /* Process a die and its children.  */
>  
>  static void
>  process_die (struct die_info *die, struct dwarf2_cu *cu)
>  {
> +  struct cleanup *in_process;
> +
> +  /* Only process those not already in process.  */
> +  if (die->in_process)
> +    return;
> +  die->in_process = 1;
> +  in_process = make_cleanup (reset_die_in_process,die);
>    switch (die->tag)
>      {
>      case DW_TAG_padding:
> @@ -8100,6 +8119,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
>        new_symbol (die, NULL, cu);
>        break;
>      }
> +    do_cleanups (in_process);
>  }
>  
>  /* DWARF name computation.  */
> -- 
> 1.8.3.2

-- 
Joel