Re: [RFA 2/4] dwarf2_physname

[Keith Seitz <keiths@redhat.com>]

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

On 02/04/2010 10:14 AM, Daniel Jacobowitz wrote:

> That's part of it.  Also, I don't think the check for DW_AT_external
> tests what we want (which is what I'm trying to pin down - what is the
> desired property?).

Yes, we are trying to ascertain whether the DIE (a DW_TAG_variable) in 
question needs to be prefixed -- same job it used to do AFAICT. The 
difference is that it is now being used for a much more complex job, 
which requires us to refine the behavior of pdi_needs_namespace for more 
inputs. In particular, it is the use of dwarf2_physname in new_symbol 
which necessitates this change.

Previously, this code (pdi_needs_namespace) unconditionally returned 1 
(for this DW_TAG_variable case), which is not unilaterally correct anymore.

>> 1. If the variable is declared in the namespace scope, but defined
>> outside, then a second defining DIE is emitted that refers to the
>> declaration DIE using DW_AT_specification.  In this case the declaration
>> DIE's namespace is used.
>>
>> ... The code above seems to cheat a tiny bit because it unconditionally
>> returns 1 in this case, whereas it should perhaps recurse.
>
> Recursing's a good idea.  I hadn't thought of that.

I hadn't either. Next iteration in attached patch.

BTW, while goofing around with the code a bit, it appears that since I 
wrote all of this many, many months ago, things have changed (for the 
better). physname_prefix and dependents are no longer necessary. We can 
use determine_prefix instead. I apologize that I did not catch this earlier.

Since dwarf2_full_name and dwarf2_physname are almost identical, I've 
also merged the two functions.

I've also added a NAME parameter so that callers can pass in the result 
of a previous dwarf2_name call. I noticed quite some time ago that this 
method was being called several times in a row.

I've appended an updated patch of dwarf2read.c with CVS HEAD.

Keith

