Re: recursive bug in dwarf2read.c

Re: recursive bug in dwarf2read.c

[Gaius Mulley]

Daniel Jacobowitz <drow@false.org> writes:

> On Wed, Dec 09, 2009 at 10:55:30AM +0000, Gaius Mulley wrote:
>> 
>> Hi,
>> 
>> I think there is a bug in dwarf2read.c which can be exposed by
>> attempting to set a break point at RealIO_WriteFixed in this
>> executable.
>> 
>> I acknowledge the (gnu modula-2 generated executable) could probably
>> be at fault, nevertheless it does contain cyclic data structures which
>> are not explicitly created in the same way as C.  For example:
>> 
>> 
>> TYPE
>>   ScanState =  PROCEDURE (CHAR, VAR ScanClass, VAR ScanState);
>
> See e.g. this from read_structure_type:
>
>   /* We need to add the type field to the die immediately so we don't
>      infinitely recurse when dealing with pointers to the structure
>      type within the structure itself. */
>   set_die_type (die, type, cu);
>
> It looks as if read_subroutine_type does not do that, but now has to.

Hi Daniel,

here is a patch which fixes the bug above.  Embarrassingly after
checking my private gdb directories I found I fixed this last year
and sat on and forgot the patch.  Guess there is a moral here
somewhere..


2009-12-10  Gaius Mulley  <gaius@glam.ac.uk>

	* dwarf2read.c (read_subroutine_type): Add the subroutine
	  type to the die immediately to allow a parameter type to be
	  the same subroutine type.


--- src-orig/gdb/dwarf2read.c	2009-12-10 16:41:40.000000000 +0000
+++ src/gdb/dwarf2read.c	2009-12-10 17:04:16.000000000 +0000
@@ -5879,6 +5879,11 @@
      the default value DW_CC_normal.  */
   attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
   TYPE_CALLING_CONVENTION (ftype) = attr ? DW_UNSND (attr) : DW_CC_normal;
+
+  /* We need to add the subroutine type to the die immediately so
+     we don't infinitely recurse when dealing with parameters
+     declared as the same subroutine type. */
+  set_die_type (die, ftype, cu);
   
   if (die->child != NULL)
     {


I've ran the testsuite on a AMD64 debian stable (lenny) system and and
it doesn't appear to cause any regression.  I've also some minor
Modula-2 language support improvement patches which I wrote in Sept
which I'll now push out to the mailing list..

regards,
Gaius

Re: recursive bug in dwarf2read.c

[Tom Tromey]

>>>>> "Gaius" == Gaius Mulley <gaius@glam.ac.uk> writes:

Gaius> 2009-12-10  Gaius Mulley  <gaius@glam.ac.uk>
Gaius> 	* dwarf2read.c (read_subroutine_type): Add the subroutine
Gaius> 	  type to the die immediately to allow a parameter type to be
Gaius> 	  the same subroutine type.

This seems almost ok to me.

With this change, read_subroutine_type calls set_die_type twice.
I think it would be preferable to eliminate the second call.

Tom

Re: recursive bug in dwarf2read.c

[Gaius Mulley]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Gaius" == Gaius Mulley <gaius@glam.ac.uk> writes:
>
> Gaius> 2009-12-10  Gaius Mulley  <gaius@glam.ac.uk>
> Gaius> 	* dwarf2read.c (read_subroutine_type): Add the subroutine
> Gaius> 	  type to the die immediately to allow a parameter type to be
> Gaius> 	  the same subroutine type.
>
> This seems almost ok to me.
>
> With this change, read_subroutine_type calls set_die_type twice.
> I think it would be preferable to eliminate the second call.

Hi Tom,

ahh yes - true, here is the modified patch without the redundant
second call:

--- src-orig/gdb/dwarf2read.c	2009-12-10 16:41:40.000000000 +0000
+++ src/gdb/dwarf2read.c	2009-12-11 20:31:52.000000000 +0000
@@ -5879,6 +5879,11 @@
      the default value DW_CC_normal.  */
   attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
   TYPE_CALLING_CONVENTION (ftype) = attr ? DW_UNSND (attr) : DW_CC_normal;
+
+  /* We need to add the subroutine type to the die immediately so
+     we don't infinitely recurse when dealing with parameters
+     declared as the same subroutine type. */
+  set_die_type (die, ftype, cu);
   
   if (die->child != NULL)
     {
@@ -5926,7 +5931,7 @@
 	}
     }
 
-  return set_die_type (die, ftype, cu);
+  return ftype;
 }
 
 static struct type *

2009-12-11  Gaius Mulley  <gaius@glam.ac.uk>

	* dwarf2read.c (read_subroutine_type): Add the subroutine
	  type to the die immediately to allow a parameter type to be
	  the same subroutine type.


regards,
Gaius

Re: recursive bug in dwarf2read.c

[Tom Tromey]

>>>>> "Gaius" == Gaius Mulley <gaius@glam.ac.uk> writes:

Gaius> ahh yes - true, here is the modified patch without the redundant
Gaius> second call:

Thanks, this is ok.

Tom

Re: recursive bug in dwarf2read.c

[Gaius Mulley]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Gaius" == Gaius Mulley <gaius@glam.ac.uk> writes:
>
> Gaius> ahh yes - true, here is the modified patch without the redundant
> Gaius> second call:
>
> Thanks, this is ok.
>
> Tom

Hi Tom,

many thanks, now committed to the head cvs,

regards,
Gaius

2009-12-15  Gaius Mulley  <gaius@glam.ac.uk>

      * dwarf2read.c (read_subroutine_type): Add the subroutine
        type to the die immediately to allow a parameter type to be
        the same subroutine type.