[PATCH 00/10] Fix two name-related bugs in DWARF reader

[PATCH 00/10] Fix two name-related bugs in DWARF reader

[Tom Tromey]

I started this series by trying to fix PR rust/25025.  This showed
that the Rust compiler sometimes emits two mangled forms that demangle
to the same thing.

In the end I could maybe have fixed this in a direct way (see patch #9
for the details), but while debugging I went on a detour into the
physname code and came up with this series.

This changes the DWARF reader to avoid demangling when constructing
partial symbols.  This obsoletes my previous patch to do the same
thing; I think this approach is cleaner.

It also attempts to fix the longstanding physname bug, PR 12707.
After this series, the demangled form is now stored on the symbol
again.

Regression tested by the buildbot.

Tom

[PATCH 01/10] Convert symbol_set_demangled_name to a method

[Tom Tromey]

This changes symbol_set_demangled_name to be a method on
general_symbol_info, and updates the users.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* symtab.h (struct general_symbol_info) <set_demangled_name>: New
	method.
	(symbol_set_demangled_name): Don't declare.
	* symtab.c (general_symbol_info::set_demangled_name): Rename from
	symbol_set_demangled_name.
	(general_symbol_info::set_language)
	(general_symbol_info::compute_and_set_names): Update.
	* minsyms.c (minimal_symbol_reader::install): Update.
	* dwarf2/read.c (new_symbol): Update.
---
 gdb/ChangeLog     | 12 ++++++++++++
 gdb/dwarf2/read.c |  4 +---
 gdb/minsyms.c     |  5 ++---
 gdb/symtab.c      | 27 ++++++++++++---------------
 gdb/symtab.h      |  9 +++++----
 5 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 0e879e08a0c..69ad352b6a7 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20573,9 +20573,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	 between gfortran, iFort etc.  */
       if (cu->language == language_fortran
           && symbol_get_demangled_name (sym) == NULL)
-	symbol_set_demangled_name (sym,
-				   dwarf2_full_name (name, die, cu),
-	                           NULL);
+	sym->set_demangled_name (dwarf2_full_name (name, die, cu), NULL);
 
       /* Default assumptions.
          Use the passed type or decode it from the die.  */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index e238355dc11..0bccf44f8ee 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1418,9 +1418,8 @@ minimal_symbol_reader::install ()
 		   /* This will be freed later, by compute_and_set_names.  */
 		   char *demangled_name
 		     = symbol_find_demangled_name (msym, msym->linkage_name ());
-		   symbol_set_demangled_name
-		     (msym, demangled_name,
-		      &m_objfile->per_bfd->storage_obstack);
+		   msym->set_demangled_name
+		     (demangled_name, &m_objfile->per_bfd->storage_obstack);
 		   msym->name_set = 1;
 		 }
 	       /* This mangled_name_hash computation has to be outside of
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f300d759e03..84ad17ee50e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -667,29 +667,27 @@ gdb_mangle_name (struct type *type, int method_id, int signature_id)
   return (mangled_name);
 }
 
-/* Set the demangled name of GSYMBOL to NAME.  NAME must be already
-   correctly allocated.  */
+/* See symtab.h.  */
 
 void
-symbol_set_demangled_name (struct general_symbol_info *gsymbol,
-                           const char *name,
-                           struct obstack *obstack)
+general_symbol_info::set_demangled_name (const char *name,
+					 struct obstack *obstack)
 {
-  if (gsymbol->language () == language_ada)
+  if (language () == language_ada)
     {
       if (name == NULL)
 	{
-	  gsymbol->ada_mangled = 0;
-	  gsymbol->language_specific.obstack = obstack;
+	  ada_mangled = 0;
+	  language_specific.obstack = obstack;
 	}
       else
 	{
-	  gsymbol->ada_mangled = 1;
-	  gsymbol->language_specific.demangled_name = name;
+	  ada_mangled = 1;
+	  language_specific.demangled_name = name;
 	}
     }
   else
-    gsymbol->language_specific.demangled_name = name;
+    language_specific.demangled_name = name;
 }
 
 /* Return the demangled name of GSYMBOL.  */
@@ -722,7 +720,7 @@ general_symbol_info::set_language (enum language language,
       || language == language_objc
       || language == language_fortran)
     {
-      symbol_set_demangled_name (this, NULL, obstack);
+      set_demangled_name (NULL, obstack);
     }
   else if (language == language_ada)
     {
@@ -872,7 +870,7 @@ general_symbol_info::compute_and_set_names (gdb::string_view linkage_name,
 	m_name = obstack_strndup (&per_bfd->storage_obstack,
 				  linkage_name.data (),
 				  linkage_name.length ());
-      symbol_set_demangled_name (this, NULL, &per_bfd->storage_obstack);
+      set_demangled_name (NULL, &per_bfd->storage_obstack);
 
       return;
     }
@@ -962,8 +960,7 @@ general_symbol_info::compute_and_set_names (gdb::string_view linkage_name,
     m_language = (*slot)->language;
 
   m_name = (*slot)->mangled.data ();
-  symbol_set_demangled_name (this, (*slot)->demangled.get (),
-			     &per_bfd->storage_obstack);
+  set_demangled_name ((*slot)->demangled.get (), &per_bfd->storage_obstack);
 }
 
 /* See symtab.h.  */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 18be5d51b85..83938d064fe 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -425,6 +425,11 @@ struct general_symbol_info
   void set_linkage_name (const char *linkage_name)
   { m_name = linkage_name; }
 
+  /* Set the demangled name of this symbol to NAME.  NAME must be
+     already correctly allocated.  If the symbol's language is Ada,
+     then the name is ignored and the obstack is set.  */
+  void set_demangled_name (const char *name, struct obstack *obstack);
+
   enum language language () const
   { return m_language; }
 
@@ -508,10 +513,6 @@ struct general_symbol_info
   short section;
 };
 
-extern void symbol_set_demangled_name (struct general_symbol_info *,
-				       const char *,
-                                       struct obstack *);
-
 extern const char *symbol_get_demangled_name
   (const struct general_symbol_info *);
 
-- 
2.17.2

[PATCH 02/10] Move the rust "{" hack

[Tom Tromey]

The DWARF reader has a special case to work around a bug in some
versions of the Rust compiler -- it ignores mangled names that contain
a "{" character.

I noticed that this check should probably be in dw2_linkage_name
rather than only in dwarf2_physname.  The former is called in some
cases that the latter is not.

Also, I noticed that this work is not done for the partial DIE reader,
so this patch adds the check there as well.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (dw2_linkage_name): Move Rust "{" hack here...
	(dwarf2_physname): ... from here.
	(partial_die_info::read): Add Rust "{" hack.
---
 gdb/ChangeLog     |  6 ++++++
 gdb/dwarf2/read.c | 17 +++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 69ad352b6a7..d5cdd49a086 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -10020,6 +10020,12 @@ dw2_linkage_name (struct die_info *die, struct dwarf2_cu *cu)
   if (linkage_name == NULL)
     linkage_name = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
 
+  /* rustc emits invalid values for DW_AT_linkage_name.  Ignore these.
+     See https://github.com/rust-lang/rust/issues/32925.  */
+  if (cu->language == language_rust && linkage_name != NULL
+      && strchr (linkage_name, '{') != NULL)
+    linkage_name = NULL;
+
   return linkage_name;
 }
 
@@ -10296,12 +10302,6 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
 
   mangled = dw2_linkage_name (die, cu);
 
-  /* rustc emits invalid values for DW_AT_linkage_name.  Ignore these.
-     See https://github.com/rust-lang/rust/issues/32925.  */
-  if (cu->language == language_rust && mangled != NULL
-      && strchr (mangled, '{') != NULL)
-    mangled = NULL;
-
   /* DW_AT_linkage_name is missing in some cases - depend on what GDB
      has computed.  */
   gdb::unique_xmalloc_ptr<char> demangled;
@@ -18032,6 +18032,11 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	     assume they will be the same, and we only store the last
 	     one we see.  */
 	  linkage_name = DW_STRING (&attr);