[-- Attachment #2: physname-dwarf2read.c-revised.patch --]
[-- Type: text/plain, Size: 16967 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.352
diff -u -p -r1.352 dwarf2read.c
--- dwarf2read.c	26 Jan 2010 15:48:25 -0000	1.352
+++ dwarf2read.c	5 Feb 2010 17:03:36 -0000
@@ -48,6 +48,8 @@
 #include "gdbcmd.h"
 #include "block.h"
 #include "addrmap.h"
+#include "typeprint.h"
+#include "jv-lang.h"
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -487,8 +489,7 @@ struct partial_die_info
     unsigned int has_byte_size : 1;
 
     /* The name of this DIE.  Normally the value of DW_AT_name, but
-       sometimes DW_TAG_MIPS_linkage_name or a string computed in some
-       other fashion.  */
+       sometimes a default name for unnamed DIEs.  */
     char *name;
 
     /* The scope to prepend to our children.  This is generally
@@ -788,8 +789,6 @@ static void scan_partial_symbols (struct
 static void add_partial_symbol (struct partial_die_info *,
 				struct dwarf2_cu *);
 
-static int pdi_needs_namespace (enum dwarf_tag tag);
-
 static void add_partial_namespace (struct partial_die_info *pdi,
 				   CORE_ADDR *lowpc, CORE_ADDR *highpc,
 				   int need_pc, struct dwarf2_cu *cu);
@@ -984,9 +983,6 @@ static void dwarf2_attach_fn_fields_to_t
 
 static void process_structure_scope (struct die_info *, struct dwarf2_cu *);
 
-static const char *determine_class_name (struct die_info *die,
-					 struct dwarf2_cu *cu);
-
 static void read_common_block (struct die_info *, struct dwarf2_cu *);
 
 static void read_namespace (struct die_info *die, struct dwarf2_cu *);
@@ -1028,8 +1024,6 @@ static gdb_byte *read_full_die (const st
 
 static void process_die (struct die_info *, struct dwarf2_cu *);
 
-static char *dwarf2_linkage_name (struct die_info *, struct dwarf2_cu *);
-
 static char *dwarf2_canonicalize_name (char *, struct dwarf2_cu *,
 				       struct obstack *);
 
@@ -2441,12 +2435,9 @@ add_partial_symbol (struct partial_die_i
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
-  if (pdi_needs_namespace (pdi->tag))
-    {
-      actual_name = partial_die_full_name (pdi, cu);
-      if (actual_name)
-	built_actual_name = 1;
-    }
+  actual_name = partial_die_full_name (pdi, cu);
+  if (actual_name)
+    built_actual_name = 1;
 
   if (actual_name == NULL)
     actual_name = pdi->name;
@@ -2586,49 +2577,10 @@ add_partial_symbol (struct partial_die_i
       break;
     }
 
-  /* Check to see if we should scan the name for possible namespace
-     info.  Only do this if this is C++, if we don't have namespace
-     debugging info in the file, if the psym is of an appropriate type
-     (otherwise we'll have psym == NULL), and if we actually had a
-     mangled name to begin with.  */
-
-  /* FIXME drow/2004-02-22: Why don't we do this for classes, i.e. the
-     cases which do not set PSYM above?  */
-
-  if (cu->language == language_cplus
-      && cu->has_namespace_info == 0
-      && psym != NULL
-      && SYMBOL_CPLUS_DEMANGLED_NAME (psym) != NULL)
-    cp_check_possible_namespace_symbols (SYMBOL_CPLUS_DEMANGLED_NAME (psym),
-					 objfile);
-
   if (built_actual_name)
     xfree (actual_name);
 }
 
-/* Determine whether a die of type TAG living in a C++ class or
-   namespace needs to have the name of the scope prepended to the
-   name listed in the die.  */
-
-static int
-pdi_needs_namespace (enum dwarf_tag tag)
-{
-  switch (tag)
-    {
-    case DW_TAG_namespace:
-    case DW_TAG_typedef:
-    case DW_TAG_class_type:
-    case DW_TAG_interface_type:
-    case DW_TAG_structure_type:
-    case DW_TAG_union_type:
-    case DW_TAG_enumeration_type:
-    case DW_TAG_enumerator:
-      return 1;
-    default:
-      return 0;
-    }
-}
-
 /* Read a partial die corresponding to a namespace; also, add a symbol
    corresponding to that namespace to the symbol table.  NAMESPACE is
    the name of the enclosing namespace.  */
@@ -2739,7 +2691,6 @@ guess_structure_name (struct partial_die
 	 could fix this by only using the demangled name to get the
 	 prefix (but see comment in read_structure_type).  */
 
-      struct partial_die_info *child_pdi = struct_pdi->die_child;
       struct partial_die_info *real_pdi;
 
       /* If this DIE (this DIE's specification, if any) has a parent, then
@@ -2752,27 +2703,6 @@ guess_structure_name (struct partial_die
 
       if (real_pdi->die_parent != NULL)
 	return;
-
-      while (child_pdi != NULL)
-	{
-	  if (child_pdi->tag == DW_TAG_subprogram)
-	    {
-	      char *actual_class_name
-		= language_class_name_from_physname (cu->language_defn,
-						     child_pdi->name);
-	      if (actual_class_name != NULL)
-		{
-		  struct_pdi->name
-		    = obsavestring (actual_class_name,
-				    strlen (actual_class_name),
-				    &cu->comp_unit_obstack);
-		  xfree (actual_class_name);
-		}
-	      break;
-	    }
-
-	  child_pdi = child_pdi->die_sibling;
-	}
     }
 }
 
@@ -3337,42 +3267,147 @@ process_die (struct die_info *die, struc
     }
 }
 
+/* A helper function for dwarf2_compute_name which determines whether DIE
+   needs to have the name of the scope prepended to the name listed in the
+   die.  */
+
+static int
+die_needs_namespace (struct die_info *die, struct dwarf2_cu *cu)
+{
+  switch (die->tag)
+    {
+    case DW_TAG_namespace:
+    case DW_TAG_typedef:
+    case DW_TAG_class_type:
+    case DW_TAG_interface_type:
+    case DW_TAG_structure_type:
+    case DW_TAG_union_type:
+    case DW_TAG_enumeration_type:
+    case DW_TAG_enumerator:
+    case DW_TAG_subprogram:
+    case DW_TAG_member:
+      return 1;
+
+    case DW_TAG_variable:
+      if (dwarf2_attr (die, DW_AT_specification, cu))
+	{
+	  struct dwarf2_cu *spec_cu = cu;
+	  return die_needs_namespace (die_specification (die, &spec_cu),
+				      spec_cu);
+	}
+
+      return die_needs_namespace (die->parent, cu);
+
+    default:
+      return 0;
+    }
+}
+
+/* Compute the fully qualified name of DIE in CU.  If PHYSNAME is nonzero,
+   compute the physname for the object, which include a method's
+   formal parameters (C++/Java) and return type (Java).
+
+   The result is allocated on the objfile_obstack and canonicalized.  */
+
+static const char *
+dwarf2_compute_name (char *name, struct die_info *die, struct dwarf2_cu *cu,
+		     int physname)
+{
+  if (name == NULL)
+    name = dwarf2_name (die, cu);
+
+  /* These are the only languages we know how to qualify names in.  */
+  if (name != NULL
+      && (cu->language == language_cplus || cu->language == language_java))
+    {
+      if (die_needs_namespace (die, cu))
+	{
+	  long length;
+	  char *prefix;
+	  struct ui_file *buf;
+
+	  prefix = determine_prefix (die, cu);
+	  buf = mem_fileopen ();
+	  if (*prefix != '\0')
+	    {
+	      char *prefixed_name = typename_concat (NULL, prefix, name, cu);
+	      fputs_unfiltered (prefixed_name, buf);
+	      xfree (prefixed_name);
+	    }
+	  else
+	    fputs_unfiltered (name ? name : "", buf);
+
+	  /* For Java and C++ methods, append formal parameter type
+	     information, if PHYSNAME.  */
+	  
+	  if (physname && die->tag == DW_TAG_subprogram
+	      && (cu->language == language_cplus
+		  || cu->language == language_java))
+	    {
+	      struct type *type = read_type_die (die, cu);
+
+	      c_type_print_args (type, buf, 0, cu->language);
+
+	      if (cu->language == language_java)
+		{
+		  /* For java, we must append the return type to method
+		     names. */
+		  if (die->tag == DW_TAG_subprogram)
+		    java_print_type (TYPE_TARGET_TYPE (type), "", buf,
+				     0, 0);
+		}
+	      else if (cu->language == language_cplus)
+		{
+		  if (TYPE_NFIELDS (type) > 0
+		      && TYPE_FIELD_ARTIFICIAL (type, 0)
+		      && TYPE_CONST (TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type, 0))))
+		    fputs_unfiltered (" const", buf);
+		}
+	    }
+
+	  name = ui_file_obsavestring (buf, &cu->objfile->objfile_obstack,
+				       &length);
+	  ui_file_delete (buf);
+
+	  if (cu->language == language_cplus)
+	    {
+	      char *cname
+		= dwarf2_canonicalize_name (name, cu,
+					    &cu->objfile->objfile_obstack);
+	      if (cname != NULL)
+		name = cname;
+	    }
+	}
+    }
+
+  return name;
+}
+
 /* Return the fully qualified name of DIE, based on its DW_AT_name.
    If scope qualifiers are appropriate they will be added.  The result
    will be allocated on the objfile_obstack, or NULL if the DIE does
-   not have a name.  */
+   not have a name.  NAME may either be from a previous call to
+   dwarf2_name or NULL.
+
+   The output string will be canonicalized (if C++/Java). */
 
 static const char *
