[rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[David Carlton]

On Tue, 16 Mar 2004 10:25:39 -0800, David Carlton <carlton@kealia.com> said:

> So what's the correct fix here?  I tend to think that the code would
> be easier to understand if we only generated symbols while going
> through the code in the obvious tree order (calling functions named
> process_XXX, ideally), instead of while following various
> cross-references (which we would only do via functions named read_XXX,
> ideally).  Is that a reasonable hope?  If so, it seems like the
> correct fix would be to change process_structure_scope to call
> process_die on all of its children, whether or not the current die is
> a declaration.  I'll play around with a patch like that - it should be
> safe, I hope, since process_structure_scope is only called from
> process_die, so we shouldn't be generating symbols twice.

Here's a patch implementing that.  It looks messier than it is - all I
did was move the loop over children before the test for whether or not
we're a declaration.  I've tested it on mainline with
i686-pc-linux-gnu, DWARF-2, and four different GCC versions; no new
regressions, and it fixes the regression in question.  Is it okay to
commit?  If so, is it also okay for 6.1 (assuming that the tests pass
there as well, which I'm about to start checking)?

David Carlton
carlton@kealia.com

2004-03-16  David Carlton  <carlton@kealia.com>

	* dwarf2read.c (process_structure_scope): Process children even
	when we're a declaration.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.140
diff -u -p -r1.140 dwarf2read.c
--- dwarf2read.c	15 Mar 2004 22:33:52 -0000	1.140
+++ dwarf2read.c	16 Mar 2004 19:10:52 -0000
@@ -3331,32 +3331,34 @@ process_structure_scope (struct die_info
 {
   struct objfile *objfile = cu->objfile;
   const char *previous_prefix = processing_current_prefix;
+  struct die_info *child_die = die->child;
 
   if (TYPE_TAG_NAME (die->type) != NULL)
     processing_current_prefix = TYPE_TAG_NAME (die->type);
 
-  if (die->child != NULL && ! die_is_declaration (die, cu))
-    {
-      struct die_info *child_die;
+  /* NOTE: carlton/2004-03-16: GCC 3.4 (or at least one of its
+     snapshots) has been known to create a die giving a declaration
+     for a class that has, as a child, a die giving a definition for a
+     nested class.  So we have to process our children even if the
+     current die is a declaration.  Normally, of course, a declaration
+     won't have any children at all.  */
 
-      child_die = die->child;
-
-      while (child_die && child_die->tag)
+  while (child_die != NULL && child_die->tag)
+    {
+      if (child_die->tag == DW_TAG_member
+	  || child_die->tag == DW_TAG_variable
+	  || child_die->tag == DW_TAG_inheritance)
 	{
-	  if (child_die->tag == DW_TAG_member
-	      || child_die->tag == DW_TAG_variable
-	      || child_die->tag == DW_TAG_inheritance)
-	    {
-	      /* Do nothing.  */
-	    }
-	  else
-	    process_die (child_die, cu);
-
-	  child_die = sibling_die (child_die);
+	  /* Do nothing.  */
 	}
+      else
+	process_die (child_die, cu);
 
-      new_symbol (die, die->type, cu);
+      child_die = sibling_die (child_die);
     }
+
+  if (die->child != NULL && ! die_is_declaration (die, cu))
+    new_symbol (die, die->type, cu);
 
   processing_current_prefix = previous_prefix;
 }

[rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[David Carlton]

On Tue, 16 Mar 2004 10:25:39 -0800, David Carlton <carlton@kealia.com> said:

> So what's the correct fix here?  I tend to think that the code would
> be easier to understand if we only generated symbols while going
> through the code in the obvious tree order (calling functions named
> process_XXX, ideally), instead of while following various
> cross-references (which we would only do via functions named read_XXX,
> ideally).  Is that a reasonable hope?  If so, it seems like the
> correct fix would be to change process_structure_scope to call
> process_die on all of its children, whether or not the current die is
> a declaration.  I'll play around with a patch like that - it should be
> safe, I hope, since process_structure_scope is only called from
> process_die, so we shouldn't be generating symbols twice.

Here's a patch implementing that.  It looks messier than it is - all I
did was move the loop over children before the test for whether or not
we're a declaration.  I've tested it on mainline with
i686-pc-linux-gnu, DWARF-2, and four different GCC versions; no new
regressions, and it fixes the regression in question.  Is it okay to
commit?  If so, is it also okay for 6.1 (assuming that the tests pass
there as well, which I'm about to start checking)?

David Carlton
carlton@kealia.com

2004-03-16  David Carlton  <carlton@kealia.com>

	* dwarf2read.c (process_structure_scope): Process children even
	when we're a declaration.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.140
diff -u -p -r1.140 dwarf2read.c
--- dwarf2read.c	15 Mar 2004 22:33:52 -0000	1.140
+++ dwarf2read.c	16 Mar 2004 19:10:52 -0000
@@ -3331,32 +3331,34 @@ process_structure_scope (struct die_info
 {
   struct objfile *objfile = cu->objfile;
   const char *previous_prefix = processing_current_prefix;
+  struct die_info *child_die = die->child;
 
   if (TYPE_TAG_NAME (die->type) != NULL)
     processing_current_prefix = TYPE_TAG_NAME (die->type);
 
-  if (die->child != NULL && ! die_is_declaration (die, cu))
-    {
-      struct die_info *child_die;
+  /* NOTE: carlton/2004-03-16: GCC 3.4 (or at least one of its
+     snapshots) has been known to create a die giving a declaration
+     for a class that has, as a child, a die giving a definition for a
+     nested class.  So we have to process our children even if the
+     current die is a declaration.  Normally, of course, a declaration
+     won't have any children at all.  */
 
-      child_die = die->child;
-
-      while (child_die && child_die->tag)
+  while (child_die != NULL && child_die->tag)
+    {
+      if (child_die->tag == DW_TAG_member
+	  || child_die->tag == DW_TAG_variable
+	  || child_die->tag == DW_TAG_inheritance)
 	{
-	  if (child_die->tag == DW_TAG_member
-	      || child_die->tag == DW_TAG_variable
-	      || child_die->tag == DW_TAG_inheritance)
-	    {
-	      /* Do nothing.  */
-	    }
-	  else
-	    process_die (child_die, cu);
-
-	  child_die = sibling_die (child_die);
+	  /* Do nothing.  */
 	}
+      else
+	process_die (child_die, cu);
 
-      new_symbol (die, die->type, cu);
+      child_die = sibling_die (child_die);
     }
+
+  if (die->child != NULL && ! die_is_declaration (die, cu))
+    new_symbol (die, die->type, cu);
 
   processing_current_prefix = previous_prefix;
 }

Re: [rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[Elena Zannoni]

David Carlton writes:
 > On Tue, 16 Mar 2004 10:25:39 -0800, David Carlton <carlton@kealia.com> said:
 > 
 > > So what's the correct fix here?  I tend to think that the code would
 > > be easier to understand if we only generated symbols while going
 > > through the code in the obvious tree order (calling functions named
 > > process_XXX, ideally), instead of while following various
 > > cross-references (which we would only do via functions named read_XXX,
 > > ideally).  Is that a reasonable hope?  If so, it seems like the
 > > correct fix would be to change process_structure_scope to call
 > > process_die on all of its children, whether or not the current die is
 > > a declaration.  I'll play around with a patch like that - it should be
 > > safe, I hope, since process_structure_scope is only called from
 > > process_die, so we shouldn't be generating symbols twice.
 > 
 > Here's a patch implementing that.  It looks messier than it is - all I
 > did was move the loop over children before the test for whether or not
 > we're a declaration.  I've tested it on mainline with
 > i686-pc-linux-gnu, DWARF-2, and four different GCC versions; no new
 > regressions, and it fixes the regression in question.  Is it okay to
 > commit?  If so, is it also okay for 6.1 (assuming that the tests pass
 > there as well, which I'm about to start checking)?

Fine, yes.

Maybe Daniel should have a look too?

elena

Re: [rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[Elena Zannoni]

David Carlton writes:
 > On Tue, 16 Mar 2004 10:25:39 -0800, David Carlton <carlton@kealia.com> said:
 > 
 > > So what's the correct fix here?  I tend to think that the code would
 > > be easier to understand if we only generated symbols while going
 > > through the code in the obvious tree order (calling functions named
 > > process_XXX, ideally), instead of while following various
 > > cross-references (which we would only do via functions named read_XXX,
 > > ideally).  Is that a reasonable hope?  If so, it seems like the
 > > correct fix would be to change process_structure_scope to call
 > > process_die on all of its children, whether or not the current die is
 > > a declaration.  I'll play around with a patch like that - it should be
 > > safe, I hope, since process_structure_scope is only called from
 > > process_die, so we shouldn't be generating symbols twice.
 > 
 > Here's a patch implementing that.  It looks messier than it is - all I
 > did was move the loop over children before the test for whether or not
 > we're a declaration.  I've tested it on mainline with
 > i686-pc-linux-gnu, DWARF-2, and four different GCC versions; no new
 > regressions, and it fixes the regression in question.  Is it okay to
 > commit?  If so, is it also okay for 6.1 (assuming that the tests pass
 > there as well, which I'm about to start checking)?

Fine, yes.

Maybe Daniel should have a look too?

elena

Re: [rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[Daniel Jacobowitz]

On Tue, Mar 16, 2004 at 03:33:29PM -0500, Elena Zannoni wrote:
> David Carlton writes:
>  > On Tue, 16 Mar 2004 10:25:39 -0800, David Carlton <carlton@kealia.com> said:
>  > 
>  > > So what's the correct fix here?  I tend to think that the code would
>  > > be easier to understand if we only generated symbols while going
>  > > through the code in the obvious tree order (calling functions named
>  > > process_XXX, ideally), instead of while following various
>  > > cross-references (which we would only do via functions named read_XXX,
>  > > ideally).  Is that a reasonable hope?  If so, it seems like the
>  > > correct fix would be to change process_structure_scope to call
>  > > process_die on all of its children, whether or not the current die is
>  > > a declaration.  I'll play around with a patch like that - it should be
>  > > safe, I hope, since process_structure_scope is only called from
>  > > process_die, so we shouldn't be generating symbols twice.
>  > 
>  > Here's a patch implementing that.  It looks messier than it is - all I
>  > did was move the loop over children before the test for whether or not
>  > we're a declaration.  I've tested it on mainline with
>  > i686-pc-linux-gnu, DWARF-2, and four different GCC versions; no new
>  > regressions, and it fixes the regression in question.  Is it okay to
>  > commit?  If so, is it also okay for 6.1 (assuming that the tests pass
>  > there as well, which I'm about to start checking)?
> 
> Fine, yes.
> 
> Maybe Daniel should have a look too?

This looks right to me, too - I think I just goofed when I split
read_structure_scope.  Thanks!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[Daniel Jacobowitz]

On Tue, Mar 16, 2004 at 03:33:29PM -0500, Elena Zannoni wrote:
> David Carlton writes:
>  > On Tue, 16 Mar 2004 10:25:39 -0800, David Carlton <carlton@kealia.com> said:
>  > 
>  > > So what's the correct fix here?  I tend to think that the code would
>  > > be easier to understand if we only generated symbols while going
>  > > through the code in the obvious tree order (calling functions named
>  > > process_XXX, ideally), instead of while following various
>  > > cross-references (which we would only do via functions named read_XXX,
>  > > ideally).  Is that a reasonable hope?  If so, it seems like the
>  > > correct fix would be to change process_structure_scope to call
>  > > process_die on all of its children, whether or not the current die is
>  > > a declaration.  I'll play around with a patch like that - it should be
>  > > safe, I hope, since process_structure_scope is only called from
>  > > process_die, so we shouldn't be generating symbols twice.
>  > 
>  > Here's a patch implementing that.  It looks messier than it is - all I
>  > did was move the loop over children before the test for whether or not
>  > we're a declaration.  I've tested it on mainline with
>  > i686-pc-linux-gnu, DWARF-2, and four different GCC versions; no new
>  > regressions, and it fixes the regression in question.  Is it okay to
>  > commit?  If so, is it also okay for 6.1 (assuming that the tests pass
>  > there as well, which I'm about to start checking)?
> 
> Fine, yes.
> 
> Maybe Daniel should have a look too?

This looks right to me, too - I think I just goofed when I split
read_structure_scope.  Thanks!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[David Carlton]

On Tue, 16 Mar 2004 17:28:42 -0500, Daniel Jacobowitz <drow@mvista.com> said:
> On Tue, Mar 16, 2004 at 03:33:29PM -0500, Elena Zannoni wrote:

>> Fine, yes.
>> 
>> Maybe Daniel should have a look too?

> This looks right to me, too - I think I just goofed when I split
> read_structure_scope.  Thanks!

Thanks, committed.

David Carlton
carlton@kealia.com

Re: [rfa+6.1]: Fix gcc 3.4 regression in gdb.cp/namespace.exp

[David Carlton]

On Tue, 16 Mar 2004 17:28:42 -0500, Daniel Jacobowitz <drow@mvista.com> said:
> On Tue, Mar 16, 2004 at 03:33:29PM -0500, Elena Zannoni wrote:

>> Fine, yes.
>> 
>> Maybe Daniel should have a look too?

> This looks right to me, too - I think I just goofed when I split
> read_structure_scope.  Thanks!

Thanks, committed.

David Carlton
carlton@kealia.com