+	  /* rustc emits invalid values for DW_AT_linkage_name.  Ignore these.
+	     See https://github.com/rust-lang/rust/issues/32925.  */
+	  if (cu->language == language_rust && linkage_name != NULL
+	      && strchr (linkage_name, '{') != NULL)
+	    linkage_name = NULL;
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
-- 
2.17.2

[PATCH 03/10] Fix two latent Rust bugs

[Tom Tromey]

Two methods on general_symbol_info did not handle the language_rust
case.  I don't think these problems can be noticed with the current
code (which is why the bugs went unnoticed), but a future patch will
change this.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* symtab.c (general_symbol_info::natural_name)
	(general_symbol_info::demangled_name): Check for language_rust.
---
 gdb/ChangeLog | 5 +++++
 gdb/symtab.c  | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 84ad17ee50e..bdbfe8e60d3 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -975,6 +975,7 @@ general_symbol_info::natural_name () const
     case language_go:
     case language_objc:
     case language_fortran:
+    case language_rust:
       if (symbol_get_demangled_name (this) != NULL)
 	return symbol_get_demangled_name (this);
       break;
@@ -1000,6 +1001,7 @@ general_symbol_info::demangled_name () const
     case language_go:
     case language_objc:
     case language_fortran:
+    case language_rust:
       dem_name = symbol_get_demangled_name (this);
       break;
     case language_ada:
-- 
2.17.2

[PATCH 04/10] Add attribute::value_as_string method

[Tom Tromey]

The full DIE reader checks that an attribute has a "string" form in
some spots, but the partial DIE reader does not.  This patch brings
the two readers in sync for one specific case, namely when examining
the linkage name.  This avoids regressions in an existing DWARF test
case.

A full fix for this problem would be preferable.  An accessor like
DW_STRING should always check the form.  However, I haven't attempted
that in this series.

Also the fact that the partial and full readers can disagree like this
is a design flaw.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (partial_die_info::read) <case
	DW_AT_linkage_name>: Use value_as_string.
	(dwarf2_string_attr): Use value_as_string.
	* dwarf2/attribute.h (struct attribute) <value_as_string>: Declare
	method.
	* dwarf2/attribute.c (attribute::value_as_string): New method.
---
 gdb/ChangeLog          |  9 +++++++++
 gdb/dwarf2/attribute.c | 18 ++++++++++++++++++
 gdb/dwarf2/attribute.h |  4 ++++
 gdb/dwarf2/read.c      | 15 +++------------
 4 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 6efff3e2c0a..273f17a2cb0 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -59,6 +59,24 @@ attribute::value_as_address () const
 
 /* See attribute.h.  */
 