-dwarf2_full_name (struct die_info *die, struct dwarf2_cu *cu)
+dwarf2_full_name (char *name, struct die_info *die, struct dwarf2_cu *cu)
 {
-  struct attribute *attr;
-  char *prefix, *name;
-  struct ui_file *buf = NULL;
+  return dwarf2_compute_name (name, die, cu, 0);
+}
 
-  name = dwarf2_name (die, cu);
-  if (!name)
-    return NULL;
+/* Construct a physname for the given DIE in CU.  NAME may either be
+   from a previous call to dwarf2_name or NULL.  The result will be
+   allocated on teh objfile_objstack or NULL if the DIE does not have a
+   name.
 
-  /* These are the only languages we know how to qualify names in.  */
-  if (cu->language != language_cplus
-      && cu->language != language_java)
-    return name;
+   The output string will be canonicalized (if C++/Java).  */
 
-  /* If no prefix is necessary for this type of DIE, return the
-     unqualified name.  The other three tags listed could be handled
-     in pdi_needs_namespace, but that requires broader changes.  */
-  if (!pdi_needs_namespace (die->tag)
-      && die->tag != DW_TAG_subprogram
-      && die->tag != DW_TAG_variable
-      && die->tag != DW_TAG_member)
-    return name;
-
-  prefix = determine_prefix (die, cu);
-  if (*prefix != '\0')
-    name = typename_concat (&cu->objfile->objfile_obstack, prefix,
-			    name, cu);
-
-  return name;
+static const char *
+dwarf2_physname (char *name, struct die_info *die, struct dwarf2_cu *cu)
+{
+  return dwarf2_compute_name (name, die, cu, 1);
 }
 
 /* Read the import statement specified by the given die and record it.  */
