[04/20] Change some uses of DW_STRING to string method

Message ID 20200328192208.11324-5-tom@tromey.com
State New
Headers show
Series
  • Make DWARF attribute references safe
Related show

Commit Message

Tom Tromey March 28, 2020, 7:21 p.m.
This changes some of the simpler spots to use attribute::string rather
than DW_STRING.

2020-03-28  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (partial_die_info::read)
	(dwarf2_const_value_attr, anonymous_struct_prefix, )
	(dwarf2_name, dwarf2_fetch_constant_bytes): Use
	attribute::string.
---
 gdb/ChangeLog     |  7 +++++++
 gdb/dwarf2/read.c | 52 +++++++++++++++++++++++------------------------
 2 files changed, 33 insertions(+), 26 deletions(-)

-- 
2.17.2

Comments

Simon Marchi March 30, 2020, 2:56 p.m. | #1
On 2020-03-28 3:21 p.m., Tom Tromey wrote:
> @@ -21247,25 +21247,25 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)

>  	 structures or unions.  These were of the form "._%d" in GCC 4.1,

>  	 or simply "<anonymous struct>" or "<anonymous union>" in GCC 4.3

>  	 and GCC 4.4.  We work around this problem by ignoring these.  */

> -      if (attr && DW_STRING (attr)

> -	  && (startswith (DW_STRING (attr), "._")

> -	      || startswith (DW_STRING (attr), "<anonymous")))

> +      if (attr && attr->string ()

> +	  && (startswith (attr->string (), "._")

> +	      || startswith (attr->string (), "<anonymous")))


In cases like this, I think it would be worth assigning the result of attr->string
to a local variable, to avoid paying the cost of the check multiple times
unnecessarily.

But otherwise, this LGTM.

Simon
Tom Tromey March 30, 2020, 11:53 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> In cases like this, I think it would be worth assigning the result of attr->string
Simon> to a local variable, to avoid paying the cost of the check multiple times
Simon> unnecessarily.

I made this change.

I did some profiling of the DWARF reader recently and I think that some
of these methods should probably be inlined as well.  I'll double-check
... my profiling was on a branch without this series, but inlining
form_is_ref and part of get_ref_die_offset was a win.

Tom

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d2b274a6e3a..eb5ee98de60 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17879,14 +17879,14 @@  partial_die_info::read (const struct die_reader_specs *reader,
 	    case DW_TAG_enumerator:
 	      /* These tags always have simple identifiers already; no need
 		 to canonicalize them.  */
-	      name = DW_STRING (&attr);
+	      name = attr.string ();
 	      break;
 	    default:
 	      {
 		struct objfile *objfile = dwarf2_per_objfile->objfile;
 
 		name
-		  = dwarf2_canonicalize_name (DW_STRING (&attr), cu, objfile);
+		  = dwarf2_canonicalize_name (attr.string (), cu, objfile);
 	      }
 	      break;
 	    }
@@ -20482,7 +20482,7 @@  dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
     case DW_FORM_GNU_strp_alt:
       /* DW_STRING is already allocated on the objfile obstack, point
 	 directly to it.  */
-      *bytes = (const gdb_byte *) DW_STRING (attr);
+      *bytes = (const gdb_byte *) attr->string ();
       break;
     case DW_FORM_block1:
     case DW_FORM_block2:
@@ -20930,21 +20930,21 @@  anonymous_struct_prefix (struct die_info *die, struct dwarf2_cu *cu)
     return NULL;
 
   attr = dw2_linkage_name_attr (die, cu);
-  if (attr == NULL || DW_STRING (attr) == NULL)
+  if (attr == NULL || attr->string () == NULL)
     return NULL;
 
   /* dwarf2_name had to be already called.  */
   gdb_assert (DW_STRING_IS_CANONICAL (attr));
 
   /* Strip the base name, keep any leading namespaces/classes.  */
