[PATCH v4] fixed inherit_abstract_dies infinite recursive call

[PATCH v4] fixed inherit_abstract_dies infinite recursive call

[manjian2006]

From: linzj <linzj@ucweb.com>

      * ChangeLog
      Modified ChangeLog as Doug Evans suggested.
      * dwarf2read.c
      Modified ChangeLog as Doug Evans suggested.

     > 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.)
      
>    Reset the die's in_process bit using cleanup facility.
>>    ChangeLog added.
>>    Please Joel Brobecker <brobecker@adacore.com> helps with the testcases.
>>>     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);
 }
 \f
 /* DWARF name computation.  */
-- 
1.8.3.2

Re: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

>      > 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);
>  }
>  \f
>  /* DWARF name computation.  */
> -- 
> 1.8.3.2

-- 
Joel

PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

Ping!

It would be interesting to have a formal review of this patch,
to know if it is an acceptable fix or not.  If not, I can schedule
some time to follow any recommendation that might come out of
this review.

Thank you!

On Tue, Jan 28, 2014 at 04:06:00PM +0400, Joel Brobecker wrote:
> >      > 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);
> >  }
> >  \f
> >  /* DWARF name computation.  */
> > -- 
> > 1.8.3.2
> 
> -- 
> Joel

-- 
Joel

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Doug Evans]

On Mon, Feb 10, 2014 at 6:28 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> Ping!
>
> It would be interesting to have a formal review of this patch,
> to know if it is an acceptable fix or not.  If not, I can schedule
> some time to follow any recommendation that might come out of
> this review.
>
> Thank you!
>
> On Tue, Jan 28, 2014 at 04:06:00PM +0400, Joel Brobecker wrote:
>> >      > 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
>>

Hi.

How hard is it to write a testcase that triggers the problem?

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

> How hard is it to write a testcase that triggers the problem?

I wrote one and I thought I posted it here. But I had forgotten
to attach the patch! (git reflog just saved my life).

Here it is again.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-icycle.S, gdb.dwarf2/dw2-icycle.c,
        gdb.dwarf2/dw2-icycle.exp: New files.

This testcase fails without the patch being proposed.

Thanks,
-- 
Joel

