Re: [rfc] physname cross-check #2

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Tue, 17 May 2011 20:15:18 +0200, Keith Seitz wrote:
> For me, what really matters is what is best for users. Is reverting
> dwarf2_physname better or worse than DW_AT_MIPS_linkage_name?

This patch has no regressions on {x86_64,x86_64-m32,i686}-fedora15-linux-gnu.
And it prefers DW_AT_linkage_name when available to solve the DWARF<->ELF
symbols discrepancies.

DMGL_VERBOSE was suggested by Keith.

The libiberty/ part needs more review and a post to GCC but it seems as
a correct fix idea anyway.


Thanks,
Jan


--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5142,7 +5142,77 @@ dwarf2_full_name (char *name, struct die_info *die, struct dwarf2_cu *cu)
 static const char *
 dwarf2_physname (char *name, struct die_info *die, struct dwarf2_cu *cu)
 {
-  return dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
+  const char *physname = dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
+  struct attribute *attr;
+  const char *mangled, *retval;
+  char *demangled, *canon;
+  struct cleanup *back_to;
+
+  /* Do not check PHYSNAME if it never will be needed by GDB.  GDB does not
+     need to support something it has no use for.  */
+  if (!die_needs_namespace (die, cu))
+    return physname;
+
+  attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
+  if (!attr)
+    attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
+  if (!attr || !DW_STRING (attr))
+    {
+      /* DW_AT_linkage_name is missing in some cases - depend on what GDB has
+	 computed.  */
+
+      return physname;
+    }
+  mangled = DW_STRING (attr);
+
+  /* As both DW_AT_linkage_name (MANGLED) and computed PHYSNAME are present
+     cross-check them.  */
+
+  back_to = make_cleanup (null_cleanup, 0);
+
+  demangled = cplus_demangle (mangled,
+			      (DMGL_PARAMS | DMGL_VERBOSE | DMGL_RET_POSTFIX
+			       | DMGL_ANSI | (cu->language == language_java
+					      ? DMGL_JAVA : 0)));
+  if (demangled)
+    make_cleanup (xfree, demangled);
+  else
+    demangled = (char *) mangled;
+
+  if (cu->language == language_cplus)
+    {
+      canon = cp_canonicalize_string (demangled);
+      if (canon != NULL)
+	make_cleanup (xfree, canon);
+      else
+	canon = demangled;
+    }
+  else
+    canon = demangled;
+
+  if (strcmp (physname, canon) != 0)
+    {
+      /* It may not mean a bug in GDB.  The compiler could also compute
+	 DW_AT_linkage_name incorrectly.  But in such case GDB would need to be
+	 bug-to-bug compatible.  */
+
+      complaint (&symfile_complaints,
+		 _("Computed physname <%s> does not match demangled <%s> "
+		   "(from linkage <%s>) - DIE at 0x%x [in module %s]\n"),
+		 physname, canon, mangled, die->offset, cu->objfile->name);
+
+      /* Prefer DW_AT_linkage_name (in the CANON form) - when it is available
+	 here - over computed PHYSNAME.  It is safer against both buggy GDB and
+	 buggy compilers.  */
+
+      retval = obsavestring (canon, strlen (canon),
+			     &cu->objfile->objfile_obstack);
+    }
+  else
+    retval = physname;
+
+  do_cleanups (back_to);
+  return retval;
 }
 
 static const char *
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -140,14 +140,14 @@ proc reach_1 {func command displacement} {
 	    }
 	    exp_continue
 	}
-	-re "Breakpoint \[0-9\]+, \\.?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
+	-re "Breakpoint \[0-9\]+, \\.?(__GI_)?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
 	    if {$func == "_dl_debug_state"} {
 		fail $test
 	    } else {
 		pass $test
 	    }
 	}
-	-re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?$func \\(\\).*\r\n$gdb_prompt $" {
+	-re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?(__GI_)?$func \\(\\).*\r\n$gdb_prompt $" {
 	    if {$func == "_dl_debug_state"} {
 		fail $test
 	    } else {
@@ -403,7 +403,7 @@ proc test_ld {file ifmain trynosym displacement} {
 
     reach "_dl_debug_state" "run" $displacement
 
-    gdb_test "bt" "#0 +\[^\r\n\]*\\m_dl_debug_state\\M.*" "dl bt"
+    gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?_dl_debug_state\\M.*" "dl bt"
 
     if $ifmain {
 	reach "main" continue "NONE"
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -45,7 +45,8 @@ extern "C" {
 #define DMGL_VERBOSE	 (1 << 3)	/* Include implementation details.  */
 #define DMGL_TYPES	 (1 << 4)	/* Also try to demangle type encodings.  */
 #define DMGL_RET_POSTFIX (1 << 5)       /* Print function return types (when
-                                           present) after function signature */
+                                           present) after function signature.
+                                           It applies only for toplevel types.  */
 
 #define DMGL_AUTO	 (1 << 8)
 #define DMGL_GNU	 (1 << 9)
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3557,6 +3557,8 @@ static void
 d_print_comp (struct d_print_info *dpi,
               const struct demangle_component *dc)
 {
+  int ret_postfix;
+
   if (dc == NULL)
     {
       d_print_error (dpi);
@@ -3565,6 +3567,9 @@ d_print_comp (struct d_print_info *dpi,
   if (d_print_saw_error (dpi))
     return;
 
+  ret_postfix = dpi->options & DMGL_RET_POSTFIX;
+  dpi->options &= ~DMGL_RET_POSTFIX;
+
   switch (dc->type)
     {
     case DEMANGLE_COMPONENT_NAME:
@@ -3576,6 +3581,7 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_QUAL_NAME:
     case DEMANGLE_COMPONENT_LOCAL_NAME:
+      dpi->options |= ret_postfix;
       d_print_comp (dpi, d_left (dc));
       if ((dpi->options & DMGL_JAVA) == 0)
 	d_append_string (dpi, "::");
@@ -3597,6 +3603,7 @@ d_print_comp (struct d_print_info *dpi,
 	   any CV-qualifiers, which apply to the this parameter.  */
 	hold_modifiers = dpi->modifiers;
 	dpi->modifiers = 0;
+	dpi->options |= ret_postfix;
 	i = 0;
 	typed_name = d_left (dc);
 	while (typed_name != NULL)
@@ -3919,7 +3926,7 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_FUNCTION_TYPE:
       {
-	if ((dpi->options & DMGL_RET_POSTFIX) != 0)
+	if (ret_postfix)
 	  d_print_function_type (dpi, dc, dpi->modifiers);
 
 	/* Print return type if present */
@@ -3944,11 +3951,11 @@ d_print_comp (struct d_print_info *dpi,
 
 	    /* In standard prefix notation, there is a space between the
 	       return type and the function signature.  */
-	    if ((dpi->options & DMGL_RET_POSTFIX) == 0)
+	    if (!ret_postfix)
 	      d_append_char (dpi, ' ');
 	  }
 
-	if ((dpi->options & DMGL_RET_POSTFIX) == 0) 
+	if (!ret_postfix)
 	  d_print_function_type (dpi, dc, dpi->modifiers);
 
 	return;