+const char *
+attribute::value_as_string () const
+{
+  if (form == DW_FORM_strp || form == DW_FORM_line_strp
+      || form == DW_FORM_string
+      || form == DW_FORM_strx
+      || form == DW_FORM_strx1
+      || form == DW_FORM_strx2
+      || form == DW_FORM_strx3
+      || form == DW_FORM_strx4
+      || form == DW_FORM_GNU_str_index
+      || form == DW_FORM_GNU_strp_alt)
+    return DW_STRING (this);
+  return nullptr;
+}
+
+/* See attribute.h.  */
+
 bool
 attribute::form_is_block () const
 {
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index c2602310715..e697e6d5fc8 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -45,6 +45,10 @@ struct attribute
      attribute's form into account.  */
   CORE_ADDR value_as_address () const;
 
+  /* If the attribute has a string form, return the string value;
+     otherwise return NULL.  */
+  const char *value_as_string () const;
+
   /* Return non-zero if ATTR's value is a section offset --- classes
      lineptr, loclistptr, macptr or rangelistptr --- or zero, otherwise.
      You may use DW_UNSND (attr) to retrieve such offsets.
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d5cdd49a086..a48552a4c6e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18031,7 +18031,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	  /* Note that both forms of linkage name might appear.  We
 	     assume they will be the same, and we only store the last
 	     one we see.  */
-	  linkage_name = DW_STRING (&attr);
+	  linkage_name = attr.value_as_string ();
 	  /* rustc emits invalid values for DW_AT_linkage_name.  Ignore these.
 	     See https://github.com/rust-lang/rust/issues/32925.  */
 	  if (cu->language == language_rust && linkage_name != NULL
@@ -19200,17 +19200,8 @@ dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *c
 
   if (attr != NULL)
     {
-      if (attr->form == DW_FORM_strp || attr->form == DW_FORM_line_strp
-	  || attr->form == DW_FORM_string
-	  || attr->form == DW_FORM_strx
-	  || attr->form == DW_FORM_strx1
-	  || attr->form == DW_FORM_strx2
-	  || attr->form == DW_FORM_strx3
-	  || attr->form == DW_FORM_strx4
-	  || attr->form == DW_FORM_GNU_str_index
-	  || attr->form == DW_FORM_GNU_strp_alt)
-	str = DW_STRING (attr);
-      else
+      str = attr->value_as_string ();
+      if (str == nullptr)
         complaint (_("string type expected for attribute %s for "
 		     "DIE at %s in module %s"),
 		   dwarf_attr_name (name), sect_offset_str (die->sect_off),
-- 
2.17.2

[PATCH 05/10] Introduce new add_psymbol_to_list overload

[Tom Tromey]

This adds a new overload of add_psymbol_to_list.  This one takes an
already constructed psymbol and adds it to the bcache and the
appropriate list.

This seemed cleaner than continuing to add parameters to the existing
add_psymbol_to_list, and is more in line with how full symbols are
constructed.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* psymtab.c (add_psymbol_to_bcache): Simplify calling convention.
	(add_psymbol_to_list): New overload.  Make old overload call new
	one.
	* psympriv.h (add_psymbol_to_list): New overload.
---
 gdb/ChangeLog  |  7 +++++++
 gdb/psympriv.h |  8 ++++++++
 gdb/psymtab.c  | 53 +++++++++++++++++++++++++++-----------------------
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index 0effedc4ec2..cf3f43f0e01 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -414,6 +414,14 @@ extern void add_psymbol_to_list (gdb::string_view name,
 				 enum language language,
 				 struct objfile *objfile);
 
+/* Add a symbol to the partial symbol table of OBJFILE.  The psymbol
+   must be fully constructed, and the names must be set and intern'd
+   as appropriate.  */
+
+extern void add_psymbol_to_list (const partial_symbol &psym,
+				 psymbol_placement where,
+				 struct objfile *objfile);
+
 /* Initialize storage for partial symbols.  If partial symbol storage
    has already been initialized, this does nothing.  TOTAL_SYMBOLS is
    an estimate of how many symbols there will be.  */
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 8aa9c6e87bb..36321856368 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1588,24 +1588,10 @@ psymbol_compare (const void *addr1, const void *addr2, int length)
    different domain (or address) is possible and correct.  */
 
 static struct partial_symbol *
-add_psymbol_to_bcache (gdb::string_view name, bool copy_name,
-		       domain_enum domain,
-		       enum address_class theclass,
-		       short section,
-		       CORE_ADDR coreaddr,
-		       enum language language, struct objfile *objfile,
+add_psymbol_to_bcache (const partial_symbol &psymbol,
+		       struct objfile *objfile,
 		       int *added)
 {
-  struct partial_symbol psymbol;
-  memset (&psymbol, 0, sizeof (psymbol));
-
-  psymbol.set_unrelocated_address (coreaddr);
-  psymbol.ginfo.section = section;
-  psymbol.domain = domain;
-  psymbol.aclass = theclass;
-  psymbol.ginfo.set_language (language, objfile->partial_symtabs->obstack ());
-  psymbol.ginfo.compute_and_set_names (name, copy_name, objfile->per_bfd);
-
   /* Stash the partial symbol away in the cache.  */
   return ((struct partial_symbol *)
 	  objfile->partial_symtabs->psymbol_cache.insert
@@ -1626,21 +1612,16 @@ append_psymbol_to_list (std::vector<partial_symbol *> *list,
 /* See psympriv.h.  */
 
 void
-add_psymbol_to_list (gdb::string_view name, bool copy_name,
-		     domain_enum domain,
-		     enum address_class theclass,
-		     short section,
+add_psymbol_to_list (const partial_symbol &psymbol,
 		     psymbol_placement where,
-		     CORE_ADDR coreaddr,
-		     enum language language, struct objfile *objfile)
+		     struct objfile *objfile)
 {
   struct partial_symbol *psym;
 
   int added;
 
   /* Stash the partial symbol away in the cache.  */
-  psym = add_psymbol_to_bcache (name, copy_name, domain, theclass,
-				section, coreaddr, language, objfile, &added);
+  psym = add_psymbol_to_bcache (psymbol, objfile, &added);
 
   /* Do not duplicate global partial symbols.  */
   if (where == psymbol_placement::GLOBAL && !added)
@@ -1656,6 +1637,30 @@ add_psymbol_to_list (gdb::string_view name, bool copy_name,
 
 /* See psympriv.h.  */
 
+void
+add_psymbol_to_list (gdb::string_view name, bool copy_name,
+		     domain_enum domain,
+		     enum address_class theclass,
+		     short section,
+		     psymbol_placement where,
+		     CORE_ADDR coreaddr,
+		     enum language language, struct objfile *objfile)
+{
+  struct partial_symbol psymbol;
+  memset (&psymbol, 0, sizeof (psymbol));
+
+  psymbol.set_unrelocated_address (coreaddr);
+  psymbol.ginfo.section = section;
+  psymbol.domain = domain;
+  psymbol.aclass = theclass;
+  psymbol.ginfo.set_language (language, objfile->partial_symtabs->obstack ());
+  psymbol.ginfo.compute_and_set_names (name, copy_name, objfile->per_bfd);
+
+  add_psymbol_to_list (psymbol, where, objfile);
+}
+
+/* See psympriv.h.  */
+
 void
 init_psymbol_list (struct objfile *objfile, int total_symbols)
 {
-- 
2.17.2

[PATCH 06/10] Use the new add_psymbol_to_list overload

[Tom Tromey]

This changes the DWARF reader to use the new add_psymbol_to_list
overload.  There should be no visible changes due to this patch.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (add_partial_symbol): Use new add_psymbol_to_list
	overload.
---
 gdb/ChangeLog     |   5 ++
 gdb/dwarf2/read.c | 127 +++++++++++++++++++++++-----------------------
 2 files changed, 68 insertions(+), 64 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a48552a4c6e..c3723684386 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -8211,6 +8211,15 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
   if (actual_name == NULL)
     actual_name = pdi->name;
 
+  partial_symbol psymbol;
+  memset (&psymbol, 0, sizeof (psymbol));
+  psymbol.ginfo.set_language (cu->language, &objfile->objfile_obstack);
+  psymbol.ginfo.section = -1;
+
+  /* The code below indicates that the psymbol should be installed by
+     setting this.  */
+  gdb::optional<psymbol_placement> where;
+
   switch (pdi->tag)
     {
     case DW_TAG_inlined_subroutine:
@@ -8227,34 +8236,25 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
              But in Ada and Fortran, we want to be able to access nested
              procedures globally.  So all Ada and Fortran subprograms are
              stored in the global scope.  */
-	  add_psymbol_to_list (actual_name,
-			       built_actual_name != NULL,
-			       VAR_DOMAIN, LOC_BLOCK,
-			       SECT_OFF_TEXT (objfile),
-			       psymbol_placement::GLOBAL,
-			       addr,
-			       cu->language, objfile);
+	  where = psymbol_placement::GLOBAL;
 	}
       else
-	{
-	  add_psymbol_to_list (actual_name,
-			       built_actual_name != NULL,
-			       VAR_DOMAIN, LOC_BLOCK,
-			       SECT_OFF_TEXT (objfile),
-			       psymbol_placement::STATIC,
-			       addr, cu->language, objfile);
-	}
+	where = psymbol_placement::STATIC;
+
+      psymbol.domain = VAR_DOMAIN;
+      psymbol.aclass = LOC_BLOCK;
+      psymbol.ginfo.section = SECT_OFF_TEXT (objfile);
+      psymbol.ginfo.value.address = addr;
 
       if (pdi->main_subprogram && actual_name != NULL)
 	set_objfile_main_name (objfile, actual_name, cu->language);
       break;
     case DW_TAG_constant:
-      add_psymbol_to_list (actual_name,
-			   built_actual_name != NULL, VAR_DOMAIN, LOC_STATIC,
-			   -1, (pdi->is_external
-				? psymbol_placement::GLOBAL
-				: psymbol_placement::STATIC),
-			   0, cu->language, objfile);
+      psymbol.domain = VAR_DOMAIN;
+      psymbol.aclass = LOC_STATIC;
+      where = (pdi->is_external
+	       ? psymbol_placement::GLOBAL
+	       : psymbol_placement::STATIC);
       break;
     case DW_TAG_variable:
       if (pdi->d.locdesc)
@@ -8285,12 +8285,13 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 	     table building.  */
 
 	  if (pdi->d.locdesc || pdi->has_type)
-	    add_psymbol_to_list (actual_name,
-				 built_actual_name != NULL,
-				 VAR_DOMAIN, LOC_STATIC,
-				 SECT_OFF_TEXT (objfile),
-				 psymbol_placement::GLOBAL,
-				 addr, cu->language, objfile);
+	    {
+	      psymbol.domain = VAR_DOMAIN;
+	      psymbol.aclass = LOC_STATIC;
+	      psymbol.ginfo.section = SECT_OFF_TEXT (objfile);
+	      psymbol.ginfo.value.address = addr;
+	      where = psymbol_placement::GLOBAL;
+	    }
 	}
       else
 	{
@@ -8301,42 +8302,37 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 	  if (!has_loc && !pdi->has_const_value)
 	    return;
 
-	  add_psymbol_to_list (actual_name,
-			       built_actual_name != NULL,
-			       VAR_DOMAIN, LOC_STATIC,
-			       SECT_OFF_TEXT (objfile),
-			       psymbol_placement::STATIC,
-			       has_loc ? addr : 0,
-			       cu->language, objfile);
+	  psymbol.domain = VAR_DOMAIN;
+	  psymbol.aclass = LOC_STATIC;
+	  psymbol.ginfo.section = SECT_OFF_TEXT (objfile);
+	  if (has_loc)
+	    psymbol.ginfo.value.address = addr;
+	  where = psymbol_placement::STATIC;
 	}
       break;
     case DW_TAG_typedef:
     case DW_TAG_base_type:
     case DW_TAG_subrange_type:
-      add_psymbol_to_list (actual_name,
-			   built_actual_name != NULL,
-			   VAR_DOMAIN, LOC_TYPEDEF, -1,
-			   psymbol_placement::STATIC,
-			   0, cu->language, objfile);
+      psymbol.domain = VAR_DOMAIN;
+      psymbol.aclass = LOC_TYPEDEF;
+      where = psymbol_placement::STATIC;
       break;
     case DW_TAG_imported_declaration:
     case DW_TAG_namespace:
-      add_psymbol_to_list (actual_name,
-			   built_actual_name != NULL,
-			   VAR_DOMAIN, LOC_TYPEDEF, -1,
-			   psymbol_placement::GLOBAL,
-			   0, cu->language, objfile);
+      psymbol.domain = VAR_DOMAIN;
+      psymbol.aclass = LOC_TYPEDEF;
+      where = psymbol_placement::GLOBAL;
       break;
     case DW_TAG_module:
       /* With Fortran 77 there might be a "BLOCK DATA" module
          available without any name.  If so, we skip the module as it
          doesn't bring any value.  */
       if (actual_name != nullptr)
-	add_psymbol_to_list (actual_name,
-			     built_actual_name != NULL,
-			     MODULE_DOMAIN, LOC_TYPEDEF, -1,
-			     psymbol_placement::GLOBAL,
-			     0, cu->language, objfile);
+	{
+	  psymbol.domain = MODULE_DOMAIN;
+	  psymbol.aclass = LOC_TYPEDEF;
+	  where = psymbol_placement::GLOBAL;
+	}
       break;
     case DW_TAG_class_type:
     case DW_TAG_interface_type:
@@ -8353,27 +8349,30 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 
       /* NOTE: carlton/2003-10-07: See comment in new_symbol about
 	 static vs. global.  */
-      add_psymbol_to_list (actual_name,
-			   built_actual_name != NULL,
-			   STRUCT_DOMAIN, LOC_TYPEDEF, -1,
-			   cu->language == language_cplus
-			   ? psymbol_placement::GLOBAL
-			   : psymbol_placement::STATIC,
-			   0, cu->language, objfile);
-
+      psymbol.domain = STRUCT_DOMAIN;
+      psymbol.aclass = LOC_TYPEDEF;
+      where = (cu->language == language_cplus
+	       ? psymbol_placement::GLOBAL
+	       : psymbol_placement::STATIC);
       break;
     case DW_TAG_enumerator:
-      add_psymbol_to_list (actual_name,
-			   built_actual_name != NULL,
-			   VAR_DOMAIN, LOC_CONST, -1,
-			   cu->language == language_cplus
-			   ? psymbol_placement::GLOBAL
-			   : psymbol_placement::STATIC,
-			   0, cu->language, objfile);
+      psymbol.domain = VAR_DOMAIN;
+      psymbol.aclass = LOC_CONST;
+      where = (cu->language == language_cplus
+	       ? psymbol_placement::GLOBAL
+	       : psymbol_placement::STATIC);
       break;
     default:
       break;
     }
+
+  if (where.has_value ())
+    {
+      psymbol.ginfo.compute_and_set_names (actual_name,
+					   built_actual_name != nullptr,
+					   objfile->per_bfd);
+      add_psymbol_to_list (psymbol, *where, objfile);
+    }
 }
 
 /* Read a partial die corresponding to a namespace; also, add a symbol
-- 
2.17.2

[PATCH 07/10] Don't call compute_and_set_names for partial symbols

[Tom Tromey]

As mentioned in another thread, there's currently no need to call
compute_and_set_names for partial symbols.  Because the DWARF partial
symbol reader constructs demangled names, this call can only demangle
a name by mistake.

So, this patch changes the DWARF reader to simply set the linkage name
on the new symbol.  This is equivalent to what was done before.  There
should be no user-visible change from this patch, aside from gdb
speeding up a bit.

... there *should* be, but this regressed
dw2-namespaceless-anonymous.exp.  However, upon examination, I think
that test is incorrect.  It puts a mangled name into DW_AT_name, and
it puts the variable at the top level, not in a namespace.  This isn't
what C++ compilers ought to do.  So, this patch also updates the test
case.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (add_partial_symbol): Do not call
	compute_and_set_names.

gdb/testsuite/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* gdb.dwarf2/dw2-namespaceless-anonymous.S: Remove.
	* gdb.dwarf2/dw2-namespaceless-anonymous.c: New file.
	* gdb.dwarf2/dw2-namespaceless-anonymous.exp: Use DWARF
	assembler.
---
 gdb/ChangeLog                                 |  5 +
 gdb/dwarf2/read.c                             |  6 +-
 gdb/testsuite/ChangeLog                       |  7 ++
 .../gdb.dwarf2/dw2-namespaceless-anonymous.S  | 93 -------------------
 .../gdb.dwarf2/dw2-namespaceless-anonymous.c  | 22 +++++
 .../dw2-namespaceless-anonymous.exp           | 44 +++++++--
 6 files changed, 75 insertions(+), 102 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.c

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c3723684386..2bc7b521f2e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -8368,9 +8368,9 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 
   if (where.has_value ())
     {
-      psymbol.ginfo.compute_and_set_names (actual_name,
-					   built_actual_name != nullptr,
-					   objfile->per_bfd);
+      if (built_actual_name != nullptr)
+	actual_name = objfile->intern (actual_name);
+      psymbol.ginfo.set_linkage_name (actual_name);
       add_psymbol_to_list (psymbol, *where, objfile);
     }
 }
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.S b/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.S
deleted file mode 100644
index e5b1d66bb3a..00000000000
--- a/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.S
+++ /dev/null
@@ -1,93 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2012-2020 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/>.  */
-
-	.data
-var:	.4byte	1
-
-	.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 */
-	.ascii	"file1.txt\0"			/* DW_AT_name */
-	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
-	.byte	4				/* DW_LANG_C_plus_plus (C++) */
-
-.Ltype_myint:
-	.uleb128	2			/* Abbrev: DW_TAG_base_type */
-	.ascii		"myint\0"			/* DW_AT_name */
-	.byte		4			/* DW_AT_byte_size */
-	.byte		5			/* DW_AT_encoding */
-
-	.uleb128	7			/* Abbrev: DW_TAG_variable (location) */
-	.ascii		"_ZN12_GLOBAL__N_11vE\0" /* DW_AT_name = "(anonymous namespace)::v" */
-	.byte		2f - 1f			/* DW_AT_location */
-1:	.byte		3			/*   DW_OP_addr */
-	.4byte		var			/*   <addr> */
-2:	.4byte		.Ltype_myint-.Lcu1_begin /* DW_AT_type */
-
-	.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	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	7			/* Abbrev code (location) */
-	.uleb128	0x34			/* DW_TAG_variable */
-	.byte		0			/* has_children */
-	.uleb128	0x3			/* DW_AT_name */
-	.uleb128	0x8			/* DW_FORM_string */
-	.uleb128	0x2			/* DW_AT_location */
-	.uleb128	0xa			/* DW_FORM_block1 */
-	.uleb128	0x49			/* DW_AT_type */
-	.uleb128	0x13			/* DW_FORM_ref4 */
-	.byte		0x0			/* Terminator */
-	.byte		0x0			/* Terminator */
-
-	.byte		0x0			/* Terminator */
-	.byte		0x0			/* Terminator */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.c b/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.c
new file mode 100644
index 00000000000..3c5e258090c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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/>.  */
+
+char _ZN12_GLOBAL__N_11vE = 1;
+
+int main ()
+{
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.exp b/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.exp
index 1edc468d62f..5b61a6ba928 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-namespaceless-anonymous.exp
@@ -14,19 +14,51 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 load_lib dwarf.exp
 
+load_lib dwarf.exp
+
 # This test can only be run on targets which support DWARF-2 and use gas.
 if {![dwarf2_support]} {
     return 0  
 }
 
-standard_testfile .S
+standard_testfile dw2-namespaceless-anonymous.c dw2-namespaceless-anonymous.S
 
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" $binfile \
-	   object {nodebug}] != "" } {
-    return -1
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {DW_AT_name     dw2-namespaceless-anonymous.c}
+	    {DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels myint
+
+	    myint: DW_TAG_base_type {
+		{DW_AT_byte_size 1 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      myint}
+	    }
+
+	    DW_TAG_namespace {} {
+		DW_TAG_variable {
+		    {DW_AT_name v}
+		    {DW_AT_linkage_name _ZN12_GLOBAL__N_11vE}
+		    {DW_AT_location {
+			DW_OP_addr [gdb_target_symbol _ZN12_GLOBAL__N_11vE]
+		    } SPECIAL_expr}
+		    {DW_AT_type :$myint}
+		}
+	    }
+	}
+    }
 }
 
-clean_restart $testfile
+if {[prepare_for_testing ${testfile}.exp ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
 
 gdb_test "ptype '(anonymous namespace)::v'" "type = myint"
-gdb_test "p '(anonymous namespace)::v'" " = 1"
+gdb_test "p/d '(anonymous namespace)::v'" " = 1"
-- 
2.17.2

[PATCH 08/10] Use the linkage name if it exists

[Tom Tromey]

The DWARF reader has had some odd code since the "physname" patches landed.

In particular, these patches caused PR symtab/12707; namely, they made
it so "set print demangle off" no longer works.

This patch attempts to fix the problem.  It arranges to store the
linkage name on the symbol if it exists, and it changes the DWARF
reader so that the demangled name is no longer (usually) stored in the
symbol's "linkage name" field.

c-linkage-name.exp needed a tweak, because it started working
correctly.  This conforms to what I think ought to happen, so this
seems like an improvement here.

compile-object-load.c needed a small change to use
symbol_matches_search_name rather than directly examining the linkage
name.  Looking directly at the name does the wrong thing for C++.

There is still some name-related confusion in the DWARF reader:

* "physname" often refers to the logical name and not what I would
  consider to be the "physical" name;

* dwarf2_full_name, dwarf2_name, and dwarf2_physname all exist and
  return different strings -- but this seems like at least one name
  too many.  For example, Fortran requires dwarf2_full_name, but other
  languages do not.

* To my surprise, dwarf2_physname prefers the form emitted by the
  demangler over the one that it computes.  This seems backward to me,
  given that the partial symbol reader prefers the opposite, and it
  seems to me that this choice may perform better as well.

I didn't attempt to clean up these things.  It would be good to do,
but whenever I contemplate it I get caught up in dreams of truly
rewriting the DWARF reader instead.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	PR symtab/12707:
	* dwarf2/read.c (add_partial_symbol): Use the linkage name if it
	exists.
	(new_symbol): Likewise.
	* compile/compile-object-load.c (get_out_value_type): Use
	symbol_matches_search_name.

gdb/testsuite/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	PR symtab/12707:
	* gdb.python/py-symbol.exp: Update expected results for
	linkage_name test.
	* gdb.cp/print-demangle.exp: New file.
	* gdb.base/c-linkage-name.exp: Fix test.
	* gdb.guile/scm-symbol.exp: Update expected results for
	linkage_name test.
---
 gdb/ChangeLog                             |  9 +++++++
 gdb/compile/compile-object-load.c         |  7 ++---
 gdb/dwarf2/read.c                         | 29 ++++++++++++++------
 gdb/testsuite/ChangeLog                   | 10 +++++++
 gdb/testsuite/gdb.base/c-linkage-name.exp |  2 +-
 gdb/testsuite/gdb.cp/print-demangle.exp   | 32 +++++++++++++++++++++++
 gdb/testsuite/gdb.guile/scm-symbol.exp    |  4 +--
 gdb/testsuite/gdb.python/py-symbol.exp    |  2 +-
 8 files changed, 79 insertions(+), 16 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/print-demangle.exp

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 3fe95183e32..be4fa767142 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -402,6 +402,9 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
   int nblocks = 0;
   int block_loop = 0;
 
+  lookup_name_info func_matcher (GCC_FE_WRAPPER_FUNCTION,
+				 symbol_name_match_type::SEARCH_NAME);
+
   bv = SYMTAB_BLOCKVECTOR (func_sym->owner.symtab);
   nblocks = BLOCKVECTOR_NBLOCKS (bv);
 
@@ -433,9 +436,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
       if (function != NULL
 	  && (BLOCK_SUPERBLOCK (function_block)
 	      == BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK))
-	  && (strcmp_iw (function->linkage_name (),
-			 GCC_FE_WRAPPER_FUNCTION)
-	      == 0))
+	  && symbol_matches_search_name (function, func_matcher))
 	break;
     }
   if (block_loop == nblocks)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2bc7b521f2e..1e824ca399d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -8370,7 +8370,14 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
     {
       if (built_actual_name != nullptr)
 	actual_name = objfile->intern (actual_name);
-      psymbol.ginfo.set_linkage_name (actual_name);
+      if (pdi->linkage_name == nullptr || cu->language == language_ada)
+	psymbol.ginfo.set_linkage_name (actual_name);
+      else
+	{
+	  psymbol.ginfo.set_demangled_name (actual_name,
+					    &objfile->objfile_obstack);
+	  psymbol.ginfo.set_linkage_name (pdi->linkage_name);
+	}
       add_psymbol_to_list (psymbol, *where, objfile);
     }
 }
@@ -20550,7 +20557,6 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   name = dwarf2_name (die, cu);
   if (name)
     {
-      const char *linkagename;
       int suppress_add = 0;
 
       if (space)
@@ -20561,14 +20567,21 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 
       /* Cache this symbol's name and the name's demangled form (if any).  */
       sym->set_language (cu->language, &objfile->objfile_obstack);
-      linkagename = dwarf2_physname (name, die, cu);
-      sym->compute_and_set_names (linkagename, false, objfile->per_bfd);
-
       /* Fortran does not have mangling standard and the mangling does differ
 	 between gfortran, iFort etc.  */
-      if (cu->language == language_fortran
-          && symbol_get_demangled_name (sym) == NULL)
-	sym->set_demangled_name (dwarf2_full_name (name, die, cu), NULL);
+      const char *physname
+	= (cu->language == language_fortran
+	   ? dwarf2_full_name (name, die, cu)
+	   : dwarf2_physname (name, die, cu));
+      const char *linkagename = dw2_linkage_name (die, cu);
+
+      if (linkagename == nullptr || cu->language == language_ada)
+	sym->set_linkage_name (physname);
+      else
+	{
+	  sym->set_demangled_name (physname, &objfile->objfile_obstack);
+	  sym->set_linkage_name (linkagename);
+	}
 
       /* Default assumptions.
          Use the passed type or decode it from the die.  */
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
index 4551c8dcac2..cf3e3780bb6 100644
--- a/gdb/testsuite/gdb.base/c-linkage-name.exp
+++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
@@ -48,7 +48,7 @@ verify_psymtab_expanded c-linkage-name-2.c no
 # Try to print MUNDANE, but using its linkage name.
 
 gdb_test "print symada__cS" \
-         "'symada__cS' has unknown type; cast it to its declared type" \
+         " = {a = 100829103}" \
          "print symada__cS before partial symtab expansion"
 
 # Force the symbols to be expanded for the unit that contains
diff --git a/gdb/testsuite/gdb.cp/print-demangle.exp b/gdb/testsuite/gdb.cp/print-demangle.exp
new file mode 100644
index 00000000000..9e0f706febf
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/print-demangle.exp
@@ -0,0 +1,32 @@
+# Copyright (C) 2013, 2020 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/>.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile bool.cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+runto_main
+
+gdb_breakpoint "return_true"
+
+gdb_continue_to_breakpoint "return_true"
+
+gdb_test_no_output "set print demangle off"
+
+gdb_test "frame" " _Z11return_truev .*"
diff --git a/gdb/testsuite/gdb.guile/scm-symbol.exp b/gdb/testsuite/gdb.guile/scm-symbol.exp
index 4addc0d10d0..486fc8fcfdb 100644
--- a/gdb/testsuite/gdb.guile/scm-symbol.exp
+++ b/gdb/testsuite/gdb.guile/scm-symbol.exp
@@ -169,10 +169,8 @@ gdb_test "guile (print (symbol-name cplusfunc))" \
     "= SimpleClass::valueofi().*" "test method.name"
 gdb_test "guile (print (symbol-print-name cplusfunc))" \
     "= SimpleClass::valueofi().*" "test method.print_name"
-# FIXME: GDB is broken here and we're verifying broken behaviour.
-# (linkage-name should be the mangled name)
 gdb_test "guile (print (symbol-linkage-name cplusfunc))" \
-    "SimpleClass::valueofi().*" "test method.linkage_name"
+    "_ZN11SimpleClass8valueofiEv.*" "test method.linkage_name"
 gdb_test "guile (print (= (symbol-addr-class cplusfunc) SYMBOL_LOC_BLOCK))" "= #t"
 
 # Test is_valid when the objfile is unloaded.  This must be the last
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index c4bae9f07f2..caa7ddc8007 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -211,7 +211,7 @@ gdb_test "python print (cplusfunc.is_function)" \
 
 gdb_test "python print (cplusfunc.name)" "SimpleClass::valueofi().*" "test method.name"
 gdb_test "python print (cplusfunc.print_name)" "SimpleClass::valueofi().*" "test method.print_name"
-gdb_test "python print (cplusfunc.linkage_name)" "SimpleClass::valueofi().*" "test method.linkage_name"
+gdb_test "python print (cplusfunc.linkage_name)" "_ZN11SimpleClass8valueofiEv.*" "test method.linkage_name"
 gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test method.addr_class"
 
 # Test is_valid when the objfile is unloaded.  This must be the last
-- 
2.17.2

[PATCH 09/10] Fix Rust test cases

[Tom Tromey]

PR rust/25025 notes that some Rust test cases fail.

Debugging gdb revealed that the Rust compiler emits different linkage
names that demangle to the same result.  Enabling complaints when
reading the test case is enough to show it:

    During symbol reading: Computed physname <generics::identity<f64>> does not match demangled <generics::identity> (from linkage <_ZN8generics8identity17h8540b320af6656d6E>) - DIE at 0x424 [in module /home/tromey/gdb/build/gdb/testsuite/outputs/gdb.rust/generics/generics]
    During symbol reading: Computed physname <generics::identity<u32>> does not match demangled <generics::identity> (from linkage <_ZN8generics8identity17hae302fad0c33bd7dE>) - DIE at 0x459 [in module /home/tromey/gdb/build/gdb/testsuite/outputs/gdb.rust/generics/generics]
    ...

This patch changes the DWARF reader to prefer the computed physname,
rather than the output of the demangler, for Rust.  This fixes the
bug.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	PR rust/25025:
	* dwarf2/read.c (dwarf2_physname): Do not demangle for Rust.
---
 gdb/ChangeLog     | 5 +++++
 gdb/dwarf2/read.c | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1e824ca399d..088306ee072 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -10306,7 +10306,8 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
   if (!die_needs_namespace (die, cu))
     return dwarf2_compute_name (name, die, cu, 1);
 
-  mangled = dw2_linkage_name (die, cu);
+  if (cu->language != language_rust)
+    mangled = dw2_linkage_name (die, cu);
 
   /* DW_AT_linkage_name is missing in some cases - depend on what GDB
      has computed.  */
-- 
2.17.2

[PATCH 10/10] Remove symbol_get_demangled_name

[Tom Tromey]

Now that symbol_get_demangled_name is only used by general_symbol_info
methods, and because these methods already check the symbol's language
to decide what to return, symbol_get_demangled_name is no longer
needed.  This patch removes it.

gdb/ChangeLog
2020-03-25  Tom Tromey  <tom@tromey.com>

	* symtab.h (symbol_get_demangled_name): Don't declare.
	* symtab.c (symbol_get_demangled_name): Remove.
	(general_symbol_info::natural_name)
	(general_symbol_info::demangled_name): Update.
---
 gdb/ChangeLog |  7 +++++++
 gdb/symtab.c  | 21 +++------------------
 gdb/symtab.h  |  3 ---
 3 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index bdbfe8e60d3..02351eb2dd0 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -690,21 +690,6 @@ general_symbol_info::set_demangled_name (const char *name,
     language_specific.demangled_name = name;
 }
 
-/* Return the demangled name of GSYMBOL.  */
-
-const char *
-symbol_get_demangled_name (const struct general_symbol_info *gsymbol)
-{
-  if (gsymbol->language () == language_ada)
-    {
-      if (!gsymbol->ada_mangled)
-	return NULL;
-      /* Fall through.  */
-    }
-
-  return gsymbol->language_specific.demangled_name;
-}
-
 \f
 /* Initialize the language dependent portion of a symbol
    depending upon the language for the symbol.  */
@@ -976,8 +961,8 @@ general_symbol_info::natural_name () const
     case language_objc:
     case language_fortran:
     case language_rust:
-      if (symbol_get_demangled_name (this) != NULL)
-	return symbol_get_demangled_name (this);
+      if (language_specific.demangled_name != nullptr)
+	return language_specific.demangled_name;
       break;
     case language_ada:
       return ada_decode_symbol (this);
@@ -1002,7 +987,7 @@ general_symbol_info::demangled_name () const
     case language_objc:
     case language_fortran:
     case language_rust:
-      dem_name = symbol_get_demangled_name (this);
+      dem_name = language_specific.demangled_name;
       break;
     case language_ada:
       dem_name = ada_decode_symbol (this);
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 83938d064fe..65e1973f2b0 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -513,9 +513,6 @@ struct general_symbol_info
   short section;
 };
 
-extern const char *symbol_get_demangled_name
-  (const struct general_symbol_info *);
-
 extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
 
 /* Return the address of SYM.  The MAYBE_COPIED flag must be set on
-- 
2.17.2

Re: [PATCH 00/10] Fix two name-related bugs in DWARF reader

[Christian Biesinger]

On Wed, Mar 25, 2020 at 3:07 PM Tom Tromey <tom@tromey.com> wrote:
>
> I started this series by trying to fix PR rust/25025.  This showed
> that the Rust compiler sometimes emits two mangled forms that demangle
> to the same thing.
>
> In the end I could maybe have fixed this in a direct way (see patch #9
> for the details), but while debugging I went on a detour into the
> physname code and came up with this series.
>
> This changes the DWARF reader to avoid demangling when constructing
> partial symbols.  This obsoletes my previous patch to do the same
> thing; I think this approach is cleaner.
>
> It also attempts to fix the longstanding physname bug, PR 12707.
> After this series, the demangled form is now stored on the symbol
> again.

I haven't read the patches yet but just to clarify, does this mean
that a psymbol will now do the expected thing of storing the mangled
name as the linkage_name and the demangled name as the demangled_name?

Christian

Re: [PATCH 00/10] Fix two name-related bugs in DWARF reader

[Tom Tromey]

Christian> I haven't read the patches yet but just to clarify, does this mean
Christian> that a psymbol will now do the expected thing of storing the mangled
Christian> name as the linkage_name and the demangled name as the demangled_name?

Yeah, both psymbols and full symbols.

They actually store different variants, surprisingly enough.
The full symbol stores the argument types and the psymbol does not.

Tom

Re: [PATCH 00/10] Fix two name-related bugs in DWARF reader

[Tom de Vries]

On 25-03-2020 21:07, Tom Tromey wrote:
> I started this series by trying to fix PR rust/25025.  This showed
> that the Rust compiler sometimes emits two mangled forms that demangle
> to the same thing.
> 
> In the end I could maybe have fixed this in a direct way (see patch #9
> for the details), but while debugging I went on a detour into the
> physname code and came up with this series.
> 
> This changes the DWARF reader to avoid demangling when constructing
> partial symbols.  This obsoletes my previous patch to do the same
> thing; I think this approach is cleaner.
> 
> It also attempts to fix the longstanding physname bug, PR 12707.
> After this series, the demangled form is now stored on the symbol
> again.
> 
> Regression tested by the buildbot.

As extra data point, I've build and tested this series, and got
unexpected failures reduced from 8 to 1:
...
                === gdb Summary ===

# of expected passes            78972
# of unexpected failures        1
# of expected failures          135
# of known failures             72
# of untested testcases         24
# of unsupported tests          77
...

Thanks,
- Tom

Re: [PATCH 00/10] Fix two name-related bugs in DWARF reader

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> As extra data point, I've build and tested this series, and got
Tom> unexpected failures reduced from 8 to 1:

Thanks for doing that.
I was set to check it in, but I ran the tests with the .debug_names
board and now there is a regression there :(

Tom

Re: [PATCH 08/10] Use the linkage name if it exists

[Tom de Vries]

On 25-03-2020 21:07, Tom Tromey wrote:
> The DWARF reader has had some odd code since the "physname" patches landed.
> 
> In particular, these patches caused PR symtab/12707; namely, they made
> it so "set print demangle off" no longer works.
> 
> This patch attempts to fix the problem.  It arranges to store the
> linkage name on the symbol if it exists, and it changes the DWARF
> reader so that the demangled name is no longer (usually) stored in the
> symbol's "linkage name" field.
> 
> c-linkage-name.exp needed a tweak, because it started working
> correctly.  This conforms to what I think ought to happen, so this
> seems like an improvement here.
> 
> compile-object-load.c needed a small change to use
> symbol_matches_search_name rather than directly examining the linkage
> name.  Looking directly at the name does the wrong thing for C++.
> 
> There is still some name-related confusion in the DWARF reader:
> 
> * "physname" often refers to the logical name and not what I would
>   consider to be the "physical" name;
> 
> * dwarf2_full_name, dwarf2_name, and dwarf2_physname all exist and
>   return different strings -- but this seems like at least one name
>   too many.  For example, Fortran requires dwarf2_full_name, but other
>   languages do not.
> 
> * To my surprise, dwarf2_physname prefers the form emitted by the
>   demangler over the one that it computes.  This seems backward to me,
>   given that the partial symbol reader prefers the opposite, and it
>   seems to me that this choice may perform better as well.
> 
> I didn't attempt to clean up these things.  It would be good to do,
> but whenever I contemplate it I get caught up in dreams of truly
> rewriting the DWARF reader instead.
> 

Hi,

As you mentioned on IRC, there's a regression with
gdb.dwarf2/dw2-bad-mips-linkage-name.exp and target board
cc-with-debug-names. I tracked that down to this patch in the series.

Using readelf -w, I can read the .debug_names section, and see without
this patch:
...
[  6] #0002b60b f: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
DW_IDX_GNU_internal=1
[  8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
DW_IDX_GNU_internal=1
...
and with this patch:
...
[  6] #0ef9dc4b _Z1fv: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
DW_IDX_GNU_internal=1
[  8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
DW_IDX_GNU_internal=1
...

So we end up with the mangled name for f in the index.  That looks
significant, but I'm not sure yet if that is the reason why we're not
able to print the type of f.

Thanks,
- Tom

> gdb/ChangeLog
> 2020-03-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR symtab/12707:
> 	* dwarf2/read.c (add_partial_symbol): Use the linkage name if it
> 	exists.
> 	(new_symbol): Likewise.
> 	* compile/compile-object-load.c (get_out_value_type): Use
> 	symbol_matches_search_name.
> 
> gdb/testsuite/ChangeLog
> 2020-03-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR symtab/12707:
> 	* gdb.python/py-symbol.exp: Update expected results for
> 	linkage_name test.
> 	* gdb.cp/print-demangle.exp: New file.
> 	* gdb.base/c-linkage-name.exp: Fix test.
> 	* gdb.guile/scm-symbol.exp: Update expected results for
> 	linkage_name test.
> ---
>  gdb/ChangeLog                             |  9 +++++++
>  gdb/compile/compile-object-load.c         |  7 ++---
>  gdb/dwarf2/read.c                         | 29 ++++++++++++++------
>  gdb/testsuite/ChangeLog                   | 10 +++++++
>  gdb/testsuite/gdb.base/c-linkage-name.exp |  2 +-
>  gdb/testsuite/gdb.cp/print-demangle.exp   | 32 +++++++++++++++++++++++
>  gdb/testsuite/gdb.guile/scm-symbol.exp    |  4 +--
>  gdb/testsuite/gdb.python/py-symbol.exp    |  2 +-
>  8 files changed, 79 insertions(+), 16 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/print-demangle.exp
> 
> diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
> index 3fe95183e32..be4fa767142 100644
> --- a/gdb/compile/compile-object-load.c
> +++ b/gdb/compile/compile-object-load.c
> @@ -402,6 +402,9 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
>    int nblocks = 0;
>    int block_loop = 0;
>  
> +  lookup_name_info func_matcher (GCC_FE_WRAPPER_FUNCTION,
> +				 symbol_name_match_type::SEARCH_NAME);
> +
>    bv = SYMTAB_BLOCKVECTOR (func_sym->owner.symtab);
>    nblocks = BLOCKVECTOR_NBLOCKS (bv);
>  
> @@ -433,9 +436,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
>        if (function != NULL
>  	  && (BLOCK_SUPERBLOCK (function_block)
>  	      == BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK))
> -	  && (strcmp_iw (function->linkage_name (),
> -			 GCC_FE_WRAPPER_FUNCTION)
> -	      == 0))
> +	  && symbol_matches_search_name (function, func_matcher))
>  	break;
>      }
>    if (block_loop == nblocks)
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 2bc7b521f2e..1e824ca399d 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -8370,7 +8370,14 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
>      {
>        if (built_actual_name != nullptr)
>  	actual_name = objfile->intern (actual_name);
> -      psymbol.ginfo.set_linkage_name (actual_name);
> +      if (pdi->linkage_name == nullptr || cu->language == language_ada)
> +	psymbol.ginfo.set_linkage_name (actual_name);
> +      else
> +	{
> +	  psymbol.ginfo.set_demangled_name (actual_name,
> +					    &objfile->objfile_obstack);
> +	  psymbol.ginfo.set_linkage_name (pdi->linkage_name);
> +	}
>        add_psymbol_to_list (psymbol, *where, objfile);
>      }
>  }
> @@ -20550,7 +20557,6 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>    name = dwarf2_name (die, cu);
>    if (name)
>      {
> -      const char *linkagename;
>        int suppress_add = 0;
>  
>        if (space)
> @@ -20561,14 +20567,21 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>  
>        /* Cache this symbol's name and the name's demangled form (if any).  */
>        sym->set_language (cu->language, &objfile->objfile_obstack);
> -      linkagename = dwarf2_physname (name, die, cu);
> -      sym->compute_and_set_names (linkagename, false, objfile->per_bfd);
> -
>        /* Fortran does not have mangling standard and the mangling does differ
>  	 between gfortran, iFort etc.  */
> -      if (cu->language == language_fortran
> -          && symbol_get_demangled_name (sym) == NULL)
> -	sym->set_demangled_name (dwarf2_full_name (name, die, cu), NULL);
> +      const char *physname
> +	= (cu->language == language_fortran
> +	   ? dwarf2_full_name (name, die, cu)
> +	   : dwarf2_physname (name, die, cu));
> +      const char *linkagename = dw2_linkage_name (die, cu);
> +
> +      if (linkagename == nullptr || cu->language == language_ada)
> +	sym->set_linkage_name (physname);
> +      else
> +	{
> +	  sym->set_demangled_name (physname, &objfile->objfile_obstack);
> +	  sym->set_linkage_name (linkagename);
> +	}
>  
>        /* Default assumptions.
>           Use the passed type or decode it from the die.  */
> diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
> index 4551c8dcac2..cf3e3780bb6 100644
> --- a/gdb/testsuite/gdb.base/c-linkage-name.exp
> +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
> @@ -48,7 +48,7 @@ verify_psymtab_expanded c-linkage-name-2.c no
>  # Try to print MUNDANE, but using its linkage name.
>  
>  gdb_test "print symada__cS" \
> -         "'symada__cS' has unknown type; cast it to its declared type" \
> +         " = {a = 100829103}" \
>           "print symada__cS before partial symtab expansion"
>  
>  # Force the symbols to be expanded for the unit that contains
> diff --git a/gdb/testsuite/gdb.cp/print-demangle.exp b/gdb/testsuite/gdb.cp/print-demangle.exp
> new file mode 100644
> index 00000000000..9e0f706febf
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/print-demangle.exp
> @@ -0,0 +1,32 @@
> +# Copyright (C) 2013, 2020 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/>.
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +standard_testfile bool.cc
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
> +    return -1
> +}
> +
> +runto_main
> +
> +gdb_breakpoint "return_true"
> +
> +gdb_continue_to_breakpoint "return_true"
> +
> +gdb_test_no_output "set print demangle off"
> +
> +gdb_test "frame" " _Z11return_truev .*"
> diff --git a/gdb/testsuite/gdb.guile/scm-symbol.exp b/gdb/testsuite/gdb.guile/scm-symbol.exp
> index 4addc0d10d0..486fc8fcfdb 100644
> --- a/gdb/testsuite/gdb.guile/scm-symbol.exp
> +++ b/gdb/testsuite/gdb.guile/scm-symbol.exp
> @@ -169,10 +169,8 @@ gdb_test "guile (print (symbol-name cplusfunc))" \
>      "= SimpleClass::valueofi().*" "test method.name"
>  gdb_test "guile (print (symbol-print-name cplusfunc))" \
>      "= SimpleClass::valueofi().*" "test method.print_name"
> -# FIXME: GDB is broken here and we're verifying broken behaviour.
> -# (linkage-name should be the mangled name)
>  gdb_test "guile (print (symbol-linkage-name cplusfunc))" \
> -    "SimpleClass::valueofi().*" "test method.linkage_name"
> +    "_ZN11SimpleClass8valueofiEv.*" "test method.linkage_name"
>  gdb_test "guile (print (= (symbol-addr-class cplusfunc) SYMBOL_LOC_BLOCK))" "= #t"
>  
>  # Test is_valid when the objfile is unloaded.  This must be the last
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index c4bae9f07f2..caa7ddc8007 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -211,7 +211,7 @@ gdb_test "python print (cplusfunc.is_function)" \
>  
>  gdb_test "python print (cplusfunc.name)" "SimpleClass::valueofi().*" "test method.name"
>  gdb_test "python print (cplusfunc.print_name)" "SimpleClass::valueofi().*" "test method.print_name"
> -gdb_test "python print (cplusfunc.linkage_name)" "SimpleClass::valueofi().*" "test method.linkage_name"
> +gdb_test "python print (cplusfunc.linkage_name)" "_ZN11SimpleClass8valueofiEv.*" "test method.linkage_name"
>  gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test method.addr_class"
>  
>  # Test is_valid when the objfile is unloaded.  This must be the last
> 

Re: [PATCH 08/10] Use the linkage name if it exists

[Tom de Vries]

On 24-04-2020 18:06, Tom de Vries wrote:
> On 25-03-2020 21:07, Tom Tromey wrote:
>> The DWARF reader has had some odd code since the "physname" patches landed.
>>
>> In particular, these patches caused PR symtab/12707; namely, they made
>> it so "set print demangle off" no longer works.
>>
>> This patch attempts to fix the problem.  It arranges to store the
>> linkage name on the symbol if it exists, and it changes the DWARF
>> reader so that the demangled name is no longer (usually) stored in the
>> symbol's "linkage name" field.
>>
>> c-linkage-name.exp needed a tweak, because it started working
>> correctly.  This conforms to what I think ought to happen, so this
>> seems like an improvement here.
>>
>> compile-object-load.c needed a small change to use
>> symbol_matches_search_name rather than directly examining the linkage
>> name.  Looking directly at the name does the wrong thing for C++.
>>
>> There is still some name-related confusion in the DWARF reader:
>>
>> * "physname" often refers to the logical name and not what I would
>>   consider to be the "physical" name;
>>
>> * dwarf2_full_name, dwarf2_name, and dwarf2_physname all exist and
>>   return different strings -- but this seems like at least one name
>>   too many.  For example, Fortran requires dwarf2_full_name, but other
>>   languages do not.
>>
>> * To my surprise, dwarf2_physname prefers the form emitted by the
>>   demangler over the one that it computes.  This seems backward to me,
>>   given that the partial symbol reader prefers the opposite, and it
>>   seems to me that this choice may perform better as well.
>>
>> I didn't attempt to clean up these things.  It would be good to do,
>> but whenever I contemplate it I get caught up in dreams of truly
>> rewriting the DWARF reader instead.
>>
> 
> Hi,
> 
> As you mentioned on IRC, there's a regression with
> gdb.dwarf2/dw2-bad-mips-linkage-name.exp and target board
> cc-with-debug-names. I tracked that down to this patch in the series.
> 
> Using readelf -w, I can read the .debug_names section, and see without
> this patch:
> ...
> [  6] #0002b60b f: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> [  8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> ...
> and with this patch:
> ...
> [  6] #0ef9dc4b _Z1fv: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> [  8] #0002b60c g: <3> DW_TAG_subprogram DW_IDX_compile_unit=3
> DW_IDX_GNU_internal=1
> ...
> 
> So we end up with the mangled name for f in the index.  That looks
> significant, but I'm not sure yet if that is the reason why we're not
> able to print the type of f.
> 

Hmm, one thing that is odd about this test-case is that it constructs a
C language CU, which contains a subprogram with DW_AT_MIPS_linkage_name
attribute with a C++ mangled name.

If I update the testcase to use a c++ language CU, the test passes:
...
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-lin
kage-name.exp
index d00308a5702..5f01c41aaa3 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -38,7 +38,7 @@ Dwarf::assemble $asm_file {

     cu {} {
        DW_TAG_compile_unit {
-                {DW_AT_language @DW_LANG_C}
+                {DW_AT_language @DW_LANG_C_plus_plus}
                 {DW_AT_name     dw2-bad-mips-linkage-name.c}
                 {DW_AT_comp_dir /tmp}

@@ -78,5 +78,5 @@ if { [prepare_for_testing "failed to prepare"
${testfile} \
 # much matter what we test here, so long as we do something to make
 # sure that the DWARF is read.

-gdb_test "ptype f" " = bool \\(\\)"
-gdb_test "ptype g" " = bool \\(\\)"
+gdb_test "ptype f" " = bool \\(void\\)"
+gdb_test "ptype g" " = bool \\(void\\)"
...

Thanks,
- Tom

Re: [PATCH 08/10] Use the linkage name if it exists

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Hmm, one thing that is odd about this test-case is that it constructs a
Tom> C language CU, which contains a subprogram with DW_AT_MIPS_linkage_name
Tom> attribute with a C++ mangled name.

This sounds very familiar.  I feel like I patched this at some point but
now I don't know where that patch went.

Tom> If I update the testcase to use a c++ language CU, the test passes:

I think this is the correct thing to do.
The main point of the test is to ensure that the linkage name with the
wrong form doesn't cause a crash:

		{DW_AT_MIPS_linkage_name 42 DW_FORM_data1}

I suggest you check this in.

thanks,
Tom

[committed][gdb/testsuite] Fix language in dw2-bad-mips-linkage-name.exp

[Tom de Vries]

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

[ was: Re: [PATCH 08/10] Use the linkage name if it exists ]
On 24-04-2020 22:50, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Hmm, one thing that is odd about this test-case is that it constructs a
> Tom> C language CU, which contains a subprogram with DW_AT_MIPS_linkage_name
> Tom> attribute with a C++ mangled name.
> 
> This sounds very familiar.  I feel like I patched this at some point but
> now I don't know where that patch went.
> 
> Tom> If I update the testcase to use a c++ language CU, the test passes:
> 
> I think this is the correct thing to do.
> The main point of the test is to ensure that the linkage name with the
> wrong form doesn't cause a crash:
> 
> 		{DW_AT_MIPS_linkage_name 42 DW_FORM_data1}
> 
> I suggest you check this in.

Ack, committed as attached below.

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-testsuite-Fix-language-in-dw2-bad-mips-linkage-name.exp.patch --]
[-- Type: text/x-patch, Size: 1494 bytes --]

[gdb/testsuite] Fix language in dw2-bad-mips-linkage-name.exp

The test-case gdb.dwarf2/dw2-bad-mips-linkage-name.exp has a CU with
language C, which contains a subprogram with a C++-mangled name as its
DW_AT_mips_linkage_name attribute.

Fix this by changing the language of the CU to C++.

Tested on x86_64-linux.

gdb/testsuite/ChangeLog:

2020-04-24  Tom de Vries  <tdevries@suse.de>

	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: Set language of CU to
	C++.

---
 gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
index d00308a5702..5f01c41aaa3 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -38,7 +38,7 @@ Dwarf::assemble $asm_file {
 
     cu {} {
 	DW_TAG_compile_unit {
-                {DW_AT_language @DW_LANG_C}
+                {DW_AT_language @DW_LANG_C_plus_plus}
                 {DW_AT_name     dw2-bad-mips-linkage-name.c}
                 {DW_AT_comp_dir /tmp}
 
@@ -78,5 +78,5 @@ if { [prepare_for_testing "failed to prepare" ${testfile} \
 # much matter what we test here, so long as we do something to make
 # sure that the DWARF is read.
 
-gdb_test "ptype f" " = bool \\(\\)"
-gdb_test "ptype g" " = bool \\(\\)"
+gdb_test "ptype f" " = bool \\(void\\)"
+gdb_test "ptype g" " = bool \\(void\\)"

Re: [committed][gdb/testsuite] Fix language in dw2-bad-mips-linkage-name.exp

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

>> I suggest you check this in.

Tom> Ack, committed as attached below.

Thank you.
I'm going to check in my series now.

Tom