[-- Attachment #2: 0001-New-asm-based-testcase-testing-DWARF-inlining-cycles.patch --]
[-- Type: text/x-diff, Size: 13159 bytes --]

From 9aa3343b7db4fceca64f587c763d4010146c7980 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 28 Jan 2014 15:52:55 +0400
Subject: [PATCH] New asm-based testcase testing DWARF inlining cycles.

This testcase uses a dramatically reduced version of an assembly file
that was generated by GCC 4.9 when compiling at -O3, where nested
functions and inlining cause some cycles.  If the DWARF data is not
processed carefully, the debugger can go into infinite recursion,
eventually leading to a crash when the stack gets exhausted.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-icycle.S, gdb.dwarf2/dw2-icycle.c,
        gdb.dwarf2/dw2-icycle.exp: New files.
---
 gdb/testsuite/gdb.dwarf2/dw2-icycle.S   | 258 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-icycle.c   |  24 +++
 gdb/testsuite/gdb.dwarf2/dw2-icycle.exp |  43 ++++++
 3 files changed, 325 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-icycle.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-icycle.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-icycle.exp

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.S b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
new file mode 100644
index 0000000..6bc533a
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
@@ -0,0 +1,258 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.text
+
+.Ltext0:
+	.type	p__top__middle__inside.3062, @function
+p__top__middle__inside.3062:
+.LFB4:
+	.file 1 "p.adb"
+        .4byte 0
+.LBE6:
+
+	.globl	p__top
+	.type	p__top, @function
+p__top:
+.LFB2:
+        .4byte 0
+.LFE2:
+.Letext0:
+
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.4byte	.Ledebug_info0 - .Lsdebug_info0  /* Length of CU Info */
+.Lsdebug_info0:
+	.2byte	0x4	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+	.ascii	"GNU Ada 4.9.0 20140126\0" /* DW_AT_producer */
+	.byte	0xd	/* DW_AT_language */
+	.ascii	"p.adb\0" /* DW_AT_name */
+	.ascii	"/tmp\0"  /* DW_AT_comp_dir */
+	.4byte	.Ltext0	/* DW_AT_low_pc */
+	.4byte	.Letext0-.Ltext0	/* DW_AT_high_pc */
+.S0x142:
+	.uleb128 0x8	/* (DIE (0x142) DW_TAG_base_type) */
+	.byte	0x4	/* DW_AT_byte_size */
+	.byte	0x5	/* DW_AT_encoding */
+	.ascii	"integer\0" /* DW_AT_name */
+
+	.uleb128 0x13	/* (DIE (0x1b4) DW_TAG_subprogram) */
+			/* DW_AT_external */
+	.ascii	"p__top\0" /* DW_AT_name */
+	.byte	0x1	/* DW_AT_decl_file (p.adb) */
+	.byte	0x3	/* DW_AT_decl_line */
+	.4byte	.LFB2	/* DW_AT_low_pc */
+	.4byte	.LFE2-.LFB2	/* DW_AT_high_pc */
+	.uleb128 0x1	/* DW_AT_frame_base */
+	.byte	0x9c	/* DW_OP_call_frame_cfa */
+			/* DW_AT_GNU_all_call_sites */
+	.4byte	.S0x4fc - .Ldebug_info0	/* DW_AT_sibling */
+.S0x1e0:
+	.uleb128 0x15	/* (DIE (0x1e0) DW_TAG_subprogram) */
+	.ascii	"p__top__middle\0" /* DW_AT_name */
+	.byte	0x1	/* DW_AT_decl_file (p.adb) */
+	.byte	0x4	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_inline */
+	.4byte	.S0x374 - .Ldebug_info0	/* DW_AT_sibling */
+.S0x202:
+	.uleb128 0x15	/* (DIE (0x202) DW_TAG_subprogram) */
+	.ascii	"p__top__middle__inside\0" /* DW_AT_name */
+	.byte	0x1	/* DW_AT_decl_file (p.adb) */
+	.byte	0x5	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_inline */
+	.4byte	.S0x225	- .Ldebug_info0 /* DW_AT_sibling */
+	.byte	0	/* end of children of DIE 0x202 */
+.S0x225:
+	.uleb128 0x18	/* (DIE (0x225) DW_TAG_subprogram) */
+	.4byte	.S0x202 - .Ldebug_info0	/* DW_AT_abstract_origin */
+	.4byte	.LFB4	/* DW_AT_low_pc */
+	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
+	.uleb128 0x1	/* DW_AT_frame_base */
+	.byte	0x9c	/* DW_OP_call_frame_cfa */
+	.uleb128 0x1	/* DW_AT_static_link */
+	.byte	0x56	/* DW_OP_reg6 */
+			/* DW_AT_GNU_all_call_sites */
+	.uleb128 0x1a	/* (DIE (0x247) DW_TAG_inlined_subroutine) */
+	.4byte	.S0x1e0 - .Ldebug_info0	/* DW_AT_abstract_origin */
+	.4byte	.LFB4	/* DW_AT_low_pc */
+	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
+	.byte	0x1	/* DW_AT_call_file (p.adb) */
+	.byte	0x14	/* DW_AT_call_line */
+	.4byte	.S0x374	- .Ldebug_info0 /* DW_AT_sibling */
+	.byte	0	/* end of children of DIE 0x247 */
+	.byte	0	/* end of children of DIE 0x225 */
+	.byte	0	/* end of children of DIE 0x1e0 */
+.S0x374:
+	.uleb128 0x23	/* (DIE (0x382) DW_TAG_inlined_subroutine) */
+	.4byte	.S0x1e0 - .Ldebug_info0 /* DW_AT_abstract_origin */
+	.4byte	.LFB4	/* DW_AT_low_pc */
+	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
+	.byte	0x1	/* DW_AT_call_file (p.adb) */
+	.byte	0x1d	/* DW_AT_call_line */
+	.byte	0	/* end of children of DIE 0x382 */
+	.byte	0	/* end of children of DIE 0x1b4 */
+.S0x4fc:
+	.uleb128 0x28	/* (DIE (0x52e) DW_TAG_subprogram) */
+			/* DW_AT_external */
+	.ascii	"__gnat_rcheck_PE_Explicit_Raise\0" /* DW_AT_name */
+			/* DW_AT_artificial */
+			/* DW_AT_declaration */
+	.byte	0	/* end of children of DIE 0x52e */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x8	/* (abbrev code) */
+	.uleb128 0x24	/* (TAG: DW_TAG_base_type) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3e	/* (DW_AT_encoding) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0
+	.byte	0
+	.uleb128 0x13	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x40	/* (DW_AT_frame_base) */
+	.uleb128 0x18	/* (DW_FORM_exprloc) */
+	.uleb128 0x2117	/* (DW_AT_GNU_all_call_sites) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x1	/* (DW_AT_sibling) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x15	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x20	/* (DW_AT_inline) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x1	/* (DW_AT_sibling) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x18	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x31	/* (DW_AT_abstract_origin) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x40	/* (DW_AT_frame_base) */
+	.uleb128 0x18	/* (DW_FORM_exprloc) */
+	.uleb128 0x48	/* (DW_AT_static_link) */
+	.uleb128 0x18	/* (DW_FORM_exprloc) */
+	.uleb128 0x2117	/* (DW_AT_GNU_all_call_sites) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.byte	0
+	.byte	0
+	.uleb128 0x1a	/* (abbrev code) */
+	.uleb128 0x1d	/* (TAG: DW_TAG_inlined_subroutine) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x31	/* (DW_AT_abstract_origin) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x58	/* (DW_AT_call_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x59	/* (DW_AT_call_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x1	/* (DW_AT_sibling) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x23	/* (abbrev code) */
+	.uleb128 0x1d	/* (TAG: DW_TAG_inlined_subroutine) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x31	/* (DW_AT_abstract_origin) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x58	/* (DW_AT_call_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x59	/* (DW_AT_call_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.byte	0
+	.byte	0
+	.uleb128 0x28	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x34	/* (DW_AT_artificial) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x3c	/* (DW_AT_declaration) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.byte	0
+	.byte	0
+	.byte	0
+	.byte	0
+	.byte	0
+
+        .section .debug_line
+.Lline1_begin:
+        .byte   0
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.c b/gdb/testsuite/gdb.dwarf2/dw2-icycle.c
new file mode 100644
index 0000000..3ddd194
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Dummy main function.  */
+
+int
+main()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp b/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp
new file mode 100644
index 0000000..fcb57b6
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp
@@ -0,0 +1,43 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .S .c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $srcfile2] {nodebug}] } {
+    return -1
+}
+
+# We are trying to verify that the partial symtab to symtab expansion
+# for the debugging info hand-coded in our assembly file does not cause
+# the debugger to crash (infinite recursion).  To facilitate the test,
+# start the debugger with -readnow.  This force expansion as soon as
+# the objfile is loaded.
+
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS "$GDBFLAGS -readnow"
+clean_restart ${testfile}
+set GDBFLAGS $saved_gdbflags
+
+# And just to be sure that the debugger did not crash after having
+# expanded our symbols, do a life-check.
+
+gdb_test "echo life check\\n" "life check"
-- 
1.8.3.2

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[manjian2006]

I don't see anything I can review.
Did you forget to attach the patch?

> Ping!
> 
> It would be interesting to have a formal review of this patch,
> to know if it is an acceptable fix or not.  If not, I can schedule
> some time to follow any recommendation that might come out of
> this review.
> 
> Thank you!
> 
> On Tue, Jan 28, 2014 at 04:06:00PM +0400, Joel Brobecker wrote:
> > >      > 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);
> > >  }
> > >  \f
> > >  /* DWARF name computation.  */
> > > -- 
> > > 1.8.3.2
> > 
> > -- 
> > Joel
> 
> -- 
> Joel
> 

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Doug Evans]

On Mon, Feb 10, 2014 at 6:19 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> How hard is it to write a testcase that triggers the problem?
>
> I wrote one and I thought I posted it here. But I had forgotten
> to attach the patch! (git reflog just saved my life).
>
> Here it is again.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.dwarf2/dw2-icycle.S, gdb.dwarf2/dw2-icycle.c,
>         gdb.dwarf2/dw2-icycle.exp: New files.
>
> This testcase fails without the patch being proposed.
>
> Thanks,
> --
> Joel

Hi.  Thanks very much for the testcase!

I don't have much experience with abstract_origin.  I've read what I
can from DWARF4.pdf.
I can imagine that the bug is really in inherit_abstract_dies, and
thus the check to avoid re-processing the die belongs there.
Then we could maybe assert-fail in process_die if it's already being processed.

E.g., where inherit_abstract_dies has this:

            /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
            process_die (origin_child_die, origin_cu);

add a check for origin_child_die->in_process here, and only call
process_die if it's zero,
and add a comment saying the check is to avoid the case of mutually
referenced abstract_origins.
And include a reference to a bug number because for me one high order
bit here is finding the testcase that exercises this check.

And then in process_die add at the start:

  gdb_assert (!die->in_process);

I don't have a strong opinion on which way to go though.
I *do* have a strong opinion on understanding *why* the code is
checking die->in_process.
If we go with the current patch, which is fine with me, though I'm
slightly leading towards the above instead but am happy to defer to
others, then I would replace this part of your patch:

+  /* Only process those not already in process.  */
+  if (die->in_process)
+    return;

with:

+  /* Only process those not already in process.  PR 12345.  */
+  if (die->in_process)
+    return;

And in either case file a bug that includes a description of the
problem (obviously :-)) and a reference to the testcase name.

Thoughts?

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

Hi Doug,

> Hi.  Thanks very much for the testcase!

You are very welcome. Surprisingly, it took me a 2-3 hours to reduce
it down. I tried using the DWARF assembler, but to no avail, so had
to work from an asm file instead. We might be able to reproduce
that asm file using the DWARF assembler, but I don't want to spend
any more time on the testcase, unless I really have to.

> I don't have much experience with abstract_origin.  I've read what I
> can from DWARF4.pdf.
> I can imagine that the bug is really in inherit_abstract_dies, and
> thus the check to avoid re-processing the die belongs there.
> Then we could maybe assert-fail in process_die if it's already being
> processed.

Same here.

> E.g., where inherit_abstract_dies has this:
> 
>             /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
>             process_die (origin_child_die, origin_cu);
> 
> add a check for origin_child_die->in_process here, and only call
> process_die if it's zero,
> and add a comment saying the check is to avoid the case of mutually
> referenced abstract_origins.
> And include a reference to a bug number because for me one high order
> bit here is finding the testcase that exercises this check.
> 
> And then in process_die add at the start:
> 
>   gdb_assert (!die->in_process);
> 
> I don't have a strong opinion on which way to go though.
> I *do* have a strong opinion on understanding *why* the code is
> checking die->in_process.
> If we go with the current patch, which is fine with me, though I'm
> slightly leading towards the above instead but am happy to defer to
> others, then I would replace this part of your patch:
> 
> +  /* Only process those not already in process.  */
> +  if (die->in_process)
> +    return;
> 
> with:
> 
> +  /* Only process those not already in process.  PR 12345.  */
> +  if (die->in_process)
> +    return;
> 
> And in either case file a bug that includes a description of the
> problem (obviously :-)) and a reference to the testcase name.
> 
> Thoughts?

I agree with your all you suggestions.

manjian2006@gmail.com, do you want to take care of this, or would you
like me to? This involves first creating a PR, change your patch
according to Doug's suggestion, re-test, and re-submit.

-- 
Joel

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[lin zuojian]

Hi Joel,
I am not really familiar with these routines,please help me with that.
Thank you.
(I carelessly used the company email in the last reply,sorry).

> Hi Doug,
>
>> Hi.  Thanks very much for the testcase!
> You are very welcome. Surprisingly, it took me a 2-3 hours to reduce
> it down. I tried using the DWARF assembler, but to no avail, so had
> to work from an asm file instead. We might be able to reproduce
> that asm file using the DWARF assembler, but I don't want to spend
> any more time on the testcase, unless I really have to.
>
>> I don't have much experience with abstract_origin.  I've read what I
>> can from DWARF4.pdf.
>> I can imagine that the bug is really in inherit_abstract_dies, and
>> thus the check to avoid re-processing the die belongs there.
>> Then we could maybe assert-fail in process_die if it's already being
>> processed.
> Same here.
>
>> E.g., where inherit_abstract_dies has this:
>>
>>             /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
>>             process_die (origin_child_die, origin_cu);
>>
>> add a check for origin_child_die->in_process here, and only call
>> process_die if it's zero,
>> and add a comment saying the check is to avoid the case of mutually
>> referenced abstract_origins.
>> And include a reference to a bug number because for me one high order
>> bit here is finding the testcase that exercises this check.
>>
>> And then in process_die add at the start:
>>
>>   gdb_assert (!die->in_process);
>>
>> I don't have a strong opinion on which way to go though.
>> I *do* have a strong opinion on understanding *why* the code is
>> checking die->in_process.
>> If we go with the current patch, which is fine with me, though I'm
>> slightly leading towards the above instead but am happy to defer to
>> others, then I would replace this part of your patch:
>>
>> +  /* Only process those not already in process.  */
>> +  if (die->in_process)
>> +    return;
>>
>> with:
>>
>> +  /* Only process those not already in process.  PR 12345.  */
>> +  if (die->in_process)
>> +    return;
>>
>> And in either case file a bug that includes a description of the
>> problem (obviously :-)) and a reference to the testcase name.
>>
>> Thoughts?
> I agree with your all you suggestions.
>
> manjian2006@gmail.com, do you want to take care of this, or would you
> like me to? This involves first creating a PR, change your patch
> according to Doug's suggestion, re-test, and re-submit.
>

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

> I am not really familiar with these routines,please help me with that.
> Thank you.

OK - Doug and I will take care of it.

For the record, I have created the PR.
https://sourceware.org/bugzilla/show_bug.cgi?id=16581

-- 
Joel

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Doug Evans]

Joel Brobecker <brobecker@adacore.com> writes:
>> I am not really familiar with these routines,please help me with that.
>> Thank you.
>
> OK - Doug and I will take care of it.
>
> For the record, I have created the PR.
> https://sourceware.org/bugzilla/show_bug.cgi?id=16581

