[patch] Follow DW_AT_linkage_name for methods #2

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

this is a repost of already approved:
	[rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]
	http://sourceware.org/ml/gdb-patches/2011-05/msg00579.html
	http://sourceware.org/ml/gdb-patches/2011-06/msg00012.html

But it waits on the libiberty/ patchset pre-requisite approval for GCC.
	[gcc patch 0/3] libiberty: New DMGL_RET_DROP
	http://sourceware.org/ml/gdb-patches/2011-06/msg00019.html

I want to highlight DMGL_VERBOSE is used for DWARF symbols as it is in use
also for ELF symbols since the physname patch which did:
@@ -498,7 +494,7 @@ symbol_find_demangled_name (struct gener
-        cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
+        cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI | DMGL_VERBOSE);


Thanks,
Jan


gdb/
2011-05-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (check_physname): New variable.
	(dwarf2_physname): Prefer DW_AT_linkage_name over dwarf2_compute_name.
	(show_check_physname): New function.
	(_initialize_dwarf2_read): Add `check-physname' for check_physname.

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

	* gdb.base/break-interp.exp (reach_1, test_ld): Allow also the prefix
	__GI_.
	* gdb.cp/psymtab-parameter.cc (func): Make it a template function.
	(f): New function.
	* gdb.cp/psymtab-parameter.exp (complete break 'func(): Rename to ...
	(complete p 'func<short>(): ... here.
	* gdb.dwarf2/dw2-linkage-name-trust-main.cc: New file.
	* gdb.dwarf2/dw2-linkage-name-trust.S: New file.
	* gdb.dwarf2/dw2-linkage-name-trust.exp: New file.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -119,6 +119,9 @@ _STATEMENT_PROLOGUE;
 /* When non-zero, dump DIEs after they are read in.  */
 static int dwarf2_die_debug = 0;
 
+/* When non-zero, cross-check physname against demangler.  */
+static int check_physname = 0;
+
 static int pagesize;
 
 /* When set, the file that we're processing is known to have debugging
@@ -5170,7 +5173,92 @@ 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, 1);
+  struct attribute *attr;
+  const char *retval, *mangled = NULL, *canon = NULL;
+  struct cleanup *back_to;
+  int need_copy = 1;
+
+  /* In this case dwarf2_compute_name is just a shortcut not building anything
+     on its own.  */
+  if (!die_needs_namespace (die, cu))
+    return dwarf2_compute_name (name, die, cu, 1);
+
+  back_to = make_cleanup (null_cleanup, NULL);
+
+  attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
+  if (!attr)
+    attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
+
+  /* DW_AT_linkage_name is missing in some cases - depend on what GDB
+     has computed.  */
+  if (attr && DW_STRING (attr))
+    {
+      char *demangled;
+
+      mangled = DW_STRING (attr);
+
+      /* Use DMGL_RET_DROP for C++ template functions to suppress their return
+	 type.  It is easier for GDB users to search for such functions as
+	 `name(params)' than `long name(params)'.  In such case the minimal
+	 symbol names do not match the full symbol names but for template
+	 functions there is never a need to look up their definition from their
+	 declaration so the only disadvantage remains the minimal symbol
+	 variant `long name(params)' does not have the proper inferior type.
+	 */
+
+      demangled = cplus_demangle (mangled, (DMGL_PARAMS | DMGL_ANSI
+					    | DMGL_VERBOSE
+					    | (cu->language == language_java
+					       ? DMGL_JAVA | DMGL_RET_POSTFIX
+					       : DMGL_RET_DROP)));
+      if (demangled)
+	{
+	  make_cleanup (xfree, demangled);
+	  canon = demangled;
+	}
+      else
+	{
+	  canon = mangled;
+	  need_copy = 0;
+	}
+    }
+
+  if (canon == NULL || check_physname)
+    {
+      const char *physname = dwarf2_compute_name (name, die, cu, 1);
+
+      if (canon != NULL && 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 = canon;
+	}
+      else
+	{
+	  retval = physname;
+	  need_copy = 0;
+	}
+    }
+  else
+    retval = canon;
+
+  if (need_copy)
+    retval = obsavestring (retval, strlen (retval),
+			   &cu->objfile->objfile_obstack);
+
+  do_cleanups (back_to);
+  return retval;
 }
 
 /* Read the import statement specified by the given die and record it.  */
@@ -16188,6 +16276,15 @@ show_dwarf2_always_disassemble (struct ui_file *file, int from_tty,
 		    value);
 }
 
+static void
+show_check_physname (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("Whether to check \"physname\" is %s.\n"),
+		    value);
+}
+
 void _initialize_dwarf2_read (void);
 
 void
@@ -16243,6 +16340,14 @@ The value is the maximum depth to print."),
 			    NULL,
 			    &setdebuglist, &showdebuglist);
 
+  add_setshow_boolean_cmd ("check-physname", no_class, &check_physname, _("\
+Set cross-checking of \"physname\" code against demangler."), _("\
+Show cross-checking of \"physname\" code against demangler."), _("\
+When enabled, GDB's internal \"physname\" code is checked against\n\
+the demangler."),
+			   NULL, show_check_physname,
+			   &setdebuglist, &showdebuglist);
+
   c = add_cmd ("gdb-index", class_files, save_gdb_index_command,
 	       _("\
 Save a gdb-index file.\n\
--- 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/gdb/testsuite/gdb.cp/psymtab-parameter.cc
+++ b/gdb/testsuite/gdb.cp/psymtab-parameter.cc
@@ -16,7 +16,14 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
    */
 
-typedef int typedefed;
-void func (typedefed param)
+template <typename T>
+long
+func ()
 {
 }
+
+void
+f ()
+{
+  func<short> ();
+}
--- a/gdb/testsuite/gdb.cp/psymtab-parameter.exp
+++ b/gdb/testsuite/gdb.cp/psymtab-parameter.exp
@@ -35,5 +35,6 @@ gdb_test_no_output "set language c++"
 # XFAIL than FAIL here.  For example -readnow breaks it.
 gdb_test_no_output "maintenance info symtabs"
 
-# GDB has shown only the `func(int)' entry before.
-gdb_test "complete break 'func(" "break 'func\\(int\\)\r\nbreak 'func\\(typedefed\\)"
+# GDB has shown only the `long func<short>()' ELF symbol before, not the DWARF
+# symbol
+gdb_test "complete p 'func<short>(" "p 'func<short>\\(\\)"
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-linkage-name-trust-main.cc
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* class C
+   {
+   public:
+     static int f ();
+   };  */
+
+asm (".globl cu_text_start");
+asm ("cu_text_start:");
+
+int
+f (void)
+{
+  return 31173;
+}
+
+int
+main (void)
+{
+  f ();
+  return 0;
+}
+
+asm (".globl cu_text_end");
+asm ("cu_text_end:");
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-linkage-name-trust.S
@@ -0,0 +1,134 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Debug information */
+
+	.data
+	.globl	c
+c:	.4byte	0
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.4byte	cu_text_start			/* DW_AT_low_pc */
+	.4byte	cu_text_end			/* DW_AT_high_pc */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	4				/* DW_AT_language (DW_LANG_C_plus_plus) */
+
+.Ltype_class:
+	.uleb128	3			/* Abbrev: DW_TAG_class_type */
+	.ascii		"C\0"			/* DW_AT_name */
+
+	.uleb128	5			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"membername\0"		/* DW_AT_name */
+	.ascii		"f\0"			/* DW_AT_MIPS_linkage_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+
+	.byte		0			/* End of children of DW_TAG_class_type */
+
+	.uleb128	4			/* Abbrev: DW_TAG_variable */
+	.ascii		"c\0"			/* DW_AT_name */
+	.4byte		.Ltype_class-.Lcu1_begin /* DW_AT_type */
+	.byte		1			/* DW_AT_external */
+
+.Ltype_int:
+	.uleb128	2			/* Abbrev: DW_TAG_base_type */
+	.ascii		"int\0"			/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		5			/* DW_AT_encoding */
+
+	.byte		0			/* End of children of CU */
+
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x2			/* DW_TAG_class_type */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	4			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	5			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x2007			/* DW_AT_MIPS_linkage_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-linkage-name-trust.exp
@@ -0,0 +1,55 @@
+# Copyright 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can call C++ functions whose parameters or return values have
+# type containing a static member of the same type.
+
+# Still no C++ compiler is used.
+if { [skip_cplus_tests] } { continue }
+
+load_lib dwarf.exp
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+set testfile "dw2-linkage-name-trust"
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+if { [gdb_compile ${srcdir}/${subdir}/${testfile}-main.cc "${objdir}/${subdir}/${testfile}-main.o" object {c++ debug}] != ""
+     || [gdb_compile "${srcdir}/${subdir}/${testfile}.S" "${objdir}/${subdir}/${testfile}.o" object {}] != ""
+     || [gdb_compile "${objdir}/${subdir}/${testfile}-main.o ${objdir}/${subdir}/${testfile}.o" "${binfile}" executable {c++}] != "" } {
+    return -1
+}
+
+clean_restart $executable
+
+if ![runto_main] then {
+    return -1
+}
+
+# main is not provided by DWARF.
+gdb_test_no_output "set language c++"
+
+# There are no mangled names in DWARF to suggest the v3 ABI.
+gdb_test_no_output "set cp-abi gnu-v3"
+
+# GDB cannot resolve external member function for which only ELF (and not
+# DWARF) symbol is available.  Therefore the function `f' must have DWARF which
+# confuses it a bit.
+
+gdb_test "p c.membername" " = {.*} 0x\[0-9a-f\]+ <f\\(\\)>"
+gdb_breakpoint "C::membername"
+gdb_test "p c.membername ()" "\r\nBreakpoint \[0-9\]+, .*"