[rfc] physname cross-check [Re: [RFA] Typedef'd method parameters [0/4]]

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

Hi Keith,

On Thu, 12 May 2011 23:27:52 +0200, Keith Seitz wrote:
> I'm sending an updated set of all of the patches just in case
> something got foobar'd along the way.

I wrote a cross-check of what GDB thinks is the physname (which is in
demangled canonical form) vs. what GCC thinkgs is the physname (which needs to
be converted from mangled form and canonicalized).

It reports for me 34524 unique failures on libwebkit.so.debug.  (Sure such
count is caused only by a few physname computation bugs.)

Therefore I would propose a sinful idea to temporarily just use
DW_AT_linkage_name if it is available to ever release gdb-7.3 and make
DW_AT_linkage_name-less GDB a feature for gdb-7.4.  After all such cross-check
should exist anyway for verifying both GCC and GDB bugs this way.

This patch preferring DW_AT_linkage_name fixes for example this regression:

cat >1.h <<EOH
struct x {};
class C { public: void m (x *xp); };
EOH
cat >1.C <<EOH
#include "1.h"
C c;
int main () { c.m(0); }
EOH
cat >1b.C <<EOH
#include "1.h"
void C::m (x *xp) {}
EOH

# gcc-c++-4.6.0-7.fc15.x86_64
g++ -c -o 1b.o -Wall 1b.C; g++ -o 1 1.C 1b.o -Wall -g; ./gdb -q -nx ./1 -ex 'set complaints 10' -ex 'p main' -ex 'set complaints 0' -ex start -ex 'p c.m' -ex c -ex q
During symbol reading, Computed physname <C::m(struct x *)> does not match demangled <C::m(x*)> (from linkage <_ZN1C1mEP1x>) - DIE at 0x3d [in module .../1].

pre-phyname:       $2 = {void (C *, x *)} 0x4004f0 <C::m(x*)>
FSF GDB HEAD:      $2 = {void (C * const, x *)} 0x4004f0 <C::m(x*)>
HEAD + your patch: Cannot take address of method m.
your+this patches: $2 = {void (C * const, x *)} 0x4004f0 <C::m(x*)>

This is a regression.

Just it has some other existing testsuite regressions:
	gdb.cp/bs15503.exp gdb.cp/cp-relocate.exp gdb.cp/cpexprs.exp
	gdb.java/jmisc.exp gdb.java/jprint.exp

From a quick loook they seem to me like bugs in GDB expecting the incompatibly
computed physname but I did not try to fix it yet.

What do you think?


Thanks,
Jan


gdb/
2011-05-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (dwarf2_physname): New variables physname, attr,
	mangled, retval, demangled, canon and back_to.  Cross-check
	DW_AT_linkage_name and PHYS_NAME if both available, return
	DW_AT_linkage_name in such case.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5142,7 +5142,74 @@ 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_ANSI);
+  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]"),
+		 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 *