Hi.
Sorry for the delay!
I spent some time looking for a cleverer patch, but in the end I think the
simplicity of this patch is nice.

2014-02-18  lin zuojian  <manjian2006@gmail.com>
	    Joel Brobecker  <brobecker@adacore.com>
	    Doug Evans  <xdje42@gmail.com>

	PR symtab/16581
	* 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.
	(inherit_abstract_dies): Only call process_die if origin_child_die
	not already being processed.

	testsuite/
	* gdb.dwarf2/dw2-icycle.S: New file.
	* gdb.dwarf2/dw2-icycle.c: New file.
	* gdb.dwarf2/dw2-icycle.exp: New file.
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 54c538a..d7f893d 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.  PR 16581.  */
+    unsigned char in_process : 1;
+
     /* Abbrev number */
     unsigned int abbrev;
 
@@ -8008,11 +8011,28 @@ 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;
+
+  /* We should only be processing those not already in process.  */
+  gdb_assert (!die->in_process);
+
+  die->in_process = 1;
+  in_process = make_cleanup (reset_die_in_process,die);
+
   switch (die->tag)
     {
     case DW_TAG_padding:
@@ -8100,6 +8120,8 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
       new_symbol (die, NULL, cu);
       break;
     }
+
+  do_cleanups (in_process);
 }
 \f
 /* DWARF name computation.  */
@@ -10967,8 +10989,12 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
       if (offsetp >= offsets_end
 	  || offsetp->sect_off > origin_child_die->offset.sect_off)
 	{
-	  /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
-	  process_die (origin_child_die, origin_cu);
+	  /* Found that ORIGIN_CHILD_DIE is really not referenced.
+	     Check whether we're already processing ORIGIN_CHILD_DIE.
+	     This can happen with mutually referenced abstract_origins.
+	     PR 16581.  */
+	  if (!origin_child_die->in_process)
+	    process_die (origin_child_die, origin_cu);
 	}
       origin_child_die = sibling_die (origin_child_die);
     }
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.S b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
new file mode 100644
index 0000000..6bc533a
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
@@ -0,0 +1,258 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.text
+
+.Ltext0:
+	.type	p__top__middle__inside.3062, @function
+p__top__middle__inside.3062:
+.LFB4:
+	.file 1 "p.adb"
+        .4byte 0
+.LBE6:
+
+	.globl	p__top
+	.type	p__top, @function
+p__top:
+.LFB2:
+        .4byte 0
+.LFE2:
+.Letext0:
+
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.4byte	.Ledebug_info0 - .Lsdebug_info0  /* Length of CU Info */
+.Lsdebug_info0:
+	.2byte	0x4	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+	.ascii	"GNU Ada 4.9.0 20140126\0" /* DW_AT_producer */
+	.byte	0xd	/* DW_AT_language */
+	.ascii	"p.adb\0" /* DW_AT_name */
+	.ascii	"/tmp\0"  /* DW_AT_comp_dir */
+	.4byte	.Ltext0	/* DW_AT_low_pc */
+	.4byte	.Letext0-.Ltext0	/* DW_AT_high_pc */
+.S0x142:
+	.uleb128 0x8	/* (DIE (0x142) DW_TAG_base_type) */
+	.byte	0x4	/* DW_AT_byte_size */
+	.byte	0x5	/* DW_AT_encoding */
+	.ascii	"integer\0" /* DW_AT_name */
+
+	.uleb128 0x13	/* (DIE (0x1b4) DW_TAG_subprogram) */
+			/* DW_AT_external */
+	.ascii	"p__top\0" /* DW_AT_name */
+	.byte	0x1	/* DW_AT_decl_file (p.adb) */
+	.byte	0x3	/* DW_AT_decl_line */
+	.4byte	.LFB2	/* DW_AT_low_pc */
+	.4byte	.LFE2-.LFB2	/* DW_AT_high_pc */
+	.uleb128 0x1	/* DW_AT_frame_base */
+	.byte	0x9c	/* DW_OP_call_frame_cfa */
+			/* DW_AT_GNU_all_call_sites */
+	.4byte	.S0x4fc - .Ldebug_info0	/* DW_AT_sibling */
+.S0x1e0:
+	.uleb128 0x15	/* (DIE (0x1e0) DW_TAG_subprogram) */
+	.ascii	"p__top__middle\0" /* DW_AT_name */
+	.byte	0x1	/* DW_AT_decl_file (p.adb) */
+	.byte	0x4	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_inline */
+	.4byte	.S0x374 - .Ldebug_info0	/* DW_AT_sibling */
+.S0x202:
+	.uleb128 0x15	/* (DIE (0x202) DW_TAG_subprogram) */
+	.ascii	"p__top__middle__inside\0" /* DW_AT_name */
+	.byte	0x1	/* DW_AT_decl_file (p.adb) */
+	.byte	0x5	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_inline */
+	.4byte	.S0x225	- .Ldebug_info0 /* DW_AT_sibling */
+	.byte	0	/* end of children of DIE 0x202 */
+.S0x225:
+	.uleb128 0x18	/* (DIE (0x225) DW_TAG_subprogram) */
+	.4byte	.S0x202 - .Ldebug_info0	/* DW_AT_abstract_origin */
+	.4byte	.LFB4	/* DW_AT_low_pc */
+	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
+	.uleb128 0x1	/* DW_AT_frame_base */
+	.byte	0x9c	/* DW_OP_call_frame_cfa */
+	.uleb128 0x1	/* DW_AT_static_link */
+	.byte	0x56	/* DW_OP_reg6 */
+			/* DW_AT_GNU_all_call_sites */
+	.uleb128 0x1a	/* (DIE (0x247) DW_TAG_inlined_subroutine) */
+	.4byte	.S0x1e0 - .Ldebug_info0	/* DW_AT_abstract_origin */
+	.4byte	.LFB4	/* DW_AT_low_pc */
+	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
+	.byte	0x1	/* DW_AT_call_file (p.adb) */
+	.byte	0x14	/* DW_AT_call_line */
+	.4byte	.S0x374	- .Ldebug_info0 /* DW_AT_sibling */
+	.byte	0	/* end of children of DIE 0x247 */
+	.byte	0	/* end of children of DIE 0x225 */
+	.byte	0	/* end of children of DIE 0x1e0 */
+.S0x374:
+	.uleb128 0x23	/* (DIE (0x382) DW_TAG_inlined_subroutine) */
+	.4byte	.S0x1e0 - .Ldebug_info0 /* DW_AT_abstract_origin */
+	.4byte	.LFB4	/* DW_AT_low_pc */
+	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
+	.byte	0x1	/* DW_AT_call_file (p.adb) */
+	.byte	0x1d	/* DW_AT_call_line */
+	.byte	0	/* end of children of DIE 0x382 */
+	.byte	0	/* end of children of DIE 0x1b4 */
+.S0x4fc:
+	.uleb128 0x28	/* (DIE (0x52e) DW_TAG_subprogram) */
+			/* DW_AT_external */
+	.ascii	"__gnat_rcheck_PE_Explicit_Raise\0" /* DW_AT_name */
+			/* DW_AT_artificial */
+			/* DW_AT_declaration */
+	.byte	0	/* end of children of DIE 0x52e */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x8	/* (abbrev code) */
+	.uleb128 0x24	/* (TAG: DW_TAG_base_type) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3e	/* (DW_AT_encoding) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0
+	.byte	0
+	.uleb128 0x13	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x40	/* (DW_AT_frame_base) */
+	.uleb128 0x18	/* (DW_FORM_exprloc) */
+	.uleb128 0x2117	/* (DW_AT_GNU_all_call_sites) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x1	/* (DW_AT_sibling) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x15	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x20	/* (DW_AT_inline) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x1	/* (DW_AT_sibling) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x18	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x31	/* (DW_AT_abstract_origin) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x40	/* (DW_AT_frame_base) */
+	.uleb128 0x18	/* (DW_FORM_exprloc) */
+	.uleb128 0x48	/* (DW_AT_static_link) */
+	.uleb128 0x18	/* (DW_FORM_exprloc) */
+	.uleb128 0x2117	/* (DW_AT_GNU_all_call_sites) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.byte	0
+	.byte	0
+	.uleb128 0x1a	/* (abbrev code) */
+	.uleb128 0x1d	/* (TAG: DW_TAG_inlined_subroutine) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x31	/* (DW_AT_abstract_origin) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x58	/* (DW_AT_call_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x59	/* (DW_AT_call_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x1	/* (DW_AT_sibling) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x23	/* (abbrev code) */
+	.uleb128 0x1d	/* (TAG: DW_TAG_inlined_subroutine) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x31	/* (DW_AT_abstract_origin) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x58	/* (DW_AT_call_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x59	/* (DW_AT_call_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.byte	0
+	.byte	0
+	.uleb128 0x28	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x34	/* (DW_AT_artificial) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.uleb128 0x3c	/* (DW_AT_declaration) */
+	.uleb128 0x19	/* (DW_FORM_flag_present) */
+	.byte	0
+	.byte	0
+	.byte	0
+	.byte	0
+	.byte	0
+
+        .section .debug_line
+.Lline1_begin:
+        .byte   0
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.c b/gdb/testsuite/gdb.dwarf2/dw2-icycle.c
new file mode 100644
index 0000000..3ddd194
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Dummy main function.  */
+
+int
+main()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp b/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp
new file mode 100644
index 0000000..fcb57b6
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp
@@ -0,0 +1,43 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .S .c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $srcfile2] {nodebug}] } {
+    return -1
+}
+
+# We are trying to verify that the partial symtab to symtab expansion
+# for the debugging info hand-coded in our assembly file does not cause
+# the debugger to crash (infinite recursion).  To facilitate the test,
+# start the debugger with -readnow.  This force expansion as soon as
+# the objfile is loaded.
+
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS "$GDBFLAGS -readnow"
+clean_restart ${testfile}
+set GDBFLAGS $saved_gdbflags
+
+# And just to be sure that the debugger did not crash after having
+# expanded our symbols, do a life-check.
+
+gdb_test "echo life check\\n" "life check"

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[lin zuojian]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 15890 bytes --]