-  base = strrchr (DW_STRING (attr), ':');
-  if (base == NULL || base == DW_STRING (attr) || base[-1] != ':')
+  base = strrchr (attr->string (), ':');
+  if (base == NULL || base == attr->string () || base[-1] != ':')
     return "";
 
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
   return obstack_strndup (&objfile->per_bfd->storage_obstack,
-			  DW_STRING (attr),
-			  &base[-1] - DW_STRING (attr));
+			  attr->string (),
+			  &base[-1] - attr->string ());
 }
 
 /* Return the name of the namespace/class that DIE is defined within,
@@ -21214,7 +21214,7 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
 
   attr = dwarf2_attr (die, DW_AT_name, cu);
-  if ((!attr || !DW_STRING (attr))
+  if ((!attr || !attr->string ())
       && die->tag != DW_TAG_namespace
       && die->tag != DW_TAG_class_type
       && die->tag != DW_TAG_interface_type
@@ -21232,11 +21232,11 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
     case DW_TAG_enumerator:
       /* These tags always have simple identifiers already; no need
 	 to canonicalize them.  */
-      return DW_STRING (attr);
+      return attr->string ();
 
     case DW_TAG_namespace:
-      if (attr != NULL && DW_STRING (attr) != NULL)
-	return DW_STRING (attr);
+      if (attr != NULL && attr->string () != NULL)
+	return attr->string ();
       return CP_ANONYMOUS_NAMESPACE_STR;
 
     case DW_TAG_class_type:
@@ -21247,25 +21247,25 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 	 structures or unions.  These were of the form "._%d" in GCC 4.1,
 	 or simply "<anonymous struct>" or "<anonymous union>" in GCC 4.3
 	 and GCC 4.4.  We work around this problem by ignoring these.  */
-      if (attr && DW_STRING (attr)
-	  && (startswith (DW_STRING (attr), "._")
-	      || startswith (DW_STRING (attr), "<anonymous")))
+      if (attr && attr->string ()
+	  && (startswith (attr->string (), "._")
+	      || startswith (attr->string (), "<anonymous")))
 	return NULL;
 
       /* GCC might emit a nameless typedef that has a linkage name.  See
 	 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510.  */
-      if (!attr || DW_STRING (attr) == NULL)
+      if (!attr || attr->string () == NULL)
 	{
 	  attr = dw2_linkage_name_attr (die, cu);
-	  if (attr == NULL || DW_STRING (attr) == NULL)
+	  if (attr == NULL || attr->string () == NULL)
 	    return NULL;
 
-	  /* Avoid demangling DW_STRING (attr) the second time on a second
+	  /* Avoid demangling attr->string () the second time on a second
 	     call for the same DIE.  */
 	  if (!DW_STRING_IS_CANONICAL (attr))
 	    {
 	      gdb::unique_xmalloc_ptr<char> demangled
-		(gdb_demangle (DW_STRING (attr), DMGL_TYPES));
+		(gdb_demangle (attr->string (), DMGL_TYPES));
 	      if (demangled == nullptr)
 		return nullptr;
 
@@ -21275,11 +21275,11 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 
 	  /* Strip any leading namespaces/classes, keep only the base name.
 	     DW_AT_name for named DIEs does not contain the prefixes.  */
-	  const char *base = strrchr (DW_STRING (attr), ':');
-	  if (base && base > DW_STRING (attr) && base[-1] == ':')
+	  const char *base = strrchr (attr->string (), ':');
+	  if (base && base > attr->string () && base[-1] == ':')
 	    return &base[1];
 	  else
-	    return DW_STRING (attr);
+	    return attr->string ();
 	}
       break;
 
@@ -21289,11 +21289,11 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 
   if (!DW_STRING_IS_CANONICAL (attr))
     {
-      DW_STRING (attr) = dwarf2_canonicalize_name (DW_STRING (attr), cu,
+      DW_STRING (attr) = dwarf2_canonicalize_name (attr->string (), cu,
 						   objfile);
       DW_STRING_IS_CANONICAL (attr) = 1;
     }
-  return DW_STRING (attr);
+  return attr->string ();
 }
 
 /* Return the die that this die in an extension of, or NULL if there
@@ -21801,8 +21801,8 @@  dwarf2_fetch_constant_bytes (sect_offset sect_off,
     case DW_FORM_GNU_strp_alt:
       /* DW_STRING is already allocated on the objfile obstack, point
 	 directly to it.  */
-      result = (const gdb_byte *) DW_STRING (attr);
-      *len = strlen (DW_STRING (attr));
+      result = (const gdb_byte *) attr->string ();
+      *len = strlen (attr->string ());
       break;
     case DW_FORM_block1:
     case DW_FORM_block2: