[rfa] set processing_current_prefix properly (PR gdb/1520)

[rfa] set processing_current_prefix properly (PR gdb/1520)

[David Carlton]

This is a fix for PR gdb/1520, a namespace problem with GCC 3.4.  The
problem was that, if we have this situation:

  namespace N {
    void foo() { }
  }

then the compiler I had been using generated dies as following in its
DWARF 2 output:

1: DW_TAG_namespace:

  2: DW_TAG_subprogram:

     // Definition of N::foo

whereas GCC head does:

1: DW_TAG_namespace:

  2: DW_TAG_subprogram:

     // Declaration for N::foo

3: DW_TAG_subprogram:

  DW_AT_specification: reference to die #2

  // Definition of N::foo.


So I've added code to notice if a die representing a function's
definition has a specification located elsewhere; if so, it looks at
that specification to discover the current enclosing class/namespace.

(Probably there are other places where we need to do this; hopefully,
after a bit more experience, we'll find a less ad-hoc way of handling
this issue.)

It also fixes an inconsistency in my last patch - I had tried to
maintain the invariant that processing_current_prefix was always
non-NULL (i.e. was an actual string, albeit possibly an empty one),
but I was using determine_prefix in ways that violated that invariant.

Tested on i686-pc-linux-gnu, DWARF 2, with GCC 3.2, GCC 3.2 +
DW_TAG_namespace patch, GCC 2.95.3, and GCC head.  No regressions;
fixes lots of FAILs in gdb.cp/namespace.exp with GCC head.  (From now
on, I'll probably stop testing with my patched GCC 3.2 and switch to
using a GCC snapshot generating DW_TAG_namespace, so I don't miss
problems like this.)

Okay to commit?

David Carlton
carlton@kealia.com

2004-01-19  David Carlton  <carlton@kealia.com>

	Patch for PR c++/1520:
	* dwarf2read.c (read_func_scope): Set processing_current_prefix
	properly if we have a specification die.
	(possibly_determine_prefix): Rename from determine_prefix.
	(determine_prefix): Like the old determine_prefix, but never
	returns NULL.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.122
diff -u -p -r1.122 dwarf2read.c
--- dwarf2read.c	17 Jan 2004 05:35:47 -0000	1.122
+++ dwarf2read.c	19 Jan 2004 20:20:13 -0000
@@ -774,6 +774,8 @@ static void read_type_die (struct die_in
 
 static char *determine_prefix (struct die_info *die);
 
+static char *possibly_determine_prefix (struct die_info *die);
+
 static char *typename_concat (const char *prefix, const char *suffix);
 
 static char *class_name (struct die_info *die);
@@ -2180,6 +2182,8 @@ read_func_scope (struct die_info *die, s
   struct die_info *child_die;
   struct attribute *attr;
   char *name;
+  const char *previous_prefix = processing_current_prefix;
+  struct cleanup *back_to = NULL;
 
   name = dwarf2_linkage_name (die);
 
@@ -2188,6 +2192,18 @@ read_func_scope (struct die_info *die, s
   if (name == NULL || !dwarf2_get_pc_bounds (die, &lowpc, &highpc, cu))
     return;
 
+  if (cu_language == language_cplus)
+    {
+      struct die_info *spec_die = die_specification (die);
+
+      if (spec_die != NULL)
+	{
+	  char *specification_prefix = determine_prefix (spec_die);
+	  processing_current_prefix = specification_prefix;
+	  back_to = make_cleanup (xfree, specification_prefix);
+	}
+    }
+
   lowpc += baseaddr;
   highpc += baseaddr;
 
@@ -2238,6 +2254,10 @@ read_func_scope (struct die_info *die, s
      symbols go in the file symbol list.  */
   if (outermost_context_p ())
     list_in_scope = &file_symbols;
+
+  processing_current_prefix = previous_prefix;
+  if (back_to != NULL)
+    do_cleanups (back_to);
 }
 
 /* Process all the DIES contained within a lexical block scope.  Start
@@ -5942,12 +5962,23 @@ read_type_die (struct die_info *die, str
   do_cleanups (back_to);
 }
 
+/* Return the name of the namespace/class that DIE is defined within,
+   or "" if we can't tell.  The caller should xfree the result.  */
+
+static char *
+determine_prefix (struct die_info *die)
+{
+  char *prefix = possibly_determine_prefix (die);
+
+  return prefix ? prefix : xstrdup ("");
+}
+
 /* Return the name of the namespace/class that DIE is defined
    within, or NULL if we can't tell.  The caller should xfree the
    result.  */
 
 static char *
-determine_prefix (struct die_info *die)
+possibly_determine_prefix (struct die_info *die)
 {
   struct die_info *parent;
 
@@ -5962,7 +5993,7 @@ determine_prefix (struct die_info *die)
     }
   else
     {
-      char *parent_prefix = determine_prefix (parent);
+      char *parent_prefix = possibly_determine_prefix (parent);
       char *retval;
 
       switch (parent->tag) {

Re: [rfa] set processing_current_prefix properly (PR gdb/1520)

[David Carlton]

On Mon, 19 Jan 2004 14:43:04 -0800, David Carlton <carlton@kealia.com> said:

> Tested on i686-pc-linux-gnu, DWARF 2, with GCC 3.2, GCC 3.2 +
> DW_TAG_namespace patch, GCC 2.95.3, and GCC head.  No regressions;
> fixes lots of FAILs in gdb.cp/namespace.exp with GCC head.

And the only FAILs in gdb.cp/namespace.exp that weren't fixed by this
patch when I tested it on Monday were due to a testsuite bug that I've
just committed a patch for.

There is still an issue with gdb.cp/rtti.exp; I have that tracked down
(classes, partial symtabs, and DW_AT_specification, sigh), and will
generate a patch for it soon.

David Carlton
carlton@kealia.com

Re: [rfa] set processing_current_prefix properly (PR gdb/1520)

[Elena Zannoni]

David Carlton writes:
 > This is a fix for PR gdb/1520, a namespace problem with GCC 3.4.  The
 > problem was that, if we have this situation:
 > 
 >   namespace N {
 >     void foo() { }
 >   }
 > 
 > then the compiler I had been using generated dies as following in its
 > DWARF 2 output:
 > 
 > 1: DW_TAG_namespace:
 > 
 >   2: DW_TAG_subprogram:
 > 
 >      // Definition of N::foo
 > 
 > whereas GCC head does:
 > 
 > 1: DW_TAG_namespace:
 > 
 >   2: DW_TAG_subprogram:
 > 
 >      // Declaration for N::foo
 > 
 > 3: DW_TAG_subprogram:
 > 
 >   DW_AT_specification: reference to die #2
 > 
 >   // Definition of N::foo.
 > 
 > 
 > So I've added code to notice if a die representing a function's
 > definition has a specification located elsewhere; if so, it looks at
 > that specification to discover the current enclosing class/namespace.
 > 


Can you add some of the above comments before the new call to check
for the specification?


 > (Probably there are other places where we need to do this; hopefully,
 > after a bit more experience, we'll find a less ad-hoc way of handling
 > this issue.)
 > 
 > It also fixes an inconsistency in my last patch - I had tried to
 > maintain the invariant that processing_current_prefix was always
 > non-NULL (i.e. was an actual string, albeit possibly an empty one),
 > but I was using determine_prefix in ways that violated that invariant.
 > 

So the only function that one should call in theory should be
determine_prefix, while possibly_determine_prefix is only there as a
worker function? Maybe this should be reflected in the names a bit
more explicitly. Like determine_prefix_worker or something like that
for the 'internal' one. I cannot think of a better term right now.

 > Tested on i686-pc-linux-gnu, DWARF 2, with GCC 3.2, GCC 3.2 +
 > DW_TAG_namespace patch, GCC 2.95.3, and GCC head.  No regressions;
 > fixes lots of FAILs in gdb.cp/namespace.exp with GCC head.  (From now
 > on, I'll probably stop testing with my patched GCC 3.2 and switch to
 > using a GCC snapshot generating DW_TAG_namespace, so I don't miss
 > problems like this.)
 > 
 > Okay to commit?
 > 

ok, modulus those 2 nits.

elena

Re: [rfa] set processing_current_prefix properly (PR gdb/1520)

[David Carlton]

On Fri, 23 Jan 2004 17:26:03 -0500, Elena Zannoni <ezannoni@redhat.com> said:

> Can you add some of the above comments before the new call to check
> for the specification?

Good idea.  I was just writing a comment like that for another patch
that I'm generating; I'll steal it and modify it accordingly for this
one.

> So the only function that one should call in theory should be
> determine_prefix, while possibly_determine_prefix is only there as a
> worker function? Maybe this should be reflected in the names a bit
> more explicitly. 

Good idea; I'll use determine_prefix_aux.

> ok, modulus those 2 nits.

Thanks, I've committed the attached.

David Carlton
carlton@kealia.com

2004-01-23  David Carlton  <carlton@kealia.com>

	Patch for PR c++/1520:
	* dwarf2read.c (read_func_scope): Set processing_current_prefix
	properly if we have a specification die.
	(determine_prefix_aux): Rename from determine_prefix.
	(determine_prefix): Like the old determine_prefix, but never
	returns NULL.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.123
diff -u -p -r1.123 dwarf2read.c
--- dwarf2read.c	22 Jan 2004 19:56:54 -0000	1.123
+++ dwarf2read.c	23 Jan 2004 22:39:42 -0000
@@ -774,6 +774,8 @@ static void read_type_die (struct die_in
 
 static char *determine_prefix (struct die_info *die);
 
+static char *determine_prefix_aux (struct die_info *die);
+
 static char *typename_concat (const char *prefix, const char *suffix);
 
 static char *class_name (struct die_info *die);
@@ -2145,6 +2147,8 @@ read_func_scope (struct die_info *die, s
   struct die_info *child_die;
   struct attribute *attr;
   char *name;
+  const char *previous_prefix = processing_current_prefix;
+  struct cleanup *back_to = NULL;
 
   name = dwarf2_linkage_name (die);
 
@@ -2153,6 +2157,40 @@ read_func_scope (struct die_info *die, s
   if (name == NULL || !dwarf2_get_pc_bounds (die, &lowpc, &highpc, cu))
     return;
 
+  if (cu_language == language_cplus)
+    {
+      struct die_info *spec_die = die_specification (die);
+
+	  /* NOTE: carlton/2004-01-23: We have to be careful in the
+	     presence of DW_AT_specification.  For example, with GCC
+	     3.4, given the code
+
+               namespace N {
+	         void foo() {
+		   // Definition of N::foo.
+	         }
+	       }
+
+	     then we'll have a tree of DIEs like this:
+
+	     1: DW_TAG_compile_unit
+               2: DW_TAG_namespace        // N
+                 3: DW_TAG_subprogram     // declaration of N::foo
+               4: DW_TAG_subprogram       // definition of N::foo
+                    DW_AT_specification   // refers to die #3
+
+             Thus, when processing die #4, we have to pretend that
+             we're in the context of its DW_AT_specification, namely
+             the contex of die #3.  */
+	
+      if (spec_die != NULL)
+	{
+	  char *specification_prefix = determine_prefix (spec_die);
+	  processing_current_prefix = specification_prefix;
+	  back_to = make_cleanup (xfree, specification_prefix);
+	}
+    }
+
   lowpc += baseaddr;
   highpc += baseaddr;
 
@@ -2203,6 +2241,10 @@ read_func_scope (struct die_info *die, s
      symbols go in the file symbol list.  */
   if (outermost_context_p ())
     list_in_scope = &file_symbols;
+
+  processing_current_prefix = previous_prefix;
+  if (back_to != NULL)
+    do_cleanups (back_to);
 }
 
 /* Process all the DIES contained within a lexical block scope.  Start
@@ -5969,12 +6011,27 @@ read_type_die (struct die_info *die, str
   do_cleanups (back_to);
 }
 
+/* Return the name of the namespace/class that DIE is defined within,
+   or "" if we can't tell.  The caller should xfree the result.  */
+
+/* NOTE: carlton/2004-01-23: See read_func_scope (and the comment
+   therein) for an example of how to use this function to deal with
+   DW_AT_specification.  */
+
+static char *
+determine_prefix (struct die_info *die)
+{
+  char *prefix = determine_prefix_aux (die);
+
+  return prefix ? prefix : xstrdup ("");
+}
+
 /* Return the name of the namespace/class that DIE is defined
    within, or NULL if we can't tell.  The caller should xfree the
    result.  */
 
 static char *
-determine_prefix (struct die_info *die)
+determine_prefix_aux (struct die_info *die)
 {
   struct die_info *parent;
 
@@ -5989,7 +6046,7 @@ determine_prefix (struct die_info *die)
     }
   else
     {
-      char *parent_prefix = determine_prefix (parent);
+      char *parent_prefix = determine_prefix_aux (parent);
       char *retval;
 
       switch (parent->tag) {