Re: [PATCH] fixed inherit_abstract_dies infinite recursive call

[Doug Evans <xdje42 at gmail dot com>]

On Sun, Jan 19, 2014 at 10:29 PM,  <manjian2006@gmail.com> wrote:
> From: linzj <linzj@ucweb.com>
>
> 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.
> ---
>  gdb/dwarf2read.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 7ca527d..c226a52 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1224,6 +1224,8 @@ struct die_info
>      /* True if we're presently building the full type name for the
>         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;
> @@ -8013,6 +8015,10 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu)
>  static void
>  process_die (struct die_info *die, struct dwarf2_cu *cu)
>  {
> +  /* Only process those who are not in process.  */
> +  if (die->in_process)
> +    return;
> +  die->in_process = 1;
>    switch (die->tag)
>      {
>      case DW_TAG_padding:
> @@ -8100,6 +8106,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
>        new_symbol (die, NULL, cu);
>        break;
>      }
> +    die->in_process = 0;
>  }
>
>  /* DWARF name computation.  */

Hi.
Setting aside whether this is The Right solution (I haven't absorbed
enough to say for sure, and I don't mind this patch if it's a step in
the right direction) ...

While gdb shouldn't throw an error while reading dwarf info, there are
times when the situation is rare enough and sufficiently hard enough
to avoid that we throw an error anyway.

If an error happens and the bit is left set, is the resulting behaviour ok?
Possibly, but it feels cleaner, and keeps things simpler to reason
about, if we use a cleanup here to reset the bit.
[E.g., it's not clear to me that gdb will necessarily always avoid
rereading the debug info, now and in the future, if an error happens
when reading it the first time]