@@ -3843,7 +3878,7 @@ read_func_scope (struct die_info *die, s
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
-  name = dwarf2_linkage_name (die, cu);
+  name = dwarf2_name (die, cu);
 
   /* Ignore functions with missing or empty names and functions with
      missing or invalid low and high pc attributes.  */
@@ -4517,7 +4552,7 @@ dwarf2_add_field (struct field_info *fip
 	return;
 
       /* Get physical name.  */
-      physname = dwarf2_linkage_name (die, cu);
+      physname = (char *) dwarf2_physname (fieldname, die, cu);
 
       /* The name is already allocated along with this objfile, so we don't
 	 need to duplicate it for the type.  */
@@ -4679,7 +4714,7 @@ dwarf2_add_member_fn (struct field_info 
     return;
 
   /* Get the mangled name.  */
-  physname = dwarf2_linkage_name (die, cu);
+  physname = (char *) dwarf2_physname (fieldname, die, cu);
 
   /* Look up member function name in fieldlist.  */
   for (i = 0; i < fip->nfnfields; i++)
@@ -4986,14 +5021,18 @@ read_structure_type (struct die_info *di
       if (cu->language == language_cplus
 	  || cu->language == language_java)
 	{
-	  const char *new_prefix = determine_class_name (die, cu);
-	  TYPE_TAG_NAME (type) = (char *) new_prefix;
+	  TYPE_TAG_NAME (type) = (char *) dwarf2_full_name (name, die, cu);
+	  if (die->tag == DW_TAG_structure_type
+	      || die->tag == DW_TAG_class_type)
+	    TYPE_NAME (type) = TYPE_TAG_NAME (type);
 	}
       else
 	{
 	  /* The name is already allocated along with this objfile, so
 	     we don't need to duplicate it for the type.  */
-	  TYPE_TAG_NAME (type) = name;
+	  TYPE_TAG_NAME (type) = (char *) name;
+	  if (die->tag == DW_TAG_class_type)
+	    TYPE_NAME (type) = TYPE_TAG_NAME (type);
 	}
     }
 
@@ -5211,7 +5250,7 @@ read_enumeration_type (struct die_info *
   type = alloc_type (objfile);
 
   TYPE_CODE (type) = TYPE_CODE_ENUM;
-  name = dwarf2_full_name (die, cu);
+  name = dwarf2_full_name (NULL, die, cu);
   if (name != NULL)
     TYPE_TAG_NAME (type) = (char *) name;
 
@@ -5236,51 +5275,6 @@ read_enumeration_type (struct die_info *
   return set_die_type (die, type, cu);
 }
 
-/* Determine the name of the type represented by DIE, which should be
-   a named C++ or Java compound type.  Return the name in question,
-   allocated on the objfile obstack.  */
-
-static const char *
-determine_class_name (struct die_info *die, struct dwarf2_cu *cu)
-{
-  const char *new_prefix = NULL;
-
-  /* If we don't have namespace debug info, guess the name by trying
-     to demangle the names of members, just like we did in
-     guess_structure_name.  */
-  if (!processing_has_namespace_info)
-    {
-      struct die_info *child;
-
-      for (child = die->child;
-	   child != NULL && child->tag != 0;
-	   child = sibling_die (child))
-	{
-	  if (child->tag == DW_TAG_subprogram)
-	    {
-	      char *phys_prefix
-		= language_class_name_from_physname (cu->language_defn,
-						     dwarf2_linkage_name
-						     (child, cu));
-
-	      if (phys_prefix != NULL)
-		{
-		  new_prefix
-		    = obsavestring (phys_prefix, strlen (phys_prefix),
-				    &cu->objfile->objfile_obstack);
-		  xfree (phys_prefix);
-		  break;
-		}
-	    }
-	}
-    }
-
-  if (new_prefix == NULL)
-    new_prefix = dwarf2_full_name (die, cu);
-
-  return new_prefix;
-}
-
 /* Given a pointer to a die which begins an enumeration, process all
    the dies that define the members of the enumeration, and create the
    symbol for the enumeration type.
@@ -5936,7 +5930,7 @@ read_typedef (struct die_info *die, stru
   const char *name = NULL;
   struct type *this_type;
 
-  name = dwarf2_full_name (die, cu);
+  name = dwarf2_full_name (NULL, die, cu);
   this_type = init_type (TYPE_CODE_TYPEDEF, 0,
 			 TYPE_FLAG_TARGET_STUB, NULL, objfile);
   TYPE_NAME (this_type) = (char *) name;
@@ -6764,7 +6758,8 @@ read_partial_die (struct partial_die_inf
 	    }
 	  break;
 	case DW_AT_MIPS_linkage_name:
-	  part_die->name = DW_STRING (&attr);
+	  if (cu->language == language_ada)
+	    part_die->name = DW_STRING (&attr);
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
@@ -8329,13 +8324,11 @@ new_symbol (struct die_info *die, struct
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
-  if (die->tag != DW_TAG_namespace)
-    name = dwarf2_linkage_name (die, cu);
-  else
-    name = TYPE_NAME (type);
-
+  name = dwarf2_name (die, cu);
   if (name)
     {
+      const char *linkagename;
+
       sym = (struct symbol *) obstack_alloc (&objfile->objfile_obstack,
 					     sizeof (struct symbol));
       OBJSTAT (objfile, n_syms++);
@@ -8343,7 +8336,8 @@ new_symbol (struct die_info *die, struct
 
       /* Cache this symbol's name and the name's demangled form (if any).  */
       SYMBOL_LANGUAGE (sym) = cu->language;
-      SYMBOL_SET_NAMES (sym, name, strlen (name), 0, objfile);
+      linkagename = dwarf2_physname (name, die, cu);
+      SYMBOL_SET_NAMES (sym, linkagename, strlen (linkagename), 0, objfile);
 
       /* Default assumptions.
          Use the passed type or decode it from the die.  */
@@ -8568,7 +8562,8 @@ new_symbol (struct die_info *die, struct
 	  }
 	  break;
 	case DW_TAG_typedef:
-	  SYMBOL_LINKAGE_NAME (sym) = (char *) dwarf2_full_name (die, cu);
+	  SYMBOL_LINKAGE_NAME (sym)
+	    = (char *) dwarf2_full_name (name, die, cu);
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
 	  add_symbol_to_list (sym, cu->list_in_scope);
@@ -8580,7 +8575,8 @@ new_symbol (struct die_info *die, struct
 	  add_symbol_to_list (sym, cu->list_in_scope);
 	  break;
 	case DW_TAG_enumerator:
-	  SYMBOL_LINKAGE_NAME (sym) = (char *) dwarf2_full_name (die, cu);
+	  SYMBOL_LINKAGE_NAME (sym)
+	    = (char *) dwarf2_full_name (name, die, cu);
 	  attr = dwarf2_attr (die, DW_AT_const_value, cu);
 	  if (attr)
 	    {
@@ -8617,8 +8613,7 @@ new_symbol (struct die_info *die, struct
       /* For the benefit of old versions of GCC, check for anonymous
 	 namespaces based on the demangled name.  */
       if (!processing_has_namespace_info
-	  && cu->language == language_cplus
-	  && dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu) != NULL)
+	  && cu->language == language_cplus)
 	cp_scan_for_anonymous_namespaces (sym);
     }
   return (sym);
@@ -9070,19 +9065,6 @@ sibling_die (struct die_info *die)
   return die->sibling;
 }
 
-/* Get linkage name of a die, return NULL if not found.  */
-
-static char *
-dwarf2_linkage_name (struct die_info *die, struct dwarf2_cu *cu)
-{
-  struct attribute *attr;
-
-  attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
-  if (attr && DW_STRING (attr))
-    return DW_STRING (attr);
-  return dwarf2_name (die, cu);
-}
-
 /* Get name of a die, return NULL if not found.  */
 
 static char *