Thank you very much Doug.Nice job.

ÓÚ 2014Äê02ÔÂ19ÈÕ 14:48, Doug Evans дµÀ:
> Joel Brobecker <brobecker@adacore.com> writes:
>>> I am not really familiar with these routines,please help me with that.
>>> Thank you.
>> OK - Doug and I will take care of it.
>>
>> For the record, I have created the PR.
>> https://sourceware.org/bugzilla/show_bug.cgi?id=16581
> Hi.
> Sorry for the delay!
> I spent some time looking for a cleverer patch, but in the end I think the
> simplicity of this patch is nice.
>
> 2014-02-18  lin zuojian  <manjian2006@gmail.com>
> 	    Joel Brobecker  <brobecker@adacore.com>
> 	    Doug Evans  <xdje42@gmail.com>
>
> 	PR symtab/16581
> 	* 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.
> 	(inherit_abstract_dies): Only call process_die if origin_child_die
> 	not already being processed.
>
> 	testsuite/
> 	* gdb.dwarf2/dw2-icycle.S: New file.
> 	* gdb.dwarf2/dw2-icycle.c: New file.
> 	* gdb.dwarf2/dw2-icycle.exp: New file.
>  
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 54c538a..d7f893d 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.  PR 16581.  */
> +    unsigned char in_process : 1;
> +
>      /* Abbrev number */
>      unsigned int abbrev;
>  
> @@ -8008,11 +8011,28 @@ 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;
> +
> +  /* We should only be processing those not already in process.  */
> +  gdb_assert (!die->in_process);
> +
> +  die->in_process = 1;
> +  in_process = make_cleanup (reset_die_in_process,die);
> +
>    switch (die->tag)
>      {
>      case DW_TAG_padding:
> @@ -8100,6 +8120,8 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
>        new_symbol (die, NULL, cu);
>        break;
>      }
> +
> +  do_cleanups (in_process);
>  }
>  \f
>  /* DWARF name computation.  */
> @@ -10967,8 +10989,12 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
>        if (offsetp >= offsets_end
>  	  || offsetp->sect_off > origin_child_die->offset.sect_off)
>  	{
> -	  /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
> -	  process_die (origin_child_die, origin_cu);
> +	  /* Found that ORIGIN_CHILD_DIE is really not referenced.
> +	     Check whether we're already processing ORIGIN_CHILD_DIE.
> +	     This can happen with mutually referenced abstract_origins.
> +	     PR 16581.  */
> +	  if (!origin_child_die->in_process)
> +	    process_die (origin_child_die, origin_cu);
>  	}
>        origin_child_die = sibling_die (origin_child_die);
>      }
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.S b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
> new file mode 100644
> index 0000000..6bc533a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
> @@ -0,0 +1,258 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2014 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +	.text
> +
> +.Ltext0:
> +	.type	p__top__middle__inside.3062, @function
> +p__top__middle__inside.3062:
> +.LFB4:
> +	.file 1 "p.adb"
> +        .4byte 0
> +.LBE6:
> +
> +	.globl	p__top
> +	.type	p__top, @function
> +p__top:
> +.LFB2:
> +        .4byte 0
> +.LFE2:
> +.Letext0:
> +
> +	.section	.debug_info,"",@progbits
> +.Ldebug_info0:
> +	.4byte	.Ledebug_info0 - .Lsdebug_info0  /* Length of CU Info */
> +.Lsdebug_info0:
> +	.2byte	0x4	/* DWARF version number */
> +	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
> +	.byte	0x4	/* Pointer Size (in bytes) */
> +	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
> +	.ascii	"GNU Ada 4.9.0 20140126\0" /* DW_AT_producer */
> +	.byte	0xd	/* DW_AT_language */
> +	.ascii	"p.adb\0" /* DW_AT_name */
> +	.ascii	"/tmp\0"  /* DW_AT_comp_dir */
> +	.4byte	.Ltext0	/* DW_AT_low_pc */
> +	.4byte	.Letext0-.Ltext0	/* DW_AT_high_pc */
> +.S0x142:
> +	.uleb128 0x8	/* (DIE (0x142) DW_TAG_base_type) */
> +	.byte	0x4	/* DW_AT_byte_size */
> +	.byte	0x5	/* DW_AT_encoding */
> +	.ascii	"integer\0" /* DW_AT_name */
> +
> +	.uleb128 0x13	/* (DIE (0x1b4) DW_TAG_subprogram) */
> +			/* DW_AT_external */
> +	.ascii	"p__top\0" /* DW_AT_name */
> +	.byte	0x1	/* DW_AT_decl_file (p.adb) */
> +	.byte	0x3	/* DW_AT_decl_line */
> +	.4byte	.LFB2	/* DW_AT_low_pc */
> +	.4byte	.LFE2-.LFB2	/* DW_AT_high_pc */
> +	.uleb128 0x1	/* DW_AT_frame_base */
> +	.byte	0x9c	/* DW_OP_call_frame_cfa */
> +			/* DW_AT_GNU_all_call_sites */
> +	.4byte	.S0x4fc - .Ldebug_info0	/* DW_AT_sibling */
> +.S0x1e0:
> +	.uleb128 0x15	/* (DIE (0x1e0) DW_TAG_subprogram) */
> +	.ascii	"p__top__middle\0" /* DW_AT_name */
> +	.byte	0x1	/* DW_AT_decl_file (p.adb) */
> +	.byte	0x4	/* DW_AT_decl_line */
> +	.byte	0x1	/* DW_AT_inline */
> +	.4byte	.S0x374 - .Ldebug_info0	/* DW_AT_sibling */
> +.S0x202:
> +	.uleb128 0x15	/* (DIE (0x202) DW_TAG_subprogram) */
> +	.ascii	"p__top__middle__inside\0" /* DW_AT_name */
> +	.byte	0x1	/* DW_AT_decl_file (p.adb) */
> +	.byte	0x5	/* DW_AT_decl_line */
> +	.byte	0x1	/* DW_AT_inline */
> +	.4byte	.S0x225	- .Ldebug_info0 /* DW_AT_sibling */
> +	.byte	0	/* end of children of DIE 0x202 */
> +.S0x225:
> +	.uleb128 0x18	/* (DIE (0x225) DW_TAG_subprogram) */
> +	.4byte	.S0x202 - .Ldebug_info0	/* DW_AT_abstract_origin */
> +	.4byte	.LFB4	/* DW_AT_low_pc */
> +	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
> +	.uleb128 0x1	/* DW_AT_frame_base */
> +	.byte	0x9c	/* DW_OP_call_frame_cfa */
> +	.uleb128 0x1	/* DW_AT_static_link */
> +	.byte	0x56	/* DW_OP_reg6 */
> +			/* DW_AT_GNU_all_call_sites */
> +	.uleb128 0x1a	/* (DIE (0x247) DW_TAG_inlined_subroutine) */
> +	.4byte	.S0x1e0 - .Ldebug_info0	/* DW_AT_abstract_origin */
> +	.4byte	.LFB4	/* DW_AT_low_pc */
> +	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
> +	.byte	0x1	/* DW_AT_call_file (p.adb) */
> +	.byte	0x14	/* DW_AT_call_line */
> +	.4byte	.S0x374	- .Ldebug_info0 /* DW_AT_sibling */
> +	.byte	0	/* end of children of DIE 0x247 */
> +	.byte	0	/* end of children of DIE 0x225 */
> +	.byte	0	/* end of children of DIE 0x1e0 */
> +.S0x374:
> +	.uleb128 0x23	/* (DIE (0x382) DW_TAG_inlined_subroutine) */
> +	.4byte	.S0x1e0 - .Ldebug_info0 /* DW_AT_abstract_origin */
> +	.4byte	.LFB4	/* DW_AT_low_pc */
> +	.4byte	.LBE6-.LFB4	/* DW_AT_high_pc */
> +	.byte	0x1	/* DW_AT_call_file (p.adb) */
> +	.byte	0x1d	/* DW_AT_call_line */
> +	.byte	0	/* end of children of DIE 0x382 */
> +	.byte	0	/* end of children of DIE 0x1b4 */
> +.S0x4fc:
> +	.uleb128 0x28	/* (DIE (0x52e) DW_TAG_subprogram) */
> +			/* DW_AT_external */
> +	.ascii	"__gnat_rcheck_PE_Explicit_Raise\0" /* DW_AT_name */
> +			/* DW_AT_artificial */
> +			/* DW_AT_declaration */
> +	.byte	0	/* end of children of DIE 0x52e */
> +	.byte	0	/* end of children of DIE 0xb */
> +.Ledebug_info0:
> +
> +	.section	.debug_abbrev,"",@progbits
> +.Ldebug_abbrev0:
> +	.uleb128 0x1	/* (abbrev code) */
> +	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x25	/* (DW_AT_producer) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.uleb128 0x13	/* (DW_AT_language) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x3	/* (DW_AT_name) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.uleb128 0x1b	/* (DW_AT_comp_dir) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.uleb128 0x11	/* (DW_AT_low_pc) */
> +	.uleb128 0x1	/* (DW_FORM_addr) */
> +	.uleb128 0x12	/* (DW_AT_high_pc) */
> +	.uleb128 0x6	/* (DW_FORM_data4) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x8	/* (abbrev code) */
> +	.uleb128 0x24	/* (TAG: DW_TAG_base_type) */
> +	.byte	0	/* DW_children_no */
> +	.uleb128 0xb	/* (DW_AT_byte_size) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x3e	/* (DW_AT_encoding) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x3	/* (DW_AT_name) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x13	/* (abbrev code) */
> +	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x3f	/* (DW_AT_external) */
> +	.uleb128 0x19	/* (DW_FORM_flag_present) */
> +	.uleb128 0x3	/* (DW_AT_name) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.uleb128 0x3a	/* (DW_AT_decl_file) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x3b	/* (DW_AT_decl_line) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x11	/* (DW_AT_low_pc) */
> +	.uleb128 0x1	/* (DW_FORM_addr) */
> +	.uleb128 0x12	/* (DW_AT_high_pc) */
> +	.uleb128 0x6	/* (DW_FORM_data4) */
> +	.uleb128 0x40	/* (DW_AT_frame_base) */
> +	.uleb128 0x18	/* (DW_FORM_exprloc) */
> +	.uleb128 0x2117	/* (DW_AT_GNU_all_call_sites) */
> +	.uleb128 0x19	/* (DW_FORM_flag_present) */
> +	.uleb128 0x1	/* (DW_AT_sibling) */
> +	.uleb128 0x13	/* (DW_FORM_ref4) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x15	/* (abbrev code) */
> +	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x3	/* (DW_AT_name) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.uleb128 0x3a	/* (DW_AT_decl_file) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x3b	/* (DW_AT_decl_line) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x20	/* (DW_AT_inline) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x1	/* (DW_AT_sibling) */
> +	.uleb128 0x13	/* (DW_FORM_ref4) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x18	/* (abbrev code) */
> +	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x31	/* (DW_AT_abstract_origin) */
> +	.uleb128 0x13	/* (DW_FORM_ref4) */
> +	.uleb128 0x11	/* (DW_AT_low_pc) */
> +	.uleb128 0x1	/* (DW_FORM_addr) */
> +	.uleb128 0x12	/* (DW_AT_high_pc) */
> +	.uleb128 0x6	/* (DW_FORM_data4) */
> +	.uleb128 0x40	/* (DW_AT_frame_base) */
> +	.uleb128 0x18	/* (DW_FORM_exprloc) */
> +	.uleb128 0x48	/* (DW_AT_static_link) */
> +	.uleb128 0x18	/* (DW_FORM_exprloc) */
> +	.uleb128 0x2117	/* (DW_AT_GNU_all_call_sites) */
> +	.uleb128 0x19	/* (DW_FORM_flag_present) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x1a	/* (abbrev code) */
> +	.uleb128 0x1d	/* (TAG: DW_TAG_inlined_subroutine) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x31	/* (DW_AT_abstract_origin) */
> +	.uleb128 0x13	/* (DW_FORM_ref4) */
> +	.uleb128 0x11	/* (DW_AT_low_pc) */
> +	.uleb128 0x1	/* (DW_FORM_addr) */
> +	.uleb128 0x12	/* (DW_AT_high_pc) */
> +	.uleb128 0x6	/* (DW_FORM_data4) */
> +	.uleb128 0x58	/* (DW_AT_call_file) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x59	/* (DW_AT_call_line) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x1	/* (DW_AT_sibling) */
> +	.uleb128 0x13	/* (DW_FORM_ref4) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x23	/* (abbrev code) */
> +	.uleb128 0x1d	/* (TAG: DW_TAG_inlined_subroutine) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x31	/* (DW_AT_abstract_origin) */
> +	.uleb128 0x13	/* (DW_FORM_ref4) */
> +	.uleb128 0x11	/* (DW_AT_low_pc) */
> +	.uleb128 0x1	/* (DW_FORM_addr) */
> +	.uleb128 0x12	/* (DW_AT_high_pc) */
> +	.uleb128 0x6	/* (DW_FORM_data4) */
> +	.uleb128 0x58	/* (DW_AT_call_file) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.uleb128 0x59	/* (DW_AT_call_line) */
> +	.uleb128 0xb	/* (DW_FORM_data1) */
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x28	/* (abbrev code) */
> +	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
> +	.byte	0x1	/* DW_children_yes */
> +	.uleb128 0x3f	/* (DW_AT_external) */
> +	.uleb128 0x19	/* (DW_FORM_flag_present) */
> +	.uleb128 0x3	/* (DW_AT_name) */
> +	.uleb128 0x8	/* (DW_FORM_string) */
> +	.uleb128 0x34	/* (DW_AT_artificial) */
> +	.uleb128 0x19	/* (DW_FORM_flag_present) */
> +	.uleb128 0x3c	/* (DW_AT_declaration) */
> +	.uleb128 0x19	/* (DW_FORM_flag_present) */
> +	.byte	0
> +	.byte	0
> +	.byte	0
> +	.byte	0
> +	.byte	0
> +
> +        .section .debug_line
> +.Lline1_begin:
> +        .byte   0
> +
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.c b/gdb/testsuite/gdb.dwarf2/dw2-icycle.c
> new file mode 100644
> index 0000000..3ddd194
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.c
> @@ -0,0 +1,24 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2004-2014 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* Dummy main function.  */
> +
> +int
> +main()
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp b/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp
> new file mode 100644
> index 0000000..fcb57b6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.exp
> @@ -0,0 +1,43 @@
> +# Copyright 2014 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    return 0
> +}
> +
> +standard_testfile .S .c
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} \
> +	  [list $srcfile $srcfile2] {nodebug}] } {
> +    return -1
> +}
> +
> +# We are trying to verify that the partial symtab to symtab expansion
> +# for the debugging info hand-coded in our assembly file does not cause
> +# the debugger to crash (infinite recursion).  To facilitate the test,
> +# start the debugger with -readnow.  This force expansion as soon as
> +# the objfile is loaded.
> +
> +set saved_gdbflags $GDBFLAGS
> +set GDBFLAGS "$GDBFLAGS -readnow"
> +clean_restart ${testfile}
> +set GDBFLAGS $saved_gdbflags
> +
> +# And just to be sure that the debugger did not crash after having
> +# expanded our symbols, do a life-check.
> +
> +gdb_test "echo life check\\n" "life check"
--
lin zuojian

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

> Hi.
> Sorry for the delay!

No, Thank You for looking into it! :)

> I spent some time looking for a cleverer patch, but in the end I think the
> simplicity of this patch is nice.

:)

> 
> 2014-02-18  lin zuojian  <manjian2006@gmail.com>
> 	    Joel Brobecker  <brobecker@adacore.com>
> 	    Doug Evans  <xdje42@gmail.com>
> 
> 	PR symtab/16581
> 	* 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.
> 	(inherit_abstract_dies): Only call process_die if origin_child_die
> 	not already being processed.
> 
> 	testsuite/
> 	* gdb.dwarf2/dw2-icycle.S: New file.
> 	* gdb.dwarf2/dw2-icycle.c: New file.
> 	* gdb.dwarf2/dw2-icycle.exp: New file.

That looks good to me, with one tiny little CS comment (see below).

> +static void
> +reset_die_in_process (void *arg)
> +{
> +  struct die_info *die = arg;
> +  die->in_process = 0;

Missing empty line after local variable declaration.

Thanks again,
-- 
Joel

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Doug Evans]

On Tue, Feb 18, 2014 at 11:59 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Hi.
>> Sorry for the delay!
>
> No, Thank You for looking into it! :)
>
>> I spent some time looking for a cleverer patch, but in the end I think the
>> simplicity of this patch is nice.
>
> :)
>
>>
>> 2014-02-18  lin zuojian  <manjian2006@gmail.com>
>>           Joel Brobecker  <brobecker@adacore.com>
>>           Doug Evans  <xdje42@gmail.com>
>>
>>       PR symtab/16581
>>       * 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.
>>       (inherit_abstract_dies): Only call process_die if origin_child_die
>>       not already being processed.
>>
>>       testsuite/
>>       * gdb.dwarf2/dw2-icycle.S: New file.
>>       * gdb.dwarf2/dw2-icycle.c: New file.
>>       * gdb.dwarf2/dw2-icycle.exp: New file.
>
> That looks good to me, with one tiny little CS comment (see below).
>
>> +static void
>> +reset_die_in_process (void *arg)
>> +{
>> +  struct die_info *die = arg;
>> +  die->in_process = 0;
>
> Missing empty line after local variable declaration.

Committed.
[with a small tweak to the testcase (added a comment)]

Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

> >> 2014-02-18  lin zuojian  <manjian2006@gmail.com>
> >>           Joel Brobecker  <brobecker@adacore.com>
> >>           Doug Evans  <xdje42@gmail.com>
> >>
> >>       PR symtab/16581
> >>       * 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.
> >>       (inherit_abstract_dies): Only call process_die if origin_child_die
> >>       not already being processed.
> >>
> >>       testsuite/
> >>       * gdb.dwarf2/dw2-icycle.S: New file.
> >>       * gdb.dwarf2/dw2-icycle.c: New file.
> >>       * gdb.dwarf2/dw2-icycle.exp: New file.
[...]
> Committed.
> [with a small tweak to the testcase (added a comment)]

I noticed something as well (in my testcase), which I think will make
it fail with an ARM bare-metal compiler. Fix attached and pushed.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-icycle.S: Remove second and third parameters
        in .section pseudo-op.

Tested on x86_64-linux...

-- 
Joel

[-- Attachment #2: 0001-Simplify-.section-in-dw2-icycle.S.patch --]
[-- Type: text/x-diff, Size: 1724 bytes --]

From 83deb43f87f098f7858db8643786301eb73875f7 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 20 Feb 2014 18:34:45 +0100
Subject: [PATCH] Simplify .section in dw2-icycle.S

The arm-elf assembler chokes on the extra parameters in the .section
pseudo-op, so this patch removes them.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-icycle.S: Remove second and third parameters
        in .section pseudo-op.
---
 gdb/testsuite/ChangeLog               | 5 +++++
 gdb/testsuite/gdb.dwarf2/dw2-icycle.S | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 52dff12..d1d99f8 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-02-20  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.dwarf2/dw2-icycle.S: Remove second and third parameters
+	in .section pseudo-op.
+
 2014-02-20  lin zuojian  <manjian2006@gmail.com>
 	    Joel Brobecker  <brobecker@adacore.com>
 	    Doug Evans  <xdje42@gmail.com>
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-icycle.S b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
index 6bc533a..1f84e4a 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
+++ b/gdb/testsuite/gdb.dwarf2/dw2-icycle.S
@@ -33,7 +33,7 @@ p__top:
 .LFE2:
 .Letext0:
 
-	.section	.debug_info,"",@progbits
+	.section	.debug_info
 .Ldebug_info0:
 	.4byte	.Ledebug_info0 - .Lsdebug_info0  /* Length of CU Info */
 .Lsdebug_info0:
@@ -118,7 +118,7 @@ p__top:
 	.byte	0	/* end of children of DIE 0xb */
 .Ledebug_info0:
 
-	.section	.debug_abbrev,"",@progbits
+	.section	.debug_abbrev
 .Ldebug_abbrev0:
 	.uleb128 0x1	/* (abbrev code) */
 	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
-- 
1.8.3.2