public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 00/20] Make DWARF attribute references safe
@ 2020-04-04 14:43 Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 01/20] Add attribute::value_as_string method Tom Tromey
                   ` (20 more replies)
  0 siblings, 21 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches

Here's v2 of the series to make DWARF attribute references safe.

v1 was here:

https://sourceware.org/pipermail/gdb-patches/2020-March/167085.html

I think this version addresses all the review comments.

Tom



^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 01/20] Add attribute::value_as_string method
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 02/20] Rename struct attribute accessors Tom Tromey
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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-04-04  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 0e5a8c8f536..a4a6db76c8f 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -61,6 +61,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 69b33513ad6..a9cabd69f9f 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -46,6 +46,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 bcc3116071e..1d1d041d303 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17925,7 +17925,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 ();
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
@@ -18978,17 +18978,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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 02/20] Rename struct attribute accessors
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 01/20] Add attribute::value_as_string method Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 03/20] Avoid using DW_* macros in dwarf2/attribute.c Tom Tromey
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes the "value_" prefix from the struct value accessors.
This seemed unnecessarily wordy to me.

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

	* dwarf2/read.c (dwarf2_find_base_address, read_call_site_scope)
	(dwarf2_get_pc_bounds, dwarf2_record_block_ranges)
	(partial_die_info::read, dwarf2_string_attr, new_symbol): Update.
	* dwarf2/attribute.h (struct attribute): Rename methods.
	* dwarf2/attribute.c (attribute::as_address): Rename from
	value_as_address.
	(attribute::as_string): Rename from value_as_string.
---
 gdb/ChangeLog          | 10 ++++++++++
 gdb/dwarf2/attribute.c |  4 ++--
 gdb/dwarf2/attribute.h |  4 ++--
 gdb/dwarf2/read.c      | 24 ++++++++++++------------
 4 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index a4a6db76c8f..65adaf51b61 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -32,7 +32,7 @@
 /* See attribute.h.  */
 
 CORE_ADDR
-attribute::value_as_address () const
+attribute::as_address () const
 {
   CORE_ADDR addr;
 
@@ -62,7 +62,7 @@ attribute::value_as_address () const
 /* See attribute.h.  */
 
 const char *
-attribute::value_as_string () const
+attribute::as_string () const
 {
   if (form == DW_FORM_strp || form == DW_FORM_line_strp
       || form == DW_FORM_string
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index a9cabd69f9f..861bdb478d1 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -44,11 +44,11 @@ struct attribute
 {
   /* Read the given attribute value as an address, taking the
      attribute's form into account.  */
-  CORE_ADDR value_as_address () const;
+  CORE_ADDR as_address () const;
 
   /* If the attribute has a string form, return the string value;
      otherwise return NULL.  */
-  const char *value_as_string () const;
+  const char *as_string () const;
 
   /* Return non-zero if ATTR's value is a section offset --- classes
      lineptr, loclistptr, macptr or rangelistptr --- or zero, otherwise.
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1d1d041d303..0d6ae4478ae 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5794,12 +5794,12 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
   if (attr != nullptr)
-    cu->base_address = attr->value_as_address ();
+    cu->base_address = attr->as_address ();
   else
     {
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr != nullptr)
-	cu->base_address = attr->value_as_address ();
+	cu->base_address = attr->as_address ();
     }
 }
 
@@ -13050,7 +13050,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		 sect_offset_str (die->sect_off), objfile_name (objfile));
       return;
     }
-  pc = attr->value_as_address () + baseaddr;
+  pc = attr->as_address () + baseaddr;
   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
 
   if (cu->call_site_htab == NULL)
@@ -13752,8 +13752,8 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr != nullptr)
         {
-	  low = attr->value_as_address ();
-	  high = attr_high->value_as_address ();
+	  low = attr->as_address ();
+	  high = attr_high->as_address ();
 	  if (cu->header.version >= 4 && attr_high->form_is_constant ())
 	    high += low;
 	}
@@ -13925,8 +13925,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr != nullptr)
         {
-          CORE_ADDR low = attr->value_as_address ();
-	  CORE_ADDR high = attr_high->value_as_address ();
+	  CORE_ADDR low = attr->as_address ();
+	  CORE_ADDR high = attr_high->as_address ();
 
 	  if (cu->header.version >= 4 && attr_high->form_is_constant ())
 	    high += low;
@@ -17925,15 +17925,15 @@ 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 = attr.value_as_string ();
+	  linkage_name = attr.as_string ();
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
-	  lowpc = attr.value_as_address ();
+	  lowpc = attr.as_address ();
 	  break;
 	case DW_AT_high_pc:
 	  has_high_pc_attr = 1;
-	  highpc = attr.value_as_address ();
+	  highpc = attr.as_address ();
 	  if (cu->header.version >= 4 && attr.form_is_constant ())
 		high_pc_relative = 1;
 	  break;
@@ -18978,7 +18978,7 @@ dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *c
 
   if (attr != NULL)
     {
-      str = attr->value_as_string ();
+      str = attr->as_string ();
       if (str == nullptr)
         complaint (_("string type expected for attribute %s for "
 		     "DIE at %s in module %s"),
@@ -20105,7 +20105,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	    {
 	      CORE_ADDR addr;
 
-	      addr = attr->value_as_address ();
+	      addr = attr->as_address ();
 	      addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
 	      SET_SYMBOL_VALUE_ADDRESS (sym, addr);
 	    }
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 03/20] Avoid using DW_* macros in dwarf2/attribute.c
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 01/20] Add attribute::value_as_string method Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 02/20] Rename struct attribute accessors Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 04/20] Change some uses of DW_STRING to string method Tom Tromey
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

There's no need to use the DW_* accessor macros in dwarf2/attribute.c,
and this is a necessary step toward our goal of removing them
entirely.

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

	* dwarf2/attribute.c (attribute::address): Don't use DW_UNSND or
	DW_ADDR.
	(attribute::string): Don't use DW_STRING.
	(attribute::get_ref_die_offset): Don't use DW_UNSND.
	(attribute::constant_value): Don't use DW_UNSND or DW_SND.
---
 gdb/ChangeLog          |  8 ++++++++
 gdb/dwarf2/attribute.c | 12 ++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 65adaf51b61..3acc13ce151 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -51,10 +51,10 @@ attribute::as_address () const
 	 as well as update callers to pass in at least the CU's DWARF
 	 version.  This is more overhead than what we're willing to
 	 expand for a pretty rare case.  */
-      addr = DW_UNSND (this);
+      addr = u.unsnd;
     }
   else
-    addr = DW_ADDR (this);
+    addr = u.addr;
 
   return addr;
 }
@@ -73,7 +73,7 @@ attribute::as_string () const
       || form == DW_FORM_strx4
       || form == DW_FORM_GNU_str_index
       || form == DW_FORM_GNU_strp_alt)
-    return DW_STRING (this);
+    return u.str;
   return nullptr;
 }
 
@@ -146,7 +146,7 @@ sect_offset
 attribute::get_ref_die_offset () const
 {
   if (form_is_ref ())
-    return (sect_offset) DW_UNSND (this);
+    return (sect_offset) u.unsnd;
 
   complaint (_("unsupported die ref attribute form: '%s'"),
 	     dwarf_form_name (form));
@@ -159,13 +159,13 @@ LONGEST
 attribute::constant_value (int default_value) const
 {
   if (form == DW_FORM_sdata || form == DW_FORM_implicit_const)
-    return DW_SND (this);
+    return u.snd;
   else if (form == DW_FORM_udata
 	   || form == DW_FORM_data1
 	   || form == DW_FORM_data2
 	   || form == DW_FORM_data4
 	   || form == DW_FORM_data8)
-    return DW_UNSND (this);
+    return u.unsnd;
   else
     {
       /* For DW_FORM_data16 see attribute::form_is_constant.  */
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 04/20] Change some uses of DW_STRING to string method
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (2 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 03/20] Avoid using DW_* macros in dwarf2/attribute.c Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-09-30 15:11   ` Tom de Vries
  2020-04-04 14:43 ` [PATCH v2 05/20] Remove some uses of DW_STRING_IS_CANONICAL Tom Tromey
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes some of the simpler spots to use attribute::string rather
than DW_STRING.

gdb/ChangeLog
2020-04-04  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::as_string.
---
 gdb/ChangeLog     |  7 +++++
 gdb/dwarf2/read.c | 66 ++++++++++++++++++++++++++---------------------
 2 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 0d6ae4478ae..cd5ae3c07f6 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17908,14 +17908,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.as_string ();
 	      break;
 	    default:
 	      {
 		struct objfile *objfile = dwarf2_per_objfile->objfile;
 
-		name
-		  = dwarf2_canonicalize_name (DW_STRING (&attr), cu, objfile);
+		name = dwarf2_canonicalize_name (attr.as_string (),
+						 cu, objfile);
 	      }
 	      break;
 	    }
@@ -20511,7 +20511,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->as_string ();
       break;
     case DW_FORM_block1:
     case DW_FORM_block2:
@@ -20959,21 +20959,22 @@ 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)
+  const char *attr_name = attr->as_string ();
+  if (attr == NULL || attr_name == 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_name, ':');
+  if (base == NULL || base == attr_name || 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_name,
+			  &base[-1] - attr_name);
 }
 
 /* Return the name of the namespace/class that DIE is defined within,
@@ -21243,7 +21244,8 @@ 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))
+  const char *attr_name = attr == nullptr ? nullptr : attr->as_string ();
+  if (attr_name == nullptr
       && die->tag != DW_TAG_namespace
       && die->tag != DW_TAG_class_type
       && die->tag != DW_TAG_interface_type
@@ -21261,11 +21263,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_name;
 
     case DW_TAG_namespace:
-      if (attr != NULL && DW_STRING (attr) != NULL)
-	return DW_STRING (attr);
+      if (attr_name != nullptr)
+	return attr_name;
       return CP_ANONYMOUS_NAMESPACE_STR;
 
     case DW_TAG_class_type:
@@ -21276,25 +21278,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_name != nullptr
+	  && (startswith (attr_name, "._")
+	      || startswith (attr_name, "<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_name == NULL)
 	{
 	  attr = dw2_linkage_name_attr (die, cu);
-	  if (attr == NULL || DW_STRING (attr) == NULL)
+	  if (attr == NULL || attr_name == NULL)
 	    return NULL;
 
-	  /* Avoid demangling DW_STRING (attr) the second time on a second
+	  /* Avoid demangling attr_name 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_name, DMGL_TYPES));
 	      if (demangled == nullptr)
 		return nullptr;
 
@@ -21302,13 +21304,14 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 	      DW_STRING_IS_CANONICAL (attr) = 1;
 	    }
 
-	  /* 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] == ':')
+	  /* 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 (attr_name, ':');
+	  if (base && base > attr_name && base[-1] == ':')
 	    return &base[1];
 	  else
-	    return DW_STRING (attr);
+	    return attr_name;
 	}
       break;
 
@@ -21318,11 +21321,13 @@ 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_name, cu,
 						   objfile);
       DW_STRING_IS_CANONICAL (attr) = 1;
     }
-  return DW_STRING (attr);
+
+  /* We might have changed it just above.  */
+  return attr->as_string ();
 }
 
 /* Return the die that this die in an extension of, or NULL if there
@@ -21830,8 +21835,11 @@ 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));
+      {
+	const char *attr_name = attr->as_string ();
+	result = (const gdb_byte *) attr_name;
+	*len = strlen (attr_name);
+      }
       break;
     case DW_FORM_block1:
     case DW_FORM_block2:
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 05/20] Remove some uses of DW_STRING_IS_CANONICAL
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (3 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 04/20] Change some uses of DW_STRING to string method Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 06/20] Remove DW_STRING and DW_STRING_IS_CANONICAL Tom Tromey
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes the rvalue uses of DW_STRING_IS_CANONICAL, replacing them
with an accessor method.

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

	* dwarf2/read.c (anonymous_struct_prefix, dwarf2_name)
	(dump_die_shallow): Use canonical_string_p.
	* dwarf2/attribute.h (struct attribute) <canonical_string_p>: New
	method.
---
 gdb/ChangeLog          |  7 +++++++
 gdb/dwarf2/attribute.h | 10 ++++++++++
 gdb/dwarf2/read.c      |  8 ++++----
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 861bdb478d1..69f280e94b3 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -100,6 +100,16 @@ struct attribute
 
   LONGEST constant_value (int default_value) const;
 
+  /* Return true if this attribute holds a canonical string.  In some
+     cases, like C++ names, gdb will rewrite the name of a DIE to a
+     canonical form.  This makes lookups robust when a name can be
+     spelled different ways (e.g., "signed" or "signed int").  This
+     flag indicates whether the value has been canonicalized.  */
+  bool canonical_string_p () const
+  {
+    return string_is_canonical;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index cd5ae3c07f6..1aa381b0483 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20964,7 +20964,7 @@ anonymous_struct_prefix (struct die_info *die, struct dwarf2_cu *cu)
     return NULL;
 
   /* dwarf2_name had to be already called.  */
-  gdb_assert (DW_STRING_IS_CANONICAL (attr));
+  gdb_assert (attr->canonical_string_p ());
 
   /* Strip the base name, keep any leading namespaces/classes.  */
   base = strrchr (attr_name, ':');
@@ -21293,7 +21293,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 
 	  /* Avoid demangling attr_name the second time on a second
 	     call for the same DIE.  */
-	  if (!DW_STRING_IS_CANONICAL (attr))
+	  if (!attr->canonical_string_p ())
 	    {
 	      gdb::unique_xmalloc_ptr<char> demangled
 		(gdb_demangle (attr_name, DMGL_TYPES));
@@ -21319,7 +21319,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
       break;
     }
 
-  if (!DW_STRING_IS_CANONICAL (attr))
+  if (!attr->canonical_string_p ())
     {
       DW_STRING (attr) = dwarf2_canonicalize_name (attr_name, cu,
 						   objfile);
@@ -21441,7 +21441,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	  fprintf_unfiltered (f, "string: \"%s\" (%s canonicalized)",
 		   DW_STRING (&die->attrs[i])
 		   ? DW_STRING (&die->attrs[i]) : "",
-		   DW_STRING_IS_CANONICAL (&die->attrs[i]) ? "is" : "not");
+		   die->attrs[i].canonical_string_p () ? "is" : "not");
 	  break;
 	case DW_FORM_flag:
 	  if (DW_UNSND (&die->attrs[i]))
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 06/20] Remove DW_STRING and DW_STRING_IS_CANONICAL
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (4 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 05/20] Remove some uses of DW_STRING_IS_CANONICAL Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 07/20] Remove DW_BLOCK Tom Tromey
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes DW_STRING and DW_STRING_IS_CANONICAL, replacing them with
accessor methods on struct attribute.  The new code ensures that a
string value will only ever be used when the form allows it.

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

	* dwarf2/read.c (read_cutu_die_from_dwo)
	(read_attribute_reprocess, read_attribute_value, read_attribute)
	(dwarf2_const_value_attr, dwarf2_name, dump_die_shallow)
	(dwarf2_fetch_constant_bytes): Update.
	* dwarf2/attribute.h (struct attribute) <form_is_string>: Declare.
	<set_string_noncanonical, set_string_canonical>: New methods.
	<string_is_canonical>: Update comment.
	<canonical_string_p>: Add assert.
	(DW_STRING, DW_STRING_IS_CANONICAL): Remove.
	* dwarf2/attribute.c (attribute::form_is_string): New method.
	(attribute::string): Use it.
---
 gdb/ChangeLog          | 14 ++++++++++
 gdb/dwarf2/attribute.c | 26 ++++++++++++------
 gdb/dwarf2/attribute.h | 29 ++++++++++++++++----
 gdb/dwarf2/read.c      | 62 ++++++++++++++++++------------------------
 4 files changed, 81 insertions(+), 50 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 3acc13ce151..fcf36fc15f2 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -61,18 +61,26 @@ attribute::as_address () const
 
 /* See attribute.h.  */
 
+bool
+attribute::form_is_string () const
+{
+  return (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);
+}
+
+/* See attribute.h.  */
+
 const char *
 attribute::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)
+  if (form_is_string ())
     return u.str;
   return nullptr;
 }
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 69f280e94b3..53d6cafb407 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -89,6 +89,9 @@ struct attribute
 
   bool form_is_block () const;
 
+  /* Check if the attribute's form is a string form.  */
+  bool form_is_string () const;
+
   /* Return DIE offset of this attribute.  Return 0 with complaint if
      the attribute is not of the required kind.  */
 
@@ -107,16 +110,34 @@ struct attribute
      flag indicates whether the value has been canonicalized.  */
   bool canonical_string_p () const
   {
+    gdb_assert (form_is_string ());
     return string_is_canonical;
   }
 
+  /* Initialize this attribute to hold a non-canonical string
+     value.  */
+  void set_string_noncanonical (const char *str)
+  {
+    gdb_assert (form_is_string ());
+    u.str = str;
+    string_is_canonical = 0;
+  }
+
+  /* Set the canonical string value for this attribute.  */
+  void set_string_canonical (const char *str)
+  {
+    gdb_assert (form_is_string ());
+    u.str = str;
+    string_is_canonical = 1;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
 
-  /* Has DW_STRING already been updated by dwarf2_canonicalize_name?  This
-     field should be in u.str (existing only for DW_STRING) but it is kept
-     here for better struct attribute alignment.  */
+  /* Has u.str already been updated by dwarf2_canonicalize_name?  This
+     field should be in u.str but it is kept here for better struct
+     attribute alignment.  */
   unsigned int string_is_canonical : 1;
 
   union
@@ -133,8 +154,6 @@ struct attribute
 
 /* Get at parts of an attribute structure.  */
 
-#define DW_STRING(attr)    ((attr)->u.str)
-#define DW_STRING_IS_CANONICAL(attr) ((attr)->string_is_canonical)
 #define DW_UNSND(attr)     ((attr)->u.unsnd)
 #define DW_BLOCK(attr)     ((attr)->u.blk)
 #define DW_SND(attr)       ((attr)->u.snd)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1aa381b0483..179fc3481fd 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6502,8 +6502,7 @@ read_cutu_die_from_dwo (struct dwarf2_per_cu_data *this_cu,
       comp_dir = XOBNEW (&cu->comp_unit_obstack, struct attribute);
       comp_dir->name = DW_AT_comp_dir;
       comp_dir->form = DW_FORM_string;
-      DW_STRING_IS_CANONICAL (comp_dir) = 0;
-      DW_STRING (comp_dir) = stub_comp_dir;
+      comp_dir->set_string_noncanonical (stub_comp_dir);
     }
 
   /* Set up for reading the DWO CU/TU.  */
@@ -18350,16 +18349,13 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_GNU_str_index:
 	{
 	  unsigned int str_index = DW_UNSND (attr);
+	  gdb_assert (!attr->canonical_string_p ());
 	  if (reader->dwo_file != NULL)
-	    {
-	      DW_STRING (attr) = read_dwo_str_index (reader, str_index);
-	      DW_STRING_IS_CANONICAL (attr) = 0;
-	    }
+	    attr->set_string_noncanonical (read_dwo_str_index (reader,
+							       str_index));
 	  else
-	    {
-	      DW_STRING (attr) = read_stub_str_index (cu, str_index);
-	      DW_STRING_IS_CANONICAL (attr) = 0;
-	    }
+	    attr->set_string_noncanonical (read_stub_str_index (cu,
+								str_index));
 	  break;
 	}
       default:
@@ -18447,17 +18443,17 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += bytes_read;
       break;
     case DW_FORM_string:
-      DW_STRING (attr) = read_direct_string (abfd, info_ptr, &bytes_read);
-      DW_STRING_IS_CANONICAL (attr) = 0;
+      attr->set_string_noncanonical (read_direct_string (abfd, info_ptr,
+							 &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_strp:
       if (!cu->per_cu->is_dwz)
 	{
-	  DW_STRING (attr) = read_indirect_string (dwarf2_per_objfile,
-						   abfd, info_ptr, cu_header,
-						   &bytes_read);
-	  DW_STRING_IS_CANONICAL (attr) = 0;
+	  attr->set_string_noncanonical
+	    (read_indirect_string (dwarf2_per_objfile,
+				   abfd, info_ptr, cu_header,
+				   &bytes_read));
 	  info_ptr += bytes_read;
 	  break;
 	}
@@ -18465,10 +18461,9 @@ read_attribute_value (const struct die_reader_specs *reader,
     case DW_FORM_line_strp:
       if (!cu->per_cu->is_dwz)
 	{
-	  DW_STRING (attr)
-	    = dwarf2_per_objfile->read_line_string (info_ptr, cu_header,
-						    &bytes_read);
-	  DW_STRING_IS_CANONICAL (attr) = 0;
+	  attr->set_string_noncanonical
+	    (dwarf2_per_objfile->read_line_string (info_ptr, cu_header,
+						   &bytes_read));
 	  info_ptr += bytes_read;
 	  break;
 	}
@@ -18479,8 +18474,8 @@ read_attribute_value (const struct die_reader_specs *reader,
 	LONGEST str_offset = cu_header->read_offset (abfd, info_ptr,
 						     &bytes_read);
 
-	DW_STRING (attr) = dwz->read_string (objfile, str_offset);
-	DW_STRING_IS_CANONICAL (attr) = 0;
+	attr->set_string_noncanonical
+	  (dwz->read_string (objfile, str_offset));
 	info_ptr += bytes_read;
       }
       break;
@@ -18644,6 +18639,7 @@ read_attribute (const struct die_reader_specs *reader,
 		const gdb_byte *info_ptr, bool *need_reprocess)
 {
   attr->name = abbrev->name;
+  attr->string_is_canonical = 0;
   return read_attribute_value (reader, attr, abbrev->form,
 			       abbrev->implicit_const, info_ptr,
 			       need_reprocess);
@@ -20509,7 +20505,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
     case DW_FORM_strx:
     case DW_FORM_GNU_str_index:
     case DW_FORM_GNU_strp_alt:
-      /* DW_STRING is already allocated on the objfile obstack, point
+      /* The string is already allocated on the objfile obstack, point
 	 directly to it.  */
       *bytes = (const gdb_byte *) attr->as_string ();
       break;
@@ -21300,8 +21296,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 	      if (demangled == nullptr)
 		return nullptr;
 
-	      DW_STRING (attr) = objfile->intern (demangled.get ());
-	      DW_STRING_IS_CANONICAL (attr) = 1;
+	      attr->set_string_canonical (objfile->intern (demangled.get ()));
 	    }
 
 	  /* Strip any leading namespaces/classes, keep only the
@@ -21320,13 +21315,8 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
     }
 
   if (!attr->canonical_string_p ())
-    {
-      DW_STRING (attr) = dwarf2_canonicalize_name (attr_name, cu,
-						   objfile);
-      DW_STRING_IS_CANONICAL (attr) = 1;
-    }
-
-  /* We might have changed it just above.  */
+    attr->set_string_canonical (dwarf2_canonicalize_name (attr_name, cu,
+							  objfile));
   return attr->as_string ();
 }
 
@@ -21439,9 +21429,9 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_GNU_str_index:
 	case DW_FORM_GNU_strp_alt:
 	  fprintf_unfiltered (f, "string: \"%s\" (%s canonicalized)",
-		   DW_STRING (&die->attrs[i])
-		   ? DW_STRING (&die->attrs[i]) : "",
-		   die->attrs[i].canonical_string_p () ? "is" : "not");
+			      die->attrs[i].as_string ()
+			      ? die->attrs[i].as_string () : "",
+			      die->attrs[i].canonical_string_p () ? "is" : "not");
 	  break;
 	case DW_FORM_flag:
 	  if (DW_UNSND (&die->attrs[i]))
@@ -21833,7 +21823,7 @@ dwarf2_fetch_constant_bytes (sect_offset sect_off,
     case DW_FORM_strx:
     case DW_FORM_GNU_str_index:
     case DW_FORM_GNU_strp_alt:
-      /* DW_STRING is already allocated on the objfile obstack, point
+      /* The string is already allocated on the objfile obstack, point
 	 directly to it.  */
       {
 	const char *attr_name = attr->as_string ();
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 07/20] Remove DW_BLOCK
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (5 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 06/20] Remove DW_STRING and DW_STRING_IS_CANONICAL Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 08/20] Remove DW_SIGNATURE Tom Tromey
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes the DW_BLOCK accessor in favor of methods on struct
attribute.  The methods, unlike the access, check the form.

Note that DW_FORM_data16 had to be handled by form_is_block, because
in practice that is how we store values of this form.

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

	* dwarf2/read.c (read_call_site_scope)
	(handle_data_member_location, dwarf2_add_member_fn)
	(mark_common_block_symbol_computed, attr_to_dynamic_prop)
	(partial_die_info::read, read_attribute_value)
	(var_decode_location, dwarf2_const_value_attr, dump_die_shallow)
	(dwarf2_fetch_die_loc_sect_off, dwarf2_fetch_constant_bytes)
	(dwarf2_symbol_mark_computed): Update.
	* dwarf2/attribute.h (struct attribute) <as_block, set_block>: New
	methods.
	(DW_BLOCK): Remove.
	* dwarf2/attribute.c (attribute::form_is_block): Add
	DW_FORM_data16.
---
 gdb/ChangeLog          |  15 ++++
 gdb/dwarf2/attribute.c |   3 +-
 gdb/dwarf2/attribute.h |  15 +++-
 gdb/dwarf2/read.c      | 155 +++++++++++++++++++++++------------------
 4 files changed, 119 insertions(+), 69 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index fcf36fc15f2..bacbd087b8a 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -94,7 +94,8 @@ attribute::form_is_block () const
 	  || form == DW_FORM_block2
 	  || form == DW_FORM_block4
 	  || form == DW_FORM_block
-	  || form == DW_FORM_exprloc);
+	  || form == DW_FORM_exprloc
+	  || form == DW_FORM_data16);
 }
 
 /* See attribute.h.  */
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 53d6cafb407..c52212f7b00 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -50,6 +50,13 @@ struct attribute
      otherwise return NULL.  */
   const char *as_string () const;
 
+  /* Return the block value.  The attribute must have block form.  */
+  dwarf_block *as_block () const
+  {
+    gdb_assert (form_is_block ());
+    return u.blk;
+  }
+
   /* 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.
@@ -131,6 +138,13 @@ struct attribute
     string_is_canonical = 1;
   }
 
+  /* Set the block value for this attribute.  */
+  void set_block (dwarf_block *blk)
+  {
+    gdb_assert (form_is_block ());
+    u.blk = blk;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
@@ -155,7 +169,6 @@ struct attribute
 /* Get at parts of an attribute structure.  */
 
 #define DW_UNSND(attr)     ((attr)->u.unsnd)
-#define DW_BLOCK(attr)     ((attr)->u.blk)
 #define DW_SND(attr)       ((attr)->u.snd)
 #define DW_ADDR(attr)	   ((attr)->u.addr)
 #define DW_SIGNATURE(attr) ((attr)->u.signature)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 179fc3481fd..1de38ef6437 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -13155,15 +13155,16 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       attr = dwarf2_attr (die, DW_AT_abstract_origin, cu);
     }
   SET_FIELD_DWARF_BLOCK (call_site->target, NULL);
-  if (!attr || (attr->form_is_block () && DW_BLOCK (attr)->size == 0))
+  if (!attr || (attr->form_is_block () && attr->as_block ()->size == 0))
     /* Keep NULL DWARF_BLOCK.  */;
   else if (attr->form_is_block ())
     {
       struct dwarf2_locexpr_baton *dlbaton;
+      struct dwarf_block *block = attr->as_block ();
 
       dlbaton = XOBNEW (&objfile->objfile_obstack, struct dwarf2_locexpr_baton);
-      dlbaton->data = DW_BLOCK (attr)->data;
-      dlbaton->size = DW_BLOCK (attr)->size;
+      dlbaton->data = block->data;
+      dlbaton->size = block->size;
       dlbaton->per_cu = cu->per_cu;
 
       SET_FIELD_DWARF_BLOCK (call_site->target, dlbaton);
@@ -13271,12 +13272,14 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 	}
       else
 	{
+	  struct dwarf_block *block = loc->as_block ();
+
 	  parameter->u.dwarf_reg = dwarf_block_to_dwarf_reg
-	    (DW_BLOCK (loc)->data, &DW_BLOCK (loc)->data[DW_BLOCK (loc)->size]);
+	    (block->data, &block->data[block->size]);
 	  if (parameter->u.dwarf_reg != -1)
 	    parameter->kind = CALL_SITE_PARAMETER_DWARF_REG;
-	  else if (dwarf_block_to_sp_offset (gdbarch, DW_BLOCK (loc)->data,
-				    &DW_BLOCK (loc)->data[DW_BLOCK (loc)->size],
+	  else if (dwarf_block_to_sp_offset (gdbarch, block->data,
+				    &block->data[block->size],
 					     &parameter->u.fb_offset))
 	    parameter->kind = CALL_SITE_PARAMETER_FB_OFFSET;
 	  else
@@ -13302,8 +13305,10 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		     objfile_name (objfile));
 	  continue;
 	}
-      parameter->value = DW_BLOCK (attr)->data;
-      parameter->value_size = DW_BLOCK (attr)->size;
+
+      struct dwarf_block *block = attr->as_block ();
+      parameter->value = block->data;
+      parameter->value_size = block->size;
 
       /* Parameters are not pre-cleared by memset above.  */
       parameter->data_value = NULL;
@@ -13322,8 +13327,9 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		       objfile_name (objfile));
 	  else
 	    {
-	      parameter->data_value = DW_BLOCK (attr)->data;
-	      parameter->data_value_size = DW_BLOCK (attr)->size;
+	      block = attr->as_block ();
+	      parameter->data_value = block->data;
+	      parameter->data_value_size = block->size;
 	    }
 	}
     }
@@ -14085,7 +14091,7 @@ handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
       else if (attr->form_is_section_offset ())
 	dwarf2_complex_location_expr_complaint ();
       else if (attr->form_is_block ())
-	*offset = decode_locdesc (DW_BLOCK (attr), cu);
+	*offset = decode_locdesc (attr->as_block (), cu);
       else
 	dwarf2_complex_location_expr_complaint ();
 
@@ -14660,19 +14666,21 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
   attr = dwarf2_attr (die, DW_AT_vtable_elem_location, cu);
   if (attr != nullptr)
     {
-      if (attr->form_is_block () && DW_BLOCK (attr)->size > 0)
+      if (attr->form_is_block () && attr->as_block ()->size > 0)
         {
-	  if (DW_BLOCK (attr)->data[0] == DW_OP_constu)
+	  struct dwarf_block *block = attr->as_block ();
+
+	  if (block->data[0] == DW_OP_constu)
 	    {
 	      /* Old-style GCC.  */
-	      fnp->voffset = decode_locdesc (DW_BLOCK (attr), cu) + 2;
+	      fnp->voffset = decode_locdesc (block, cu) + 2;
 	    }
-	  else if (DW_BLOCK (attr)->data[0] == DW_OP_deref
-		   || (DW_BLOCK (attr)->size > 1
-		       && DW_BLOCK (attr)->data[0] == DW_OP_deref_size
-		       && DW_BLOCK (attr)->data[1] == cu->header.addr_size))
+	  else if (block->data[0] == DW_OP_deref
+		   || (block->size > 1
+		       && block->data[0] == DW_OP_deref_size
+		       && block->data[1] == cu->header.addr_size))
 	    {
-	      fnp->voffset = decode_locdesc (DW_BLOCK (attr), cu);
+	      fnp->voffset = decode_locdesc (block, cu);
 	      if ((fnp->voffset % cu->header.addr_size) != 0)
 		dwarf2_complex_location_expr_complaint ();
 	      else
@@ -15888,7 +15896,7 @@ mark_common_block_symbol_computed (struct symbol *sym,
       baton->size += 1 /* DW_OP_addr */ + cu->header.addr_size;
     }
   else
-    baton->size += DW_BLOCK (member_loc)->size;
+    baton->size += member_loc->as_block ()->size;
 
   ptr = (gdb_byte *) obstack_alloc (&objfile->objfile_obstack, baton->size);
   baton->data = ptr;
@@ -15908,8 +15916,9 @@ mark_common_block_symbol_computed (struct symbol *sym,
     {
       /* We have to copy the data here, because DW_OP_call4 will only
 	 use a DW_AT_location attribute.  */
-      memcpy (ptr, DW_BLOCK (member_loc)->data, DW_BLOCK (member_loc)->size);
-      ptr += DW_BLOCK (member_loc)->size;
+      struct dwarf_block *block = member_loc->as_block ();
+      memcpy (ptr, block->data, block->size);
+      ptr += block->size;
     }
 
   *ptr++ = DW_OP_plus;
@@ -17001,8 +17010,10 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
       baton = XOBNEW (obstack, struct dwarf2_property_baton);
       baton->property_type = default_type;
       baton->locexpr.per_cu = cu->per_cu;
-      baton->locexpr.size = DW_BLOCK (attr)->size;
-      baton->locexpr.data = DW_BLOCK (attr)->data;
+
+      struct dwarf_block *block = attr->as_block ();
+      baton->locexpr.size = block->size;
+      baton->locexpr.data = block->data;
       switch (attr->name)
 	{
 	case DW_AT_string_length:
@@ -17047,8 +17058,9 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 		baton = XOBNEW (obstack, struct dwarf2_property_baton);
 		baton->property_type = die_type (target_die, target_cu);
 		baton->locexpr.per_cu = cu->per_cu;
-		baton->locexpr.size = DW_BLOCK (target_attr)->size;
-		baton->locexpr.data = DW_BLOCK (target_attr)->data;
+		struct dwarf_block *block = target_attr->as_block ();
+		baton->locexpr.size = block->size;
+		baton->locexpr.data = block->data;
 		baton->locexpr.is_reference = true;
 		prop->data.baton = baton;
 		prop->kind = PROP_LOCEXPR;
@@ -17940,7 +17952,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
           /* Support the .debug_loc offsets.  */
           if (attr.form_is_block ())
             {
-	       d.locdesc = DW_BLOCK (&attr);
+	      d.locdesc = attr.as_block ();
             }
           else if (attr.form_is_section_offset ())
             {
@@ -18409,7 +18421,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += 2;
       blk->data = read_n_bytes (abfd, info_ptr, blk->size);
       info_ptr += blk->size;
-      DW_BLOCK (attr) = blk;
+      attr->set_block (blk);
       break;
     case DW_FORM_block4:
       blk = dwarf_alloc_block (cu);
@@ -18417,7 +18429,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += 4;
       blk->data = read_n_bytes (abfd, info_ptr, blk->size);
       info_ptr += blk->size;
-      DW_BLOCK (attr) = blk;
+      attr->set_block (blk);
       break;
     case DW_FORM_data2:
       DW_UNSND (attr) = read_2_bytes (abfd, info_ptr);
@@ -18436,7 +18448,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       blk->size = 16;
       blk->data = read_n_bytes (abfd, info_ptr, 16);
       info_ptr += 16;
-      DW_BLOCK (attr) = blk;
+      attr->set_block (blk);
       break;
     case DW_FORM_sec_offset:
       DW_UNSND (attr) = cu->header.read_offset (abfd, info_ptr, &bytes_read);
@@ -18486,7 +18498,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += bytes_read;
       blk->data = read_n_bytes (abfd, info_ptr, blk->size);
       info_ptr += blk->size;
-      DW_BLOCK (attr) = blk;
+      attr->set_block (blk);
       break;
     case DW_FORM_block1:
       blk = dwarf_alloc_block (cu);
@@ -18494,7 +18506,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += 1;
       blk->data = read_n_bytes (abfd, info_ptr, blk->size);
       info_ptr += blk->size;
-      DW_BLOCK (attr) = blk;
+      attr->set_block (blk);
       break;
     case DW_FORM_data1:
       DW_UNSND (attr) = read_1_byte (abfd, info_ptr);
@@ -19954,7 +19966,7 @@ var_decode_location (struct attribute *attr, struct symbol *sym,
 
   /* A DW_AT_location attribute with no contents indicates that a
      variable has been optimized away.  */
-  if (attr->form_is_block () && DW_BLOCK (attr)->size == 0)
+  if (attr->form_is_block () && attr->as_block ()->size == 0)
     {
       SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
       return;
@@ -19965,32 +19977,36 @@ var_decode_location (struct attribute *attr, struct symbol *sym,
      specified.  If this is just a DW_OP_addr, DW_OP_addrx, or
      DW_OP_GNU_addr_index then mark this symbol as LOC_STATIC.  */
 
-  if (attr->form_is_block ()
-      && ((DW_BLOCK (attr)->data[0] == DW_OP_addr
-	   && DW_BLOCK (attr)->size == 1 + cu_header->addr_size)
-	  || ((DW_BLOCK (attr)->data[0] == DW_OP_GNU_addr_index
-               || DW_BLOCK (attr)->data[0] == DW_OP_addrx)
-	      && (DW_BLOCK (attr)->size
-		  == 1 + leb128_size (&DW_BLOCK (attr)->data[1])))))
-    {
-      unsigned int dummy;
-
-      if (DW_BLOCK (attr)->data[0] == DW_OP_addr)
-	SET_SYMBOL_VALUE_ADDRESS
-	  (sym, cu->header.read_address (objfile->obfd,
-					 DW_BLOCK (attr)->data + 1,
-					 &dummy));
-      else
-	SET_SYMBOL_VALUE_ADDRESS
-	  (sym, read_addr_index_from_leb128 (cu, DW_BLOCK (attr)->data + 1,
+  if (attr->form_is_block ())
+    {
+      struct dwarf_block *block = attr->as_block ();
+
+      if ((block->data[0] == DW_OP_addr
+	   && block->size == 1 + cu_header->addr_size)
+	  || ((block->data[0] == DW_OP_GNU_addr_index
+               || block->data[0] == DW_OP_addrx)
+	      && (block->size
+		  == 1 + leb128_size (&block->data[1]))))
+	{
+	  unsigned int dummy;
+
+	  if (block->data[0] == DW_OP_addr)
+	    SET_SYMBOL_VALUE_ADDRESS
+	      (sym, cu->header.read_address (objfile->obfd,
+					     block->data + 1,
 					     &dummy));
-      SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
-      fixup_symbol_section (sym, objfile);
-      SET_SYMBOL_VALUE_ADDRESS
-	(sym,
-	 SYMBOL_VALUE_ADDRESS (sym)
-	 + objfile->section_offsets[SYMBOL_SECTION (sym)]);
-      return;
+	  else
+	    SET_SYMBOL_VALUE_ADDRESS
+	      (sym, read_addr_index_from_leb128 (cu, block->data + 1,
+						 &dummy));
+	  SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
+	  fixup_symbol_section (sym, objfile);
+	  SET_SYMBOL_VALUE_ADDRESS
+	    (sym,
+	     SYMBOL_VALUE_ADDRESS (sym)
+	     + objfile->section_offsets[SYMBOL_SECTION (sym)]);
+	  return;
+	}
     }
 
   /* NOTE drow/2002-01-30: It might be worthwhile to have a static
@@ -20515,7 +20531,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
     case DW_FORM_block:
     case DW_FORM_exprloc:
     case DW_FORM_data16:
-      blk = DW_BLOCK (attr);
+      blk = attr->as_block ();
       if (TYPE_LENGTH (type) != blk->size)
 	dwarf2_const_value_length_mismatch_complaint (name, blk->size,
 						      TYPE_LENGTH (type));
@@ -21380,11 +21396,11 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_block:
 	case DW_FORM_block1:
 	  fprintf_unfiltered (f, "block: size %s",
-			      pulongest (DW_BLOCK (&die->attrs[i])->size));
+			      pulongest (die->attrs[i].as_block ()->size));
 	  break;
 	case DW_FORM_exprloc:
 	  fprintf_unfiltered (f, "expression: size %s",
-			      pulongest (DW_BLOCK (&die->attrs[i])->size));
+			      pulongest (die->attrs[i].as_block ()->size));
 	  break;
 	case DW_FORM_data16:
 	  fprintf_unfiltered (f, "constant of 16 bytes");
@@ -21722,8 +21738,9 @@ dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
 		 "is neither DW_FORM_block* nor DW_FORM_exprloc"),
 	       sect_offset_str (sect_off), objfile_name (objfile));
 
-      retval.data = DW_BLOCK (attr)->data;
-      retval.size = DW_BLOCK (attr)->size;
+      struct dwarf_block *block = attr->as_block ();
+      retval.data = block->data;
+      retval.size = block->size;
     }
   retval.per_cu = cu->per_cu;
 
@@ -21837,8 +21854,11 @@ dwarf2_fetch_constant_bytes (sect_offset sect_off,
     case DW_FORM_block:
     case DW_FORM_exprloc:
     case DW_FORM_data16:
-      result = DW_BLOCK (attr)->data;
-      *len = DW_BLOCK (attr)->size;
+      {
+	struct dwarf_block *block = attr->as_block ();
+	result = block->data;
+	*len = block->size;
+      }
       break;
 
       /* The DW_AT_const_value attributes are supposed to carry the
@@ -22618,8 +22638,9 @@ dwarf2_symbol_mark_computed (const struct attribute *attr, struct symbol *sym,
 	     info_buffer for SYM's objfile; right now we never release
 	     that buffer, but when we do clean up properly this may
 	     need to change.  */
-	  baton->size = DW_BLOCK (attr)->size;
-	  baton->data = DW_BLOCK (attr)->data;
+	  struct dwarf_block *block = attr->as_block ();
+	  baton->size = block->size;
+	  baton->data = block->data;
 	}
       else
 	{
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 08/20] Remove DW_SIGNATURE
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (6 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 07/20] Remove DW_BLOCK Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 09/20] Remove DW_SND Tom Tromey
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes DW_SIGNATURE in favor of methods on struct attribute.  As
usual, the methods check the form, which DW_SIGNATURE did not do.

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

	* dwarf2/read.c (read_attribute_value, lookup_die_type)
	(dump_die_shallow, follow_die_sig, get_DW_AT_signature_type):
	Update.
	* dwarf2/attribute.h (struct attribute) <as_signature,
	set_signature>: New methods.
	(DW_SIGNATURE): Remove.
---
 gdb/ChangeLog          |  9 +++++++++
 gdb/dwarf2/attribute.h | 16 +++++++++++++++-
 gdb/dwarf2/read.c      | 10 +++++-----
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index c52212f7b00..4a77e856669 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -57,6 +57,14 @@ struct attribute
     return u.blk;
   }
 
+  /* Return the signature.  The attribute must have signature
+     form.  */
+  ULONGEST as_signature () const
+  {
+    gdb_assert (form == DW_FORM_ref_sig8);
+    return u.signature;
+  }
+
   /* 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.
@@ -145,6 +153,13 @@ struct attribute
     u.blk = blk;
   }
 
+  /* Set the signature value for this attribute.  */
+  void set_signature (ULONGEST signature)
+  {
+    gdb_assert (form == DW_FORM_ref_sig8);
+    u.signature = signature;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
@@ -171,6 +186,5 @@ struct attribute
 #define DW_UNSND(attr)     ((attr)->u.unsnd)
 #define DW_SND(attr)       ((attr)->u.snd)
 #define DW_ADDR(attr)	   ((attr)->u.addr)
-#define DW_SIGNATURE(attr) ((attr)->u.signature)
 
 #endif /* GDB_DWARF2_ATTRIBUTE_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1de38ef6437..1f4c7b01bcd 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18549,7 +18549,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += 8;
       break;
     case DW_FORM_ref_sig8:
-      DW_SIGNATURE (attr) = read_8_bytes (abfd, info_ptr);
+      attr->set_signature (read_8_bytes (abfd, info_ptr));
       info_ptr += 8;
       break;
     case DW_FORM_ref_udata:
@@ -20743,7 +20743,7 @@ lookup_die_type (struct die_info *die, const struct attribute *attr,
     }
   else if (attr->form == DW_FORM_ref_sig8)
     {
-      ULONGEST signature = DW_SIGNATURE (attr);
+      ULONGEST signature = attr->as_signature ();
 
       return get_signatured_type (die, signature, cu);
     }
@@ -21436,7 +21436,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	  break;
 	case DW_FORM_ref_sig8:
 	  fprintf_unfiltered (f, "signature: %s",
-			      hex_string (DW_SIGNATURE (&die->attrs[i])));
+			      hex_string (die->attrs[i].as_signature ()));
 	  break;
 	case DW_FORM_string:
 	case DW_FORM_strp:
@@ -22010,7 +22010,7 @@ static struct die_info *
 follow_die_sig (struct die_info *src_die, const struct attribute *attr,
 		struct dwarf2_cu **ref_cu)
 {
-  ULONGEST signature = DW_SIGNATURE (attr);
+  ULONGEST signature = attr->as_signature ();
   struct signatured_type *sig_type;
   struct die_info *die;
 
@@ -22117,7 +22117,7 @@ get_DW_AT_signature_type (struct die_info *die, const struct attribute *attr,
     }
   else if (attr->form == DW_FORM_ref_sig8)
     {
-      return get_signatured_type (die, DW_SIGNATURE (attr), cu);
+      return get_signatured_type (die, attr->as_signature (), cu);
     }
   else
     {
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 09/20] Remove DW_SND
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (7 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 08/20] Remove DW_SIGNATURE Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 10/20] Use setter for attribute's unsigned value Tom Tromey
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes DW_SND in favor of accessors on struct attribute.
These accessors check that the form is appropriate.

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

	* dwarf2/read.c (get_alignment, read_array_order)
	(read_attribute_value, dwarf2_const_value_attr)
	(dump_die_shallow, dwarf2_fetch_constant_bytes): Update.
	* dwarf2/attribute.h (struct attribute) <as_signed, set_signed>:
	New methods.
	(DW_SND): Remove.
---
 gdb/ChangeLog          |  9 +++++++++
 gdb/dwarf2/attribute.h | 16 +++++++++++++++-
 gdb/dwarf2/read.c      | 20 ++++++++++++--------
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 4a77e856669..b40cb369030 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -65,6 +65,14 @@ struct attribute
     return u.signature;
   }
 
+  /* Return the signed value.  The attribute must have the appropriate
+     form.  */
+  LONGEST as_signed () const
+  {
+    gdb_assert (form == DW_FORM_sdata || form == DW_FORM_implicit_const);
+    return u.snd;
+  }
+
   /* 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.
@@ -160,6 +168,13 @@ struct attribute
     u.signature = signature;
   }
 
+  /* Set this attribute to a signed integer.  */
+  void set_signed (LONGEST snd)
+  {
+    gdb_assert (form == DW_FORM_sdata || form == DW_FORM_implicit_const);
+    u.snd = snd;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
@@ -184,7 +199,6 @@ struct attribute
 /* Get at parts of an attribute structure.  */
 
 #define DW_UNSND(attr)     ((attr)->u.unsnd)
-#define DW_SND(attr)       ((attr)->u.snd)
 #define DW_ADDR(attr)	   ((attr)->u.addr)
 
 #endif /* GDB_DWARF2_ATTRIBUTE_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1f4c7b01bcd..aedac3fc006 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14848,7 +14848,7 @@ get_alignment (struct dwarf2_cu *cu, struct die_info *die)
   ULONGEST align;
   if (attr->form == DW_FORM_sdata)
     {
-      LONGEST val = DW_SND (attr);
+      LONGEST val = attr->as_signed ();
       if (val < 0)
 	{
 	  complaint (_("DW_AT_alignment value must not be negative"
@@ -15802,7 +15802,11 @@ read_array_order (struct die_info *die, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_ordering, cu);
 
   if (attr != nullptr)
-    return (enum dwarf_array_dim_ordering) DW_SND (attr);
+    {
+      LONGEST val = attr->constant_value (-1);
+      if (val == DW_ORD_row_major || val == DW_ORD_col_major)
+	return (enum dwarf_array_dim_ordering) val;
+    }
 
   /* GNU F77 is a special case, as at 08/2004 array type info is the
      opposite order to the dwarf2 specification, but data is still
@@ -18520,7 +18524,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       DW_UNSND (attr) = 1;
       break;
     case DW_FORM_sdata:
-      DW_SND (attr) = read_signed_leb128 (abfd, info_ptr, &bytes_read);
+      attr->set_signed (read_signed_leb128 (abfd, info_ptr, &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_udata:
@@ -18569,7 +18573,7 @@ read_attribute_value (const struct die_reader_specs *reader,
 				       info_ptr, need_reprocess);
       break;
     case DW_FORM_implicit_const:
-      DW_SND (attr) = implicit_const;
+      attr->set_signed (implicit_const);
       break;
     case DW_FORM_addrx:
     case DW_FORM_GNU_addr_index:
@@ -20558,7 +20562,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
 
     case DW_FORM_sdata:
     case DW_FORM_implicit_const:
-      *value = DW_SND (attr);
+      *value = attr->as_signed ();
       break;
 
     case DW_FORM_udata:
@@ -21426,7 +21430,6 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_data4:
 	case DW_FORM_data8:
 	case DW_FORM_udata:
-	case DW_FORM_sdata:
 	  fprintf_unfiltered (f, "constant: %s",
 			      pulongest (DW_UNSND (&die->attrs[i])));
 	  break;
@@ -21464,9 +21467,10 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	  fprintf_unfiltered (f,
 			      "unexpected attribute form: DW_FORM_indirect");
 	  break;
+	case DW_FORM_sdata:
 	case DW_FORM_implicit_const:
 	  fprintf_unfiltered (f, "constant: %s",
-			      plongest (DW_SND (&die->attrs[i])));
+			      plongest (die->attrs[i].as_signed ()));
 	  break;
 	default:
 	  fprintf_unfiltered (f, "unsupported attribute form: %d.",
@@ -21899,7 +21903,7 @@ dwarf2_fetch_constant_bytes (sect_offset sect_off,
     case DW_FORM_implicit_const:
       type = die_type (die, cu);
       result = write_constant_as_bytes (obstack, byte_order,
-					type, DW_SND (attr), len);
+					type, attr->as_signed (), len);
       break;
 
     case DW_FORM_udata:
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 10/20] Use setter for attribute's unsigned value
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (8 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 09/20] Remove DW_SND Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 11/20] Add reprocessing flag to struct attribute Tom Tromey
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds form_is_unsigned and an unsigned setter method to struct
attribute, and updates the remaining code.  Now DW_UNSND is no longer
used as an lvalue.

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

	* dwarf2/read.c (read_attribute_value): Update.
	* dwarf2/attribute.h (struct attribute) <form_is_unsigned,
	set_unsigned>: New methods.
	* dwarf2/attribute.c (attribute::form_is_unsigned): New method.
---
 gdb/ChangeLog          |  7 ++++++
 gdb/dwarf2/attribute.c | 23 +++++++++++++++++++
 gdb/dwarf2/attribute.h | 10 ++++++++
 gdb/dwarf2/read.c      | 52 +++++++++++++++++++++---------------------
 4 files changed, 66 insertions(+), 26 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index bacbd087b8a..092a09295ba 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -183,3 +183,26 @@ attribute::constant_value (int default_value) const
       return default_value;
     }
 }
+
+/* See attribute.h.  */
+
+bool
+attribute::form_is_unsigned () const
+{
+  return (form == DW_FORM_ref_addr
+	  || form == DW_FORM_GNU_ref_alt
+	  || form == DW_FORM_data2
+	  || form == DW_FORM_data4
+	  || form == DW_FORM_data8
+	  || form == DW_FORM_sec_offset
+	  || form == DW_FORM_data1
+	  || form == DW_FORM_flag
+	  || form == DW_FORM_flag_present
+	  || form == DW_FORM_udata
+	  || form == DW_FORM_rnglistx
+	  || form == DW_FORM_ref1
+	  || form == DW_FORM_ref2
+	  || form == DW_FORM_ref4
+	  || form == DW_FORM_ref8
+	  || form == DW_FORM_ref_udata);
+}
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index b40cb369030..2617cab7188 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -115,6 +115,9 @@ struct attribute
   /* Check if the attribute's form is a string form.  */
   bool form_is_string () const;
 
+  /* Check if the attribute's form is an unsigned integer form.  */
+  bool form_is_unsigned () const;
+
   /* Return DIE offset of this attribute.  Return 0 with complaint if
      the attribute is not of the required kind.  */
 
@@ -175,6 +178,13 @@ struct attribute
     u.snd = snd;
   }
 
+  /* Set this attribute to an unsigned integer.  */
+  void set_unsigned (ULONGEST unsnd)
+  {
+    gdb_assert (form_is_unsigned ());
+    u.unsnd = unsnd;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index aedac3fc006..3539fb0136e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18403,15 +18403,16 @@ read_attribute_value (const struct die_reader_specs *reader,
     {
     case DW_FORM_ref_addr:
       if (cu->header.version == 2)
-	DW_UNSND (attr) = cu->header.read_address (abfd, info_ptr,
-						   &bytes_read);
+	attr->set_unsigned (cu->header.read_address (abfd, info_ptr,
+						     &bytes_read));
       else
-	DW_UNSND (attr) = cu->header.read_offset (abfd, info_ptr,
-						  &bytes_read);
+	attr->set_unsigned (cu->header.read_offset (abfd, info_ptr,
+						    &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_GNU_ref_alt:
-      DW_UNSND (attr) = cu->header.read_offset (abfd, info_ptr, &bytes_read);
+      attr->set_unsigned (cu->header.read_offset (abfd, info_ptr,
+						  &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_addr:
@@ -18436,15 +18437,15 @@ read_attribute_value (const struct die_reader_specs *reader,
       attr->set_block (blk);
       break;
     case DW_FORM_data2:
-      DW_UNSND (attr) = read_2_bytes (abfd, info_ptr);
+      attr->set_unsigned (read_2_bytes (abfd, info_ptr));
       info_ptr += 2;
       break;
     case DW_FORM_data4:
-      DW_UNSND (attr) = read_4_bytes (abfd, info_ptr);
+      attr->set_unsigned (read_4_bytes (abfd, info_ptr));
       info_ptr += 4;
       break;
     case DW_FORM_data8:
-      DW_UNSND (attr) = read_8_bytes (abfd, info_ptr);
+      attr->set_unsigned (read_8_bytes (abfd, info_ptr));
       info_ptr += 8;
       break;
     case DW_FORM_data16:
@@ -18455,7 +18456,8 @@ read_attribute_value (const struct die_reader_specs *reader,
       attr->set_block (blk);
       break;
     case DW_FORM_sec_offset:
-      DW_UNSND (attr) = cu->header.read_offset (abfd, info_ptr, &bytes_read);
+      attr->set_unsigned (cu->header.read_offset (abfd, info_ptr,
+						  &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_string:
@@ -18513,15 +18515,12 @@ read_attribute_value (const struct die_reader_specs *reader,
       attr->set_block (blk);
       break;
     case DW_FORM_data1:
-      DW_UNSND (attr) = read_1_byte (abfd, info_ptr);
-      info_ptr += 1;
-      break;
     case DW_FORM_flag:
-      DW_UNSND (attr) = read_1_byte (abfd, info_ptr);
+      attr->set_unsigned (read_1_byte (abfd, info_ptr));
       info_ptr += 1;
       break;
     case DW_FORM_flag_present:
-      DW_UNSND (attr) = 1;
+      attr->set_unsigned (1);
       break;
     case DW_FORM_sdata:
       attr->set_signed (read_signed_leb128 (abfd, info_ptr, &bytes_read));
@@ -18529,27 +18528,27 @@ read_attribute_value (const struct die_reader_specs *reader,
       break;
     case DW_FORM_udata:
     case DW_FORM_rnglistx:
-      DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+      attr->set_unsigned (read_unsigned_leb128 (abfd, info_ptr, &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_ref1:
-      DW_UNSND (attr) = (to_underlying (cu->header.sect_off)
-			 + read_1_byte (abfd, info_ptr));
+      attr->set_unsigned ((to_underlying (cu->header.sect_off)
+			   + read_1_byte (abfd, info_ptr)));
       info_ptr += 1;
       break;
     case DW_FORM_ref2:
-      DW_UNSND (attr) = (to_underlying (cu->header.sect_off)
-			 + read_2_bytes (abfd, info_ptr));
+      attr->set_unsigned ((to_underlying (cu->header.sect_off)
+			   + read_2_bytes (abfd, info_ptr)));
       info_ptr += 2;
       break;
     case DW_FORM_ref4:
-      DW_UNSND (attr) = (to_underlying (cu->header.sect_off)
-			 + read_4_bytes (abfd, info_ptr));
+      attr->set_unsigned ((to_underlying (cu->header.sect_off)
+			   + read_4_bytes (abfd, info_ptr)));
       info_ptr += 4;
       break;
     case DW_FORM_ref8:
-      DW_UNSND (attr) = (to_underlying (cu->header.sect_off)
-			 + read_8_bytes (abfd, info_ptr));
+      attr->set_unsigned ((to_underlying (cu->header.sect_off)
+			   + read_8_bytes (abfd, info_ptr)));
       info_ptr += 8;
       break;
     case DW_FORM_ref_sig8:
@@ -18557,8 +18556,9 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += 8;
       break;
     case DW_FORM_ref_udata:
-      DW_UNSND (attr) = (to_underlying (cu->header.sect_off)
-			 + read_unsigned_leb128 (abfd, info_ptr, &bytes_read));
+      attr->set_unsigned ((to_underlying (cu->header.sect_off)
+			   + read_unsigned_leb128 (abfd, info_ptr,
+						   &bytes_read)));
       info_ptr += bytes_read;
       break;
     case DW_FORM_indirect:
@@ -18641,7 +18641,7 @@ read_attribute_value (const struct die_reader_specs *reader,
       complaint
         (_("Suspicious DW_AT_byte_size value treated as zero instead of %s"),
          hex_string (DW_UNSND (attr)));
-      DW_UNSND (attr) = 0;
+      attr->set_unsigned (0);
     }
 
   return info_ptr;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 11/20] Add reprocessing flag to struct attribute
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (9 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 10/20] Use setter for attribute's unsigned value Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 12/20] Remove DW_ADDR Tom Tromey
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Some forms require "reprocessing" -- a second pass to update their
value appropriately.  In this case, we'll set the unsigned value on
the attribute, and then later set it to the correct value.

To handle this, we introduce a reprocessing flag to attribute.  Then,
we manage this flag to ensure that setting and unsetting is done
properly.

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

	* dwarf2/read.c (read_cutu_die_from_dwo): Use OBSTACK_ZALLOC.
	(read_attribute_reprocess, read_attribute_value): Update.
	(read_attribute): Clear requires_reprocessing.
	* dwarf2/attribute.h (struct attribute) <as_unsigned_reprocess,
	form_requires_reprocessing>: New methods.
	<string_init>: Clear requires_reprocessing.
	<set_unsigned_reprocess>: New method.
	<name>: Shrink by one bit.
	<requires_reprocessing>: New member.
	* dwarf2/attribute.c (attribute::form_requires_reprocessing): New
	method.
---
 gdb/ChangeLog          | 14 ++++++++++++++
 gdb/dwarf2/attribute.c | 14 ++++++++++++++
 gdb/dwarf2/attribute.h | 37 ++++++++++++++++++++++++++++++++++++-
 gdb/dwarf2/read.c      | 12 +++++++-----
 4 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 092a09295ba..1b72f2b53d6 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -206,3 +206,17 @@ attribute::form_is_unsigned () const
 	  || form == DW_FORM_ref8
 	  || form == DW_FORM_ref_udata);
 }
+
+/* See attribute.h.  */
+
+bool
+attribute::form_requires_reprocessing () const
+{
+  return (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_addrx
+	  || form == DW_FORM_GNU_addr_index);
+}
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 2617cab7188..3218b8be711 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -73,6 +73,15 @@ struct attribute
     return u.snd;
   }
 
+  /* Return the unsigned value, but only for attributes requiring
+     reprocessing.  */
+  ULONGEST as_unsigned_reprocess () const
+  {
+    gdb_assert (form_requires_reprocessing ());
+    gdb_assert (requires_reprocessing);
+    return u.unsnd;
+  }
+
   /* 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.
@@ -118,6 +127,10 @@ struct attribute
   /* Check if the attribute's form is an unsigned integer form.  */
   bool form_is_unsigned () const;
 
+  /* Check if the attribute's form is a form that requires
+     "reprocessing".  */
+  bool form_requires_reprocessing () const;
+
   /* Return DIE offset of this attribute.  Return 0 with complaint if
      the attribute is not of the required kind.  */
 
@@ -147,6 +160,7 @@ struct attribute
     gdb_assert (form_is_string ());
     u.str = str;
     string_is_canonical = 0;
+    requires_reprocessing = 0;
   }
 
   /* Set the canonical string value for this attribute.  */
@@ -185,8 +199,29 @@ struct attribute
     u.unsnd = unsnd;
   }
 
+  /* Temporarily set this attribute to an unsigned integer.  This is
+     used only for those forms that require reprocessing.  */
+  void set_unsigned_reprocess (ULONGEST unsnd)
+  {
+    gdb_assert (form_requires_reprocessing ());
+    u.unsnd = unsnd;
+    requires_reprocessing = 1;
+  }
+
+
+  ENUM_BITFIELD(dwarf_attribute) name : 15;
+
+  /* A boolean that is used for forms that require reprocessing.  A
+     form may require data not directly available in the attribute.
+     E.g., DW_FORM_strx requires the corresponding
+     DW_AT_str_offsets_base.  In this case, the processing for the
+     attribute must be done in two passes.  In the first past, this
+     flag is set and the value is an unsigned.  In the second pass,
+     the unsigned value is turned into the correct value for the form,
+     and this flag is cleared.  This flag is unused for other
+     forms.  */
+  unsigned int requires_reprocessing : 1;
 
-  ENUM_BITFIELD(dwarf_attribute) name : 16;
   ENUM_BITFIELD(dwarf_form) form : 15;
 
   /* Has u.str already been updated by dwarf2_canonicalize_name?  This
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3539fb0136e..92f018ad5d2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6499,7 +6499,7 @@ read_cutu_die_from_dwo (struct dwarf2_per_cu_data *this_cu,
   else if (stub_comp_dir != NULL)
     {
       /* Reconstruct the comp_dir attribute to simplify the code below.  */
-      comp_dir = XOBNEW (&cu->comp_unit_obstack, struct attribute);
+      comp_dir = OBSTACK_ZALLOC (&cu->comp_unit_obstack, struct attribute);
       comp_dir->name = DW_AT_comp_dir;
       comp_dir->form = DW_FORM_string;
       comp_dir->set_string_noncanonical (stub_comp_dir);
@@ -18355,7 +18355,7 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
     {
       case DW_FORM_addrx:
       case DW_FORM_GNU_addr_index:
-        DW_ADDR (attr) = read_addr_index (cu, DW_UNSND (attr));
+        DW_ADDR (attr) = read_addr_index (cu, attr->as_unsigned_reprocess ());
         break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
@@ -18364,7 +18364,7 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_strx4:
       case DW_FORM_GNU_str_index:
 	{
-	  unsigned int str_index = DW_UNSND (attr);
+	  unsigned int str_index = attr->as_unsigned_reprocess ();
 	  gdb_assert (!attr->canonical_string_p ());
 	  if (reader->dwo_file != NULL)
 	    attr->set_string_noncanonical (read_dwo_str_index (reader,
@@ -18578,7 +18578,8 @@ read_attribute_value (const struct die_reader_specs *reader,
     case DW_FORM_addrx:
     case DW_FORM_GNU_addr_index:
       *need_reprocess = true;
-      DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+      attr->set_unsigned_reprocess (read_unsigned_leb128 (abfd, info_ptr,
+							  &bytes_read));
       info_ptr += bytes_read;
       break;
     case DW_FORM_strx:
@@ -18615,7 +18616,7 @@ read_attribute_value (const struct die_reader_specs *reader,
 	    info_ptr += bytes_read;
 	  }
 	*need_reprocess = true;
-	 DW_UNSND (attr) = str_index;
+	attr->set_unsigned_reprocess (str_index);
 	}
       break;
     default:
@@ -18656,6 +18657,7 @@ read_attribute (const struct die_reader_specs *reader,
 {
   attr->name = abbrev->name;
   attr->string_is_canonical = 0;
+  attr->requires_reprocessing = 0;
   return read_attribute_value (reader, attr, abbrev->form,
 			       abbrev->implicit_const, info_ptr,
 			       need_reprocess);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 12/20] Remove DW_ADDR
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (10 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 11/20] Add reprocessing flag to struct attribute Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 13/20] Change how reprocessing is done Tom Tromey
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes DW_ADDR in favor of accessor methods.

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

	* dwarf2/read.c (read_attribute_reprocess, read_attribute_value)
	(dwarf2_const_value_attr, dump_die_shallow)
	(dwarf2_fetch_constant_bytes): Update.
	* dwarf2/attribute.h (struct attribute) <form_is_ref>: Update
	comment.
	<set_address>: New method.
	(DW_ADDR): Remove.
	* dwarf2/attribute.c (attribute::form_is_ref): Update comment.
	(attribute::as_string, attribute::as_address): Add assert.
---
 gdb/ChangeLog          | 12 ++++++++++++
 gdb/dwarf2/attribute.c |  6 ++++--
 gdb/dwarf2/attribute.h | 17 ++++++++++++++---
 gdb/dwarf2/read.c      | 18 +++++++++++-------
 4 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 1b72f2b53d6..4ec778bc2e2 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -36,6 +36,8 @@ attribute::as_address () const
 {
   CORE_ADDR addr;
 
+  gdb_assert (!requires_reprocessing);
+
   if (form != DW_FORM_addr && form != DW_FORM_addrx
       && form != DW_FORM_GNU_addr_index)
     {
@@ -80,6 +82,7 @@ attribute::form_is_string () const
 const char *
 attribute::as_string () const
 {
+  gdb_assert (!requires_reprocessing);
   if (form_is_string ())
     return u.str;
   return nullptr;
@@ -128,8 +131,7 @@ attribute::form_is_constant () const
     }
 }
 
-/* DW_ADDR is always stored already as sect_offset; despite for the forms
-   besides DW_FORM_ref_addr it is stored as cu_offset in the DWARF file.  */
+/* See attribute.h.  */
 
 bool
 attribute::form_is_ref () const
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 3218b8be711..e2b06294490 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -111,8 +111,9 @@ struct attribute
 
   bool form_is_constant () const;
 
-  /* DW_ADDR is always stored already as sect_offset; despite for the forms
-     besides DW_FORM_ref_addr it is stored as cu_offset in the DWARF file.  */
+  /* The address is always stored already as sect_offset; despite for
+     the forms besides DW_FORM_ref_addr it is stored as cu_offset in
+     the DWARF file.  */
 
   bool form_is_ref () const;
 
@@ -208,6 +209,17 @@ struct attribute
     requires_reprocessing = 1;
   }
 
+  /* Set this attribute to an address.  */
+  void set_address (CORE_ADDR addr)
+  {
+    gdb_assert (form == DW_FORM_addr
+		|| ((form == DW_FORM_addrx
+		     || form == DW_FORM_GNU_addr_index)
+		    && requires_reprocessing));
+    u.addr = addr;
+    requires_reprocessing = 0;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
@@ -244,6 +256,5 @@ struct attribute
 /* Get at parts of an attribute structure.  */
 
 #define DW_UNSND(attr)     ((attr)->u.unsnd)
-#define DW_ADDR(attr)	   ((attr)->u.addr)
 
 #endif /* GDB_DWARF2_ATTRIBUTE_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 92f018ad5d2..725a568c703 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18355,7 +18355,8 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
     {
       case DW_FORM_addrx:
       case DW_FORM_GNU_addr_index:
-        DW_ADDR (attr) = read_addr_index (cu, attr->as_unsigned_reprocess ());
+	attr->set_address (read_addr_index (cu,
+					    attr->as_unsigned_reprocess ()));
         break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
@@ -18416,9 +18417,12 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += bytes_read;
       break;
     case DW_FORM_addr:
-      DW_ADDR (attr) = cu->header.read_address (abfd, info_ptr, &bytes_read);
-      DW_ADDR (attr) = gdbarch_adjust_dwarf2_addr (gdbarch, DW_ADDR (attr));
-      info_ptr += bytes_read;
+      {
+	CORE_ADDR addr = cu->header.read_address (abfd, info_ptr, &bytes_read);
+	addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr);
+	attr->set_address (addr);
+	info_ptr += bytes_read;
+      }
       break;
     case DW_FORM_block2:
       blk = dwarf_alloc_block (cu);
@@ -20518,7 +20522,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
 
 	data[0] = DW_OP_addr;
 	store_unsigned_integer (&data[1], cu_header->addr_size,
-				byte_order, DW_ADDR (attr));
+				byte_order, attr->as_address ());
 	data[cu_header->addr_size + 1] = DW_OP_stack_value;
       }
       break;
@@ -21395,7 +21399,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_addrx:
 	case DW_FORM_GNU_addr_index:
 	  fprintf_unfiltered (f, "address: ");
-	  fputs_filtered (hex_string (DW_ADDR (&die->attrs[i])), f);
+	  fputs_filtered (hex_string (die->attrs[i].as_address ()), f);
 	  break;
 	case DW_FORM_block2:
 	case DW_FORM_block4:
@@ -21837,7 +21841,7 @@ dwarf2_fetch_constant_bytes (sect_offset sect_off,
 
 	*len = cu->header.addr_size;
 	tem = (gdb_byte *) obstack_alloc (obstack, *len);
-	store_unsigned_integer (tem, *len, byte_order, DW_ADDR (attr));
+	store_unsigned_integer (tem, *len, byte_order, attr->as_address ());
 	result = tem;
       }
       break;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 13/20] Change how reprocessing is done
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (11 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 12/20] Remove DW_ADDR Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 14/20] Change how accessibility is handled in dwarf2/read.c Tom Tromey
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Currently gdb keeps a vector of attributes that require reprocessing.
However, now that there is a reprocessing flag in the attribute, we
can remove the vector and instead simply loop over attributes a second
time.  Normally there are not many attributes, so this should be
reasonably cheap.

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

	* dwarf2/read.c (skip_one_die): Update.
	(read_full_die_1): Change how reprocessing is done.
	(partial_die_info::read): Update.
	(read_attribute_value): Remove need_reprocess parameter.
	(read_attribute): Likewise.
	* dwarf2/attribute.h (struct attribute) <requires_reprocessing_p>:
	New method.
---
 gdb/ChangeLog          | 10 +++++++++
 gdb/dwarf2/attribute.h |  6 ++++++
 gdb/dwarf2/read.c      | 49 +++++++++++++++++++-----------------------
 3 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index e2b06294490..63b9a761bf9 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -220,6 +220,12 @@ struct attribute
     requires_reprocessing = 0;
   }
 
+  /* True if this attribute requires reprocessing.  */
+  bool requires_reprocessing_p () const
+  {
+    return requires_reprocessing;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 725a568c703..a3815a921c9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1192,7 +1192,7 @@ static const struct cu_partial_die_info find_partial_die (sect_offset, int,
 
 static const gdb_byte *read_attribute (const struct die_reader_specs *,
 				       struct attribute *, struct attr_abbrev *,
-				       const gdb_byte *, bool *need_reprocess);
+				       const gdb_byte *);
 
 static void read_attribute_reprocess (const struct die_reader_specs *reader,
 				      struct attribute *attr);
@@ -8492,9 +8492,7 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
       /* The only abbrev we care about is DW_AT_sibling.  */
       if (abbrev->attrs[i].name == DW_AT_sibling)
 	{
-	  bool ignored;
-	  read_attribute (reader, &attr, &abbrev->attrs[i], info_ptr,
-			  &ignored);
+	  read_attribute (reader, &attr, &abbrev->attrs[i], info_ptr);
 	  if (attr.form == DW_FORM_ref_addr)
 	    complaint (_("ignoring absolute DW_AT_sibling"));
 	  else
@@ -17526,15 +17524,13 @@ read_full_die_1 (const struct die_reader_specs *reader,
      attributes.  */
   die->num_attrs = abbrev->num_attrs;
 
-  std::vector<int> indexes_that_need_reprocess;
+  bool any_need_reprocess = false;
   for (i = 0; i < abbrev->num_attrs; ++i)
     {
-      bool need_reprocess;
-      info_ptr =
-        read_attribute (reader, &die->attrs[i], &abbrev->attrs[i],
-			info_ptr, &need_reprocess);
-      if (need_reprocess)
-        indexes_that_need_reprocess.push_back (i);
+      info_ptr = read_attribute (reader, &die->attrs[i], &abbrev->attrs[i],
+				 info_ptr);
+      if (die->attrs[i].requires_reprocessing_p ())
+	any_need_reprocess = true;
     }
 
   struct attribute *attr = die->attr (DW_AT_str_offsets_base);
@@ -17544,8 +17540,14 @@ read_full_die_1 (const struct die_reader_specs *reader,
   auto maybe_addr_base = die->addr_base ();
   if (maybe_addr_base.has_value ())
     cu->addr_base = *maybe_addr_base;
-  for (int index : indexes_that_need_reprocess)
-    read_attribute_reprocess (reader, &die->attrs[index]);
+  if (any_need_reprocess)
+    {
+      for (i = 0; i < abbrev->num_attrs; ++i)
+	{
+	  if (die->attrs[i].requires_reprocessing_p ())
+	    read_attribute_reprocess (reader, &die->attrs[i]);
+	}
+    }
   *diep = die;
   return info_ptr;
 }
@@ -17899,13 +17901,11 @@ partial_die_info::read (const struct die_reader_specs *reader,
   for (i = 0; i < abbrev.num_attrs; ++i)
     {
       attribute attr;
-      bool need_reprocess;
-      info_ptr = read_attribute (reader, &attr, &abbrev.attrs[i],
-				 info_ptr, &need_reprocess);
+      info_ptr = read_attribute (reader, &attr, &abbrev.attrs[i], info_ptr);
       /* String and address offsets that need to do the reprocessing have
          already been read at this point, so there is no need to wait until
 	 the loop terminates to do the reprocessing.  */
-      if (need_reprocess)
+      if (attr.requires_reprocessing_p ())
 	read_attribute_reprocess (reader, &attr);
       /* Store the data if it is of an attribute we want to keep in a
          partial symbol table.  */
@@ -18385,8 +18385,7 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
 static const gdb_byte *
 read_attribute_value (const struct die_reader_specs *reader,
 		      struct attribute *attr, unsigned form,
-		      LONGEST implicit_const, const gdb_byte *info_ptr,
-		      bool *need_reprocess)
+		      LONGEST implicit_const, const gdb_byte *info_ptr)
 {
   struct dwarf2_cu *cu = reader->cu;
   struct dwarf2_per_objfile *dwarf2_per_objfile
@@ -18397,7 +18396,6 @@ read_attribute_value (const struct die_reader_specs *reader,
   struct comp_unit_head *cu_header = &cu->header;
   unsigned int bytes_read;
   struct dwarf_block *blk;
-  *need_reprocess = false;
 
   attr->form = (enum dwarf_form) form;
   switch (form)
@@ -18574,14 +18572,13 @@ read_attribute_value (const struct die_reader_specs *reader,
 	  info_ptr += bytes_read;
 	}
       info_ptr = read_attribute_value (reader, attr, form, implicit_const,
-				       info_ptr, need_reprocess);
+				       info_ptr);
       break;
     case DW_FORM_implicit_const:
       attr->set_signed (implicit_const);
       break;
     case DW_FORM_addrx:
     case DW_FORM_GNU_addr_index:
-      *need_reprocess = true;
       attr->set_unsigned_reprocess (read_unsigned_leb128 (abfd, info_ptr,
 							  &bytes_read));
       info_ptr += bytes_read;
@@ -18619,9 +18616,8 @@ read_attribute_value (const struct die_reader_specs *reader,
 	    str_index = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
 	    info_ptr += bytes_read;
 	  }
-	*need_reprocess = true;
 	attr->set_unsigned_reprocess (str_index);
-	}
+      }
       break;
     default:
       error (_("Dwarf Error: Cannot handle %s in DWARF reader [in module %s]"),
@@ -18657,14 +18653,13 @@ read_attribute_value (const struct die_reader_specs *reader,
 static const gdb_byte *
 read_attribute (const struct die_reader_specs *reader,
 		struct attribute *attr, struct attr_abbrev *abbrev,
-		const gdb_byte *info_ptr, bool *need_reprocess)
+		const gdb_byte *info_ptr)
 {
   attr->name = abbrev->name;
   attr->string_is_canonical = 0;
   attr->requires_reprocessing = 0;
   return read_attribute_value (reader, attr, abbrev->form,
-			       abbrev->implicit_const, info_ptr,
-			       need_reprocess);
+			       abbrev->implicit_const, info_ptr);
 }
 
 /* Return pointer to string at .debug_str offset STR_OFFSET.  */
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 14/20] Change how accessibility is handled in dwarf2/read.c
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (12 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 13/20] Change how reprocessing is done Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 15/20] Add attribute::as_unsigned method Tom Tromey
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

dwarf2/read.c uses dwarf2_default_access_attribute to check for the
default access attribute.  This patch simplifies the code by moving
more of the access processing into this function, changing its name to
reflect the difference.  This also ensures that the attribute's form
is respected, by changing to code to use the constant_value method.

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

	* dwarf2/read.c (dwarf2_access_attribute): Rename from
	dwarf2_default_access_attribute.  Look up attribute.
	(dwarf2_add_field, dwarf2_add_type_defn, dwarf2_add_member_fn):
	Update.
---
 gdb/ChangeLog     |  7 +++++++
 gdb/dwarf2/read.c | 41 +++++++++++++++++++----------------------
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a3815a921c9..52fc688c47d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14035,12 +14035,25 @@ producer_is_codewarrior (struct dwarf2_cu *cu)
   return cu->producer_is_codewarrior;
 }
 
-/* Return the default accessibility type if it is not overridden by
-   DW_AT_accessibility.  */
+/* Return the accessibility of DIE, as given by DW_AT_accessibility.
+   If that attribute is not available, return the appropriate
+   default.  */
 
 static enum dwarf_access_attribute
-dwarf2_default_access_attribute (struct die_info *die, struct dwarf2_cu *cu)
+dwarf2_access_attribute (struct die_info *die, struct dwarf2_cu *cu)
 {
+  attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu);
+  if (attr != nullptr)
+    {
+      LONGEST value = attr->constant_value (-1);
+      if (value == DW_ACCESS_public
+	  || value == DW_ACCESS_protected
+	  || value == DW_ACCESS_private)
+	return (dwarf_access_attribute) value;
+      complaint (_("Unhandled DW_AT_accessibility value (%s)"),
+		 plongest (value));
+    }
+
   if (cu->header.version < 3 || producer_is_gxx_lt_4_6 (cu))
     {
       /* The default DWARF 2 accessibility for members is public, the default
@@ -14123,11 +14136,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
       new_field = &fip->fields.back ();
     }
 
-  attr = dwarf2_attr (die, DW_AT_accessibility, cu);
-  if (attr != nullptr)
-    new_field->accessibility = DW_UNSND (attr);
-  else
-    new_field->accessibility = dwarf2_default_access_attribute (die, cu);
+  new_field->accessibility = dwarf2_access_attribute (die, cu);
   if (new_field->accessibility != DW_ACCESS_public)
     fip->non_public_fields = 1;
 
@@ -14349,12 +14358,7 @@ dwarf2_add_type_defn (struct field_info *fip, struct die_info *die,
   fp.type = read_type_die (die, cu);
 
   /* Save accessibility.  */
-  enum dwarf_access_attribute accessibility;
-  struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu);
-  if (attr != NULL)
-    accessibility = (enum dwarf_access_attribute) DW_UNSND (attr);
-  else
-    accessibility = dwarf2_default_access_attribute (die, cu);
+  dwarf_access_attribute accessibility = dwarf2_access_attribute (die, cu);
   switch (accessibility)
     {
     case DW_ACCESS_public:
@@ -14366,8 +14370,6 @@ dwarf2_add_type_defn (struct field_info *fip, struct die_info *die,
     case DW_ACCESS_protected:
       fp.is_protected = 1;
       break;
-    default:
-      complaint (_("Unhandled DW_AT_accessibility value (%x)"), accessibility);
     }
 
   if (die->tag == DW_TAG_typedef)
@@ -14544,7 +14546,6 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
   struct fn_field *fnp;
   const char *fieldname;
   struct type *this_type;
-  enum dwarf_access_attribute accessibility;
 
   if (cu->language == language_ada)
     error (_("unexpected member function in Ada type"));
@@ -14623,11 +14624,7 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
      is_volatile is irrelevant, as it is needed by gdb_mangle_name only.  */
 
   /* Get accessibility.  */
-  attr = dwarf2_attr (die, DW_AT_accessibility, cu);
-  if (attr != nullptr)
-    accessibility = (enum dwarf_access_attribute) DW_UNSND (attr);
-  else
-    accessibility = dwarf2_default_access_attribute (die, cu);
+  dwarf_access_attribute accessibility = dwarf2_access_attribute (die, cu);
   switch (accessibility)
     {
     case DW_ACCESS_private:
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 15/20] Add attribute::as_unsigned method
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (13 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 14/20] Change how accessibility is handled in dwarf2/read.c Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 16/20] Change is_valid_DW_AT_defaulted to a method on attribute Tom Tromey
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces a new attribute::as_unsigned method and changes a few
spots to use it.

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

	* dwarf2/read.c (dw2_get_file_names_reader)
	(dwarf2_build_include_psymtabs, handle_DW_AT_stmt_list)
	(dwarf2_cu::setup_type_unit_groups, fill_in_loclist_baton)
	(dwarf2_symbol_mark_computed): Use as_unsigned.
	* dwarf2/attribute.h (struct attribute) <as_unsigned>: New
	method.
	<form_is_section_offset>: Update comment.
---
 gdb/ChangeLog          | 10 ++++++++++
 gdb/dwarf2/attribute.h | 11 ++++++++++-
 gdb/dwarf2/read.c      | 22 +++++++++++-----------
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 63b9a761bf9..5d4d424d15d 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -82,9 +82,18 @@ struct attribute
     return u.unsnd;
   }
 
+  /* Return the unsigned value.  Requires that the form be an unsigned
+     form, and that reprocessing not be needed.  */
+  ULONGEST as_unsigned () const
+  {
+    gdb_assert (form_is_unsigned ());
+    gdb_assert (!requires_reprocessing);
+    return u.unsnd;
+  }
+
   /* 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.
+     You may use the as_unsigned method to retrieve such offsets.
 
      Section 7.5.4, "Attribute Encodings", explains that no attribute
      may have a value that belongs to more than one of these classes; it
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 52fc688c47d..2be28ea1944 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3046,11 +3046,11 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   sect_offset line_offset {};
 
   attr = dwarf2_attr (comp_unit_die, DW_AT_stmt_list, cu);
-  if (attr != nullptr)
+  if (attr != nullptr && attr->form_is_unsigned ())
     {
       struct quick_file_names find_entry;
 
-      line_offset = (sect_offset) DW_UNSND (attr);
+      line_offset = (sect_offset) attr->as_unsigned ();
 
       /* We may have already read in this line header (TU line header sharing).
 	 If we have we're done.  */
@@ -5920,8 +5920,8 @@ dwarf2_build_include_psymtabs (struct dwarf2_cu *cu,
   struct attribute *attr;
 
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
-  if (attr != nullptr)
-    lh = dwarf_decode_line_header ((sect_offset) DW_UNSND (attr), cu);
+  if (attr != nullptr && attr->form_is_unsigned ())
+    lh = dwarf_decode_line_header ((sect_offset) attr->as_unsigned (), cu);
   if (lh == NULL)
     return;  /* No linetable, so no includes.  */
 
@@ -10588,10 +10588,10 @@ handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
   gdb_assert (! cu->per_cu->is_debug_types);
 
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
-  if (attr == NULL)
+  if (attr == NULL || !attr->form_is_unsigned ())
     return;
 
-  sect_offset line_offset = (sect_offset) DW_UNSND (attr);
+  sect_offset line_offset = (sect_offset) attr->as_unsigned ();
 
   /* The line header hash table is only created if needed (it exists to
      prevent redundant reading of the line table for partial_units).
@@ -10780,9 +10780,9 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
   /* We have to handle the case of both a missing DW_AT_stmt_list or bad
      debug info.  */
   line_header_up lh;
-  if (attr != NULL)
+  if (attr != NULL && attr->form_is_unsigned ())
     {
-      sect_offset line_offset = (sect_offset) DW_UNSND (attr);
+      sect_offset line_offset = (sect_offset) attr->as_unsigned ();
       lh = dwarf_decode_line_header (line_offset, this);
     }
   if (lh == NULL)
@@ -22586,8 +22586,8 @@ fill_in_loclist_baton (struct dwarf2_cu *cu,
   gdb_assert (baton->per_cu);
   /* We don't know how long the location list is, but make sure we
      don't run off the edge of the section.  */
-  baton->size = section->size - DW_UNSND (attr);
-  baton->data = section->buffer + DW_UNSND (attr);
+  baton->size = section->size - attr->as_unsigned ();
+  baton->data = section->buffer + attr->as_unsigned ();
   if (cu->base_address.has_value ())
     baton->base_address = *cu->base_address;
   else
@@ -22608,7 +22608,7 @@ dwarf2_symbol_mark_computed (const struct attribute *attr, struct symbol *sym,
       /* .debug_loc{,.dwo} may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < section->get_size (objfile))
+      && attr->as_unsigned () < section->get_size (objfile))
     {
       struct dwarf2_loclist_baton *baton;
 
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 16/20] Change is_valid_DW_AT_defaulted to a method on attribute
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (14 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 15/20] Add attribute::as_unsigned method Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 17/20] Change die_info methods to check the attribute's form Tom Tromey
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes is_valid_DW_AT_defaulted to be a method on struct attribute.
Now it correctly respects the form of the attribute.

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

	* dwarf2/read.c (is_valid_DW_AT_defaulted): Move to attribute.c.
	(dwarf2_add_member_fn): Update.
	* dwarf2/attribute.h (struct attribute) <defaulted>: Declare.
	* dwarf2/attribute.c (attribute::defaulted): New method, from
	is_valid_DW_AT_defaulted.
---
 gdb/ChangeLog          |  8 ++++++++
 gdb/dwarf2/attribute.c | 23 +++++++++++++++++++++++
 gdb/dwarf2/attribute.h |  7 +++++++
 gdb/dwarf2/read.c      | 23 ++---------------------
 4 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 4ec778bc2e2..e92bb9f18e0 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -222,3 +222,26 @@ attribute::form_requires_reprocessing () const
 	  || form == DW_FORM_addrx
 	  || form == DW_FORM_GNU_addr_index);
 }
+
+/* See attribute.h.  */
+
+dwarf_defaulted_attribute
+attribute::defaulted () const
+{
+  LONGEST value = constant_value (-1);
+
+  switch (value)
+    {
+    case DW_DEFAULTED_no:
+    case DW_DEFAULTED_in_class:
+    case DW_DEFAULTED_out_of_class:
+      return (dwarf_defaulted_attribute) value;
+    }
+
+  /* If the form was not constant, we already complained in
+     constant_value, so there's no need to complain again.  */
+  if (form_is_constant ())
+    complaint (_("unrecognized DW_AT_defaulted value (%s)"),
+	       plongest (value));
+  return DW_DEFAULTED_no;
+}
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 5d4d424d15d..311147e257f 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -29,6 +29,7 @@
 
 #include "dwarf2.h"
 #include "gdbtypes.h"
+#include "gdbsupport/gdb_optional.h"
 
 /* Blocks are a bunch of untyped bytes.  */
 struct dwarf_block
@@ -235,6 +236,12 @@ struct attribute
     return requires_reprocessing;
   }
 
+  /* Return the value as one of the recognized enum
+     dwarf_defaulted_attribute constants according to DWARF5 spec,
+     Table 7.24.  If the value is incorrect, or if this attribute has
+     the wrong form, then a complaint is issued and DW_DEFAULTED_no is
+     returned.  */
+  dwarf_defaulted_attribute defaulted () const;
 
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2be28ea1944..ed139e11f83 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14514,25 +14514,6 @@ dwarf2_is_constructor (struct die_info *die, struct dwarf2_cu *cu)
 	  && (type_name[len] == '\0' || type_name[len] == '<'));
 }
 
-/* Check if the given VALUE is a recognized enum
-   dwarf_defaulted_attribute constant according to DWARF5 spec,
-   Table 7.24.  */
-
-static bool
-is_valid_DW_AT_defaulted (ULONGEST value)
-{
-  switch (value)
-    {
-    case DW_DEFAULTED_no:
-    case DW_DEFAULTED_in_class:
-    case DW_DEFAULTED_out_of_class:
-      return true;
-    }
-
-  complaint (_("unrecognized DW_AT_defaulted value (%s)"), pulongest (value));
-  return false;
-}
-
 /* Add a member function to the proper fieldlist.  */
 
 static void
@@ -14642,8 +14623,8 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
 
   /* Check for defaulted methods.  */
   attr = dwarf2_attr (die, DW_AT_defaulted, cu);
-  if (attr != nullptr && is_valid_DW_AT_defaulted (DW_UNSND (attr)))
-    fnp->defaulted = (enum dwarf_defaulted_attribute) DW_UNSND (attr);
+  if (attr != nullptr)
+    fnp->defaulted = attr->defaulted ();
 
   /* Check for deleted methods.  */
   attr = dwarf2_attr (die, DW_AT_deleted, cu);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 17/20] Change die_info methods to check the attribute's form
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (15 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 16/20] Change is_valid_DW_AT_defaulted to a method on attribute Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 18/20] Add attribute::as_virtuality method Tom Tromey
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes two die_info methods to check the form of the attribute
before using it.

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

	* dwarf2/die.h (struct die_info) <addr_base, ranges_base>: Check
	the attribute's form.
---
 gdb/ChangeLog    |  5 +++++
 gdb/dwarf2/die.h | 22 +++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/die.h b/gdb/dwarf2/die.h
index 5522ebdf311..4bc15d631e3 100644
--- a/gdb/dwarf2/die.h
+++ b/gdb/dwarf2/die.h
@@ -20,6 +20,8 @@
 #ifndef GDB_DWARF2_DIE_H
 #define GDB_DWARF2_DIE_H
 
+#include "complaints.h"
+
 /* This data structure holds a complete die structure.  */
 struct die_info
 {
@@ -40,10 +42,15 @@ struct die_info
   {
     for (unsigned i = 0; i < num_attrs; ++i)
       if (attrs[i].name == DW_AT_addr_base
-	  || attrs[i].name == DW_AT_GNU_addr_base)
+	   || attrs[i].name == DW_AT_GNU_addr_base)
 	{
-	  /* If both exist, just use the first one.  */
-	  return DW_UNSND (&attrs[i]);
+	  if (attrs[i].form_is_unsigned ())
+	    {
+	      /* If both exist, just use the first one.  */
+	      return attrs[i].as_unsigned ();
+	    }
+	  complaint (_("address base attribute (offset %s) as wrong form"),
+		     sect_offset_str (sect_off));
 	}
     return gdb::optional<ULONGEST> ();
   }
@@ -57,8 +64,13 @@ struct die_info
       if (attrs[i].name == DW_AT_rnglists_base
 	  || attrs[i].name == DW_AT_GNU_ranges_base)
 	{
-	  /* If both exist, just use the first one.  */
-	  return DW_UNSND (&attrs[i]);
+	  if (attrs[i].form_is_unsigned ())
+	    {
+	      /* If both exist, just use the first one.  */
+	      return attrs[i].as_unsigned ();
+	    }
+	  complaint (_("ranges base attribute (offset %s) as wrong form"),
+		     sect_offset_str (sect_off));
 	}
     return 0;
   }
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 18/20] Add attribute::as_virtuality method
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (16 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 17/20] Change die_info methods to check the attribute's form Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 19/20] Add attribute::as_boolean method Tom Tromey
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds a new attribute::as_virtuality method and changes the DWARF
reader to use it.  This also ensures that the attibute's form will now
be respected.

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

	* dwarf2/read.c (dwarf2_add_field, dwarf2_add_member_fn): Update.
	* dwarf2/attribute.h (struct attribute) <as_virtuality>: New
	method.
	* dwarf2/attribute.c (attribute::as_virtuality): New method.
---
 gdb/ChangeLog          |  7 +++++++
 gdb/dwarf2/attribute.c | 23 +++++++++++++++++++++++
 gdb/dwarf2/attribute.h |  5 +++++
 gdb/dwarf2/read.c      |  4 ++--
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index e92bb9f18e0..0737c27ab75 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -245,3 +245,26 @@ attribute::defaulted () const
 	       plongest (value));
   return DW_DEFAULTED_no;
 }
+
+/* See attribute.h.  */
+
+dwarf_virtuality_attribute
+attribute::as_virtuality () const
+{
+  LONGEST value = constant_value (-1);
+
+  switch (value)
+    {
+    case DW_VIRTUALITY_none:
+    case DW_VIRTUALITY_virtual:
+    case DW_VIRTUALITY_pure_virtual:
+      return (dwarf_virtuality_attribute) value;
+    }
+
+  /* If the form was not constant, we already complained in
+     constant_value, so there's no need to complain again.  */
+  if (form_is_constant ())
+    complaint (_("unrecognized DW_AT_virtuality value (%s)"),
+	       plongest (value));
+  return DW_VIRTUALITY_none;
+}
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 311147e257f..0d242af09d5 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -243,6 +243,11 @@ struct attribute
      returned.  */
   dwarf_defaulted_attribute defaulted () const;
 
+  /* Return the attribute's value as a dwarf_virtuality_attribute
+     constant according to DWARF spec.  An unrecognized value will
+     issue a complaint and return DW_VIRTUALITY_none.  */
+  dwarf_virtuality_attribute as_virtuality () const;
+
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
   /* A boolean that is used for forms that require reprocessing.  A
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ed139e11f83..4a3458ab4f7 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14142,7 +14142,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 
   attr = dwarf2_attr (die, DW_AT_virtuality, cu);
   if (attr != nullptr)
-    new_field->virtuality = DW_UNSND (attr);
+    new_field->virtuality = attr->as_virtuality ();
   else
     new_field->virtuality = DW_VIRTUALITY_none;
 
@@ -14698,7 +14698,7 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
   else
     {
       attr = dwarf2_attr (die, DW_AT_virtuality, cu);
-      if (attr && DW_UNSND (attr))
+      if (attr != nullptr && attr->as_virtuality () != DW_VIRTUALITY_none)
 	{
 	  /* GCC does this, as of 2008-08-25; PR debug/37237.  */
 	  complaint (_("Member function \"%s\" (offset %s) is virtual "
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 19/20] Add attribute::as_boolean method
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (17 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 18/20] Add attribute::as_virtuality method Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-04-04 14:43 ` [PATCH v2 20/20] Remove DW_UNSND Tom Tromey
  2020-09-30  2:28 ` [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds a new attribute::as_boolean method, and updates the reader
to use it.  The main benefit of this change is that now the code will
respect the attribute's form.

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

	* dwarf2/read.c (read_func_scope, prototyped_function_p)
	(read_subroutine_type, partial_die_info::read)
	(dwarf2_flag_true_p, new_symbol, dump_die_shallow)
	(dwarf2_add_member_fn): Update.
	* dwarf2/attribute.h (struct attribute) <as_boolean>: Declare.
	* dwarf2/attribute.c (attribute::as_boolean): New method.
---
 gdb/ChangeLog          |  9 +++++++++
 gdb/dwarf2/attribute.c | 12 ++++++++++++
 gdb/dwarf2/attribute.h |  4 ++++
 gdb/dwarf2/read.c      | 30 +++++++++++++++---------------
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 0737c27ab75..0bb2c276aaa 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -268,3 +268,15 @@ attribute::as_virtuality () const
 	       plongest (value));
   return DW_VIRTUALITY_none;
 }
+
+/* See attribute.h.  */
+
+bool
+attribute::as_boolean () const
+{
+  if (form == DW_FORM_flag_present)
+    return true;
+  else if (form == DW_FORM_flag)
+    return u.unsnd != 0;
+  return constant_value (0) != 0;
+}
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 0d242af09d5..f2a6efdcb93 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -248,6 +248,10 @@ struct attribute
      issue a complaint and return DW_VIRTUALITY_none.  */
   dwarf_virtuality_attribute as_virtuality () const;
 
+  /* Return the attribute's value as a boolean.  An unrecognized form
+     will issue a complaint and return false.  */
+  bool as_boolean () const;
+
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
   /* A boolean that is used for forms that require reprocessing.  A
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4a3458ab4f7..cb0202a617f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12796,7 +12796,7 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
       <= PC_BOUNDS_INVALID)
     {
       attr = dwarf2_attr (die, DW_AT_external, cu);
-      if (!attr || !DW_UNSND (attr))
+      if (attr == nullptr || !attr->as_boolean ())
 	complaint (_("cannot get low and high bounds "
 		     "for subprogram DIE at %s"),
 		   sect_offset_str (die->sect_off));
@@ -14618,7 +14618,7 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
 
   /* Check for artificial methods.  */
   attr = dwarf2_attr (die, DW_AT_artificial, cu);
-  if (attr && DW_UNSND (attr) != 0)
+  if (attr && attr->as_boolean ())
     fnp->is_artificial = 1;
 
   /* Check for defaulted methods.  */
@@ -14628,7 +14628,7 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
 
   /* Check for deleted methods.  */
   attr = dwarf2_attr (die, DW_AT_deleted, cu);
-  if (attr != nullptr && DW_UNSND (attr) != 0)
+  if (attr != nullptr && attr->as_boolean ())
     fnp->is_deleted = 1;
 
   fnp->is_constructor = dwarf2_is_constructor (die, cu);
@@ -16526,7 +16526,7 @@ prototyped_function_p (struct die_info *die, struct dwarf2_cu *cu)
   struct attribute *attr;
 
   attr = dwarf2_attr (die, DW_AT_prototyped, cu);
-  if (attr && (DW_UNSND (attr) != 0))
+  if (attr && attr->as_boolean ())
     return 1;
 
   /* The DWARF standard implies that the DW_AT_prototyped attribute
@@ -16595,7 +16595,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
   /* Record whether the function returns normally to its caller or not
      if the DWARF producer set that information.  */
   attr = dwarf2_attr (die, DW_AT_noreturn, cu);
-  if (attr && (DW_UNSND (attr) != 0))
+  if (attr && attr->as_boolean ())
     TYPE_NO_RETURN (ftype) = 1;
 
   /* We need to add the subroutine type to the die immediately so
@@ -16652,7 +16652,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
 		 4.5 does not yet generate.  */
 	      attr = dwarf2_attr (child_die, DW_AT_artificial, cu);
 	      if (attr != nullptr)
-		TYPE_FIELD_ARTIFICIAL (ftype, iparams) = DW_UNSND (attr);
+		TYPE_FIELD_ARTIFICIAL (ftype, iparams) = attr->as_boolean ();
 	      else
 		TYPE_FIELD_ARTIFICIAL (ftype, iparams) = 0;
 	      arg_type = die_type (child_die, cu);
@@ -17947,10 +17947,10 @@ partial_die_info::read (const struct die_reader_specs *reader,
             }
 	  break;
 	case DW_AT_external:
-	  is_external = DW_UNSND (&attr);
+	  is_external = attr.as_boolean ();
 	  break;
 	case DW_AT_declaration:
-	  is_declaration = DW_UNSND (&attr);
+	  is_declaration = attr.as_boolean ();
 	  break;
 	case DW_AT_type:
 	  has_type = 1;
@@ -18023,7 +18023,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	  break;
 
 	case DW_AT_main_subprogram:
-	  main_subprogram = DW_UNSND (&attr);
+	  main_subprogram = attr.as_boolean ();
 	  break;
 
 	case DW_AT_ranges:
@@ -19000,7 +19000,7 @@ dwarf2_flag_true_p (struct die_info *die, unsigned name, struct dwarf2_cu *cu)
 {
   struct attribute *attr = dwarf2_attr (die, name, cu);
 
-  return (attr && DW_UNSND (attr));
+  return attr != nullptr && attr->as_boolean ();
 }
 
 static int
@@ -20114,7 +20114,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	     finish_block.  */
 	  SYMBOL_ACLASS_INDEX (sym) = LOC_BLOCK;
 	  attr2 = dwarf2_attr (die, DW_AT_external, cu);
-	  if ((attr2 && (DW_UNSND (attr2) != 0))
+	  if ((attr2 != nullptr && attr2->as_boolean ())
 	      || cu->language == language_ada
 	      || cu->language == language_fortran)
 	    {
@@ -20166,7 +20166,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
 	      if (!suppress_add)
 		{
-		  if (attr2 && (DW_UNSND (attr2) != 0))
+		  if (attr2 != nullptr && attr2->as_boolean ())
 		    list_to_add = cu->get_builder ()->get_global_symbols ();
 		  else
 		    list_to_add = cu->list_in_scope;
@@ -20194,7 +20194,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 		     out, but the variable address is set to null;
 		     do not add such variables into symbol table.  */
 		}
-	      else if (attr2 && (DW_UNSND (attr2) != 0))
+	      else if (attr2 != nullptr && attr2->as_boolean ())
 		{
 		  if (SYMBOL_CLASS (sym) == LOC_STATIC
 		      && (objfile->flags & OBJF_MAINLINE) == 0
@@ -20243,7 +20243,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 		  if (!suppress_add)
 		    list_to_add = cu->list_in_scope;
 		}
-	      else if (attr2 && (DW_UNSND (attr2) != 0)
+	      else if (attr2 != nullptr && attr2->as_boolean ()
 		       && dwarf2_attr (die, DW_AT_type, cu) != NULL)
 		{
 		  /* A variable with DW_AT_external is never static, but it
@@ -21432,7 +21432,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 			      die->attrs[i].canonical_string_p () ? "is" : "not");
 	  break;
 	case DW_FORM_flag:
-	  if (DW_UNSND (&die->attrs[i]))
+	  if (die->attrs[i].as_boolean ())
 	    fprintf_unfiltered (f, "flag: TRUE");
 	  else
 	    fprintf_unfiltered (f, "flag: FALSE");
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 20/20] Remove DW_UNSND
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (18 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 19/20] Add attribute::as_boolean method Tom Tromey
@ 2020-04-04 14:43 ` Tom Tromey
  2020-09-30  8:35   ` Tom de Vries
  2020-09-30  2:28 ` [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
  20 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2020-04-04 14:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes DW_UNSND, replacing uses with either as_unsigned or
constant_value, depending primarily on whether or not the form is
already known to be appropriate.

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

	* dwarf2/read.c (lookup_dwo_id, get_type_unit_group)
	(read_file_scope, dwarf2_get_pc_bounds)
	(dwarf2_record_block_ranges, dwarf2_add_field, get_alignment)
	(read_structure_type, handle_struct_member_die)
	(read_enumeration_type, read_array_type, read_set_type)
	(read_tag_pointer_type, read_tag_reference_type)
	(read_subroutine_type, read_base_type, read_subrange_type)
	(read_full_die_1, partial_die_info::read)
	(partial_die_info::read, by, new_symbol)
	(dwarf2_const_value_data, dwarf2_const_value_attr)
	(dump_die_shallow, dwarf2_fetch_constant_bytes)
	(prepare_one_comp_unit): Update.
	* dwarf2/attribute.h (DW_UNSND): Remove.
---
 gdb/ChangeLog          |  16 +++++
 gdb/dwarf2/attribute.h |   4 --
 gdb/dwarf2/read.c      | 152 ++++++++++++++++++++---------------------
 3 files changed, 91 insertions(+), 81 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index f2a6efdcb93..51658dcedc1 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -284,8 +284,4 @@ struct attribute
   u;
 };
 
-/* Get at parts of an attribute structure.  */
-
-#define DW_UNSND(attr)     ((attr)->u.unsnd)
-
 #endif /* GDB_DWARF2_ATTRIBUTE_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index cb0202a617f..b10cfb0947e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6617,9 +6617,9 @@ lookup_dwo_id (struct dwarf2_cu *cu, struct die_info* comp_unit_die)
     return cu->header.signature;
   struct attribute *attr;
   attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_id, cu);
-  if (attr == nullptr)
+  if (attr == nullptr || !attr->form_is_unsigned ())
     return gdb::optional<ULONGEST> ();
-  return DW_UNSND (attr);
+  return attr->as_unsigned ();
 }
 
 /* Subroutine of cutu_reader to simplify it.
@@ -7109,9 +7109,9 @@ get_type_unit_group (struct dwarf2_cu *cu, const struct attribute *stmt_list)
 
   /* Do we need to create a new group, or can we use an existing one?  */
 
-  if (stmt_list)
+  if (stmt_list != nullptr && stmt_list->form_is_unsigned ())
     {
-      line_offset = DW_UNSND (stmt_list);
+      line_offset = stmt_list->as_unsigned ();
       ++tu_stats->nr_symtab_sharers;
     }
   else
@@ -10732,19 +10732,19 @@ read_file_scope (struct die_info *die, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_macros, cu);
   if (attr == NULL)
     attr = dwarf2_attr (die, DW_AT_GNU_macros, cu);
-  if (attr && cu->line_header)
+  if (attr != nullptr && attr->form_is_unsigned () && cu->line_header)
     {
       if (dwarf2_attr (die, DW_AT_macro_info, cu))
 	complaint (_("CU refers to both DW_AT_macros and DW_AT_macro_info"));
 
-      dwarf_decode_macros (cu, DW_UNSND (attr), 1);
+      dwarf_decode_macros (cu, attr->as_unsigned (), 1);
     }
   else
     {
       attr = dwarf2_attr (die, DW_AT_macro_info, cu);
-      if (attr && cu->line_header)
+      if (attr != nullptr && attr->form_is_unsigned () && cu->line_header)
 	{
-	  unsigned int macro_offset = DW_UNSND (attr);
+	  unsigned int macro_offset = attr->as_unsigned ();
 
 	  dwarf_decode_macros (cu, macro_offset, 0);
 	}
@@ -13770,13 +13770,13 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
   else
     {
       attr = dwarf2_attr (die, DW_AT_ranges, cu);
-      if (attr != NULL)
+      if (attr != nullptr && attr->form_is_unsigned ())
 	{
 	  /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
 	     We take advantage of the fact that DW_AT_ranges does not appear
 	     in DW_TAG_compile_unit of DWO files.  */
 	  int need_ranges_base = die->tag != DW_TAG_compile_unit;
-	  unsigned int ranges_offset = (DW_UNSND (attr)
+	  unsigned int ranges_offset = (attr->as_unsigned ()
 					+ (need_ranges_base
 					   ? cu->ranges_base
 					   : 0));
@@ -13941,7 +13941,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
     }
 
   attr = dwarf2_attr (die, DW_AT_ranges, cu);
-  if (attr != nullptr)
+  if (attr != nullptr && attr->form_is_unsigned ())
     {
       /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
 	 We take advantage of the fact that DW_AT_ranges does not appear
@@ -13950,7 +13950,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
 
       /* The value of the DW_AT_ranges attribute is the offset of the
          address range list in the .debug_ranges section.  */
-      unsigned long offset = (DW_UNSND (attr)
+      unsigned long offset = (attr->as_unsigned ()
 			      + (need_ranges_base ? cu->ranges_base : 0));
 
       std::vector<blockrange> blockvec;
@@ -14163,7 +14163,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
       attr = dwarf2_attr (die, DW_AT_bit_size, cu);
       if (attr != nullptr)
 	{
-	  FIELD_BITSIZE (*fp) = DW_UNSND (attr);
+	  FIELD_BITSIZE (*fp) = attr->constant_value (0);
 	}
       else
 	{
@@ -14174,7 +14174,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
       if (handle_data_member_location (die, cu, &offset))
 	SET_FIELD_BITPOS (*fp, offset * bits_per_byte);
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
-      if (attr != nullptr)
+      if (attr != nullptr && attr->form_is_unsigned ())
 	{
 	  if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
 	    {
@@ -14183,7 +14183,8 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 	         anonymous object to the MSB of the field.  We don't
 	         have to do anything special since we don't need to
 	         know the size of the anonymous object.  */
-	      SET_FIELD_BITPOS (*fp, FIELD_BITPOS (*fp) + DW_UNSND (attr));
+	      SET_FIELD_BITPOS (*fp, (FIELD_BITPOS (*fp)
+				      + attr->as_unsigned ()));
 	    }
 	  else
 	    {
@@ -14194,15 +14195,15 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 	         the field itself.  The result is the bit offset of
 	         the LSB of the field.  */
 	      int anonymous_size;
-	      int bit_offset = DW_UNSND (attr);
+	      int bit_offset = attr->as_unsigned ();
 
 	      attr = dwarf2_attr (die, DW_AT_byte_size, cu);
-	      if (attr != nullptr)
+	      if (attr != nullptr && attr->form_is_unsigned ())
 		{
 		  /* The size of the anonymous object containing
 		     the bit field is explicit, so use the
 		     indicated size (in bytes).  */
-		  anonymous_size = DW_UNSND (attr);
+		  anonymous_size = attr->as_unsigned ();
 		}
 	      else
 		{
@@ -14821,22 +14822,16 @@ get_alignment (struct dwarf2_cu *cu, struct die_info *die)
       return 0;
     }
 
-  ULONGEST align;
-  if (attr->form == DW_FORM_sdata)
+  LONGEST val = attr->constant_value (0);
+  if (val < 0)
     {
-      LONGEST val = attr->as_signed ();
-      if (val < 0)
-	{
-	  complaint (_("DW_AT_alignment value must not be negative"
-		       " - DIE at %s [in module %s]"),
-		     sect_offset_str (die->sect_off),
-		     objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
-	  return 0;
-	}
-      align = val;
+      complaint (_("DW_AT_alignment value must not be negative"
+		   " - DIE at %s [in module %s]"),
+		 sect_offset_str (die->sect_off),
+		 objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
+      return 0;
     }
-  else
-    align = DW_UNSND (attr);
+  ULONGEST align = val;
 
   if (align == 0)
     {
@@ -15004,18 +14999,18 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
      the default value DW_CC_normal.  */
   attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
   if (attr != nullptr
-      && is_valid_DW_AT_calling_convention_for_type (DW_UNSND (attr)))
+      && is_valid_DW_AT_calling_convention_for_type (attr->constant_value (0)))
     {
       ALLOCATE_CPLUS_STRUCT_TYPE (type);
       TYPE_CPLUS_CALLING_CONVENTION (type)
-	= (enum dwarf_calling_convention) (DW_UNSND (attr));
+	= (enum dwarf_calling_convention) (attr->constant_value (0));
     }
 
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr != nullptr)
     {
       if (attr->form_is_constant ())
-        TYPE_LENGTH (type) = DW_UNSND (attr);
+        TYPE_LENGTH (type) = attr->constant_value (0);
       else
 	{
 	  /* For the moment, dynamic type sizes are not supported
@@ -15145,7 +15140,8 @@ handle_struct_member_die (struct die_info *child_die, struct type *type,
       if (discr == NULL)
 	fi->fields.back ().variant.default_branch = true;
       else
-	fi->fields.back ().variant.discriminant_value = DW_UNSND (discr);
+	fi->fields.back ().variant.discriminant_value
+	  = discr->constant_value (0);
     }
 }
 
@@ -15505,7 +15501,7 @@ read_enumeration_type (struct die_info *die, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr != nullptr)
     {
-      TYPE_LENGTH (type) = DW_UNSND (attr);
+      TYPE_LENGTH (type) = attr->constant_value (0);
     }
   else
     {
@@ -15680,7 +15676,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_bit_stride, cu);
   if (attr != NULL)
-    bit_stride = DW_UNSND (attr);
+    bit_stride = attr->constant_value (0);
 
   /* Irix 6.2 native cc creates array types without children for
      arrays with unspecified length.  */
@@ -15746,10 +15742,10 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
      implementation may choose to implement triple vectors using this
      attribute.  */
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
-  if (attr != nullptr)
+  if (attr != nullptr && attr->form_is_unsigned ())
     {
-      if (DW_UNSND (attr) >= TYPE_LENGTH (type))
-	TYPE_LENGTH (type) = DW_UNSND (attr);
+      if (attr->as_unsigned () >= TYPE_LENGTH (type))
+	TYPE_LENGTH (type) = attr->as_unsigned ();
       else
 	complaint (_("DW_AT_byte_size for array type smaller "
 		     "than the total size of elements"));
@@ -15826,8 +15822,8 @@ read_set_type (struct die_info *die, struct dwarf2_cu *cu)
   set_type = create_set_type (NULL, domain_type);
 
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
-  if (attr != nullptr)
-    TYPE_LENGTH (set_type) = DW_UNSND (attr);
+  if (attr != nullptr && attr->form_is_unsigned ())
+    TYPE_LENGTH (set_type) = attr->as_unsigned ();
 
   maybe_set_alignment (cu, die, set_type);
 
@@ -16184,13 +16180,13 @@ read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
 
   attr_byte_size = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr_byte_size)
-    byte_size = DW_UNSND (attr_byte_size);
+    byte_size = attr_byte_size->constant_value (cu_header->addr_size);
   else
     byte_size = cu_header->addr_size;
 
   attr_address_class = dwarf2_attr (die, DW_AT_address_class, cu);
   if (attr_address_class)
-    addr_class = DW_UNSND (attr_address_class);
+    addr_class = attr_address_class->constant_value (DW_ADDR_none);
   else
     addr_class = DW_ADDR_none;
 
@@ -16296,7 +16292,7 @@ read_tag_reference_type (struct die_info *die, struct dwarf2_cu *cu,
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr != nullptr)
     {
-      TYPE_LENGTH (type) = DW_UNSND (attr);
+      TYPE_LENGTH (type) = attr->constant_value (cu_header->addr_size);
     }
   else
     {
@@ -16584,9 +16580,9 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
      the default value DW_CC_normal.  */
   attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
   if (attr != nullptr
-      && is_valid_DW_AT_calling_convention_for_subroutine (DW_UNSND (attr)))
+      && is_valid_DW_AT_calling_convention_for_subroutine (attr->constant_value (0)))
     TYPE_CALLING_CONVENTION (ftype)
-      = (enum dwarf_calling_convention) (DW_UNSND (attr));
+      = (enum dwarf_calling_convention) attr->constant_value (0);
   else if (cu->producer && strstr (cu->producer, "IBM XL C for OpenCL"))
     TYPE_CALLING_CONVENTION (ftype) = DW_CC_GDB_IBM_OpenCL;
   else
@@ -16854,11 +16850,11 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   gdbarch *arch;
 
   attr = dwarf2_attr (die, DW_AT_encoding, cu);
-  if (attr != nullptr)
-    encoding = DW_UNSND (attr);
+  if (attr != nullptr && attr->form_is_constant ())
+    encoding = attr->constant_value (0);
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr != nullptr)
-    bits = DW_UNSND (attr) * TARGET_CHAR_BIT;
+    bits = attr->constant_value (0) * TARGET_CHAR_BIT;
   name = dwarf2_name (die, cu);
   if (!name)
     complaint (_("DW_AT_name missing from DW_TAG_base_type"));
@@ -16867,9 +16863,9 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   enum bfd_endian byte_order = gdbarch_byte_order (arch);
 
   attr = dwarf2_attr (die, DW_AT_endianity, cu);
-  if (attr)
+  if (attr != nullptr && attr->form_is_constant ())
     {
-      int endianity = DW_UNSND (attr);
+      int endianity = attr->constant_value (0);
 
       switch (endianity)
 	{
@@ -17328,7 +17324,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr != nullptr)
-    TYPE_LENGTH (range_type) = DW_UNSND (attr);
+    TYPE_LENGTH (range_type) = attr->constant_value (0);
 
   maybe_set_alignment (cu, die, range_type);
 
@@ -17512,8 +17508,8 @@ read_full_die_1 (const struct die_reader_specs *reader,
     }
 
   struct attribute *attr = die->attr (DW_AT_str_offsets_base);
-  if (attr != nullptr)
-    cu->str_offsets_base = DW_UNSND (attr);
+  if (attr != nullptr && attr->form_is_unsigned ())
+    cu->str_offsets_base = attr->as_unsigned ();
 
   auto maybe_addr_base = die->addr_base ();
   if (maybe_addr_base.has_value ())
@@ -18003,14 +17999,17 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	     Although DWARF now specifies a way to provide this
 	     information, we support this practice for backward
 	     compatibility.  */
-	  if (DW_UNSND (&attr) == DW_CC_program
+	  if (attr.constant_value (0) == DW_CC_program
 	      && cu->language == language_fortran)
 	    main_subprogram = 1;
 	  break;
 	case DW_AT_inline:
-	  if (DW_UNSND (&attr) == DW_INL_inlined
-	      || DW_UNSND (&attr) == DW_INL_declared_inlined)
-	    may_be_inlined = 1;
+	  {
+	    LONGEST value = attr.constant_value (-1);
+	    if (value == DW_INL_inlined
+		|| value == DW_INL_declared_inlined)
+	      may_be_inlined = 1;
+	  }
 	  break;
 
 	case DW_AT_import:
@@ -18032,7 +18031,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	       but that requires a full DIE, so instead we just
 	       reimplement it.  */
 	    int need_ranges_base = tag != DW_TAG_compile_unit;
-	    unsigned int ranges_offset = (DW_UNSND (&attr)
+	    unsigned int ranges_offset = (attr.constant_value (0)
 					  + (need_ranges_base
 					     ? cu->ranges_base
 					     : 0));
@@ -18615,11 +18614,11 @@ read_attribute_value (const struct die_reader_specs *reader,
      treat them as zero by default.  */
   if (attr->name == DW_AT_byte_size
       && form == DW_FORM_data4
-      && DW_UNSND (attr) >= 0xffffffff)
+      && attr->as_unsigned () >= 0xffffffff)
     {
       complaint
         (_("Suspicious DW_AT_byte_size value treated as zero instead of %s"),
-         hex_string (DW_UNSND (attr)));
+         hex_string (attr->as_unsigned ()));
       attr->set_unsigned (0);
     }
 
@@ -20069,16 +20068,15 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 			  inlined_func ? DW_AT_call_line : DW_AT_decl_line,
 			  cu);
       if (attr != nullptr)
-	{
-	  SYMBOL_LINE (sym) = DW_UNSND (attr);
-	}
+	SYMBOL_LINE (sym) = attr->constant_value (0);
 
       attr = dwarf2_attr (die,
 			  inlined_func ? DW_AT_call_file : DW_AT_decl_file,
 			  cu);
-      if (attr != nullptr)
+      if (attr != nullptr && attr->form_is_unsigned ())
 	{
-	  file_name_index file_index = (file_name_index) DW_UNSND (attr);
+	  file_name_index file_index
+	    = (file_name_index) attr->as_unsigned ();
 	  struct file_entry *fe;
 
 	  if (cu->line_header != NULL)
@@ -20428,7 +20426,7 @@ dwarf2_const_value_data (const struct attribute *attr, struct obstack *obstack,
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
   enum bfd_endian byte_order = bfd_big_endian (objfile->obfd) ?
 				BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE;
-  LONGEST l = DW_UNSND (attr);
+  LONGEST l = attr->constant_value (0);
 
   if (bits < sizeof (*value) * 8)
     {
@@ -20545,7 +20543,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
       break;
 
     case DW_FORM_udata:
-      *value = DW_UNSND (attr);
+      *value = attr->as_unsigned ();
       break;
 
     default:
@@ -21390,11 +21388,11 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	  break;
 	case DW_FORM_ref_addr:
 	  fprintf_unfiltered (f, "ref address: ");
-	  fputs_filtered (hex_string (DW_UNSND (&die->attrs[i])), f);
+	  fputs_filtered (hex_string (die->attrs[i].as_unsigned ()), f);
 	  break;
 	case DW_FORM_GNU_ref_alt:
 	  fprintf_unfiltered (f, "alt ref address: ");
-	  fputs_filtered (hex_string (DW_UNSND (&die->attrs[i])), f);
+	  fputs_filtered (hex_string (die->attrs[i].as_unsigned ()), f);
 	  break;
 	case DW_FORM_ref1:
 	case DW_FORM_ref2:
@@ -21402,7 +21400,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_ref8:
 	case DW_FORM_ref_udata:
 	  fprintf_unfiltered (f, "constant ref: 0x%lx (adjusted)",
-			      (long) (DW_UNSND (&die->attrs[i])));
+			      (long) (die->attrs[i].as_unsigned ()));
 	  break;
 	case DW_FORM_data1:
 	case DW_FORM_data2:
@@ -21410,11 +21408,11 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_data8:
 	case DW_FORM_udata:
 	  fprintf_unfiltered (f, "constant: %s",
-			      pulongest (DW_UNSND (&die->attrs[i])));
+			      pulongest (die->attrs[i].as_unsigned ()));
 	  break;
 	case DW_FORM_sec_offset:
 	  fprintf_unfiltered (f, "section offset: %s",
-			      pulongest (DW_UNSND (&die->attrs[i])));
+			      pulongest (die->attrs[i].as_unsigned ()));
 	  break;
 	case DW_FORM_ref_sig8:
 	  fprintf_unfiltered (f, "signature: %s",
@@ -21888,7 +21886,7 @@ dwarf2_fetch_constant_bytes (sect_offset sect_off,
     case DW_FORM_udata:
       type = die_type (die, cu);
       result = write_constant_as_bytes (obstack, byte_order,
-					type, DW_UNSND (attr), len);
+					type, attr->as_unsigned (), len);
       break;
 
     default:
@@ -22900,7 +22898,7 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
   /* Set the language we're debugging.  */
   attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);
   if (attr != nullptr)
-    set_cu_language (DW_UNSND (attr), cu);
+    set_cu_language (attr->constant_value (0), cu);
   else
     {
       cu->language = pretend_language;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 00/20] Make DWARF attribute references safe
  2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
                   ` (19 preceding siblings ...)
  2020-04-04 14:43 ` [PATCH v2 20/20] Remove DW_UNSND Tom Tromey
@ 2020-09-30  2:28 ` Tom Tromey
  20 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-09-30  2:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Here's v2 of the series to make DWARF attribute references safe.
Tom> v1 was here:

Tom> https://sourceware.org/pipermail/gdb-patches/2020-March/167085.html

Tom> I think this version addresses all the review comments.

I'm going to check this in now.

Tom

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 20/20] Remove DW_UNSND
  2020-04-04 14:43 ` [PATCH v2 20/20] Remove DW_UNSND Tom Tromey
@ 2020-09-30  8:35   ` Tom de Vries
  2020-09-30 16:41     ` Tom Tromey
  0 siblings, 1 reply; 26+ messages in thread
From: Tom de Vries @ 2020-09-30  8:35 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 4/4/20 4:43 PM, Tom Tromey wrote:
> This removes DW_UNSND, replacing uses with either as_unsigned or
> constant_value, depending primarily on whether or not the form is
> already known to be appropriate.
> 
> gdb/ChangeLog
> 2020-04-04  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.c (lookup_dwo_id, get_type_unit_group)
> 	(read_file_scope, dwarf2_get_pc_bounds)
> 	(dwarf2_record_block_ranges, dwarf2_add_field, get_alignment)
> 	(read_structure_type, handle_struct_member_die)
> 	(read_enumeration_type, read_array_type, read_set_type)
> 	(read_tag_pointer_type, read_tag_reference_type)
> 	(read_subroutine_type, read_base_type, read_subrange_type)
> 	(read_full_die_1, partial_die_info::read)
> 	(partial_die_info::read, by, new_symbol)
> 	(dwarf2_const_value_data, dwarf2_const_value_attr)
> 	(dump_die_shallow, dwarf2_fetch_constant_bytes)
> 	(prepare_one_comp_unit): Update.
> 	* dwarf2/attribute.h (DW_UNSND): Remove.

This caused an internal-error:
https://sourceware.org/bugzilla/show_bug.cgi?id=26680 .

Thanks,
- Tom


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 04/20] Change some uses of DW_STRING to string method
  2020-04-04 14:43 ` [PATCH v2 04/20] Change some uses of DW_STRING to string method Tom Tromey
@ 2020-09-30 15:11   ` Tom de Vries
  2020-09-30 19:50     ` [committed][gdb] Fix regression in dwarf2_name Tom de Vries
  0 siblings, 1 reply; 26+ messages in thread
From: Tom de Vries @ 2020-09-30 15:11 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 4/4/20 4:43 PM, Tom Tromey wrote:
> This changes some of the simpler spots to use attribute::string rather
> than DW_STRING.
> 
> gdb/ChangeLog
> 2020-04-04  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::as_string.

This caused https://sourceware.org/bugzilla/show_bug.cgi?id=26683 :
...
FAIL: gdb.base/info-types-c++.exp: info types
FAIL: gdb.cp/anon-struct.exp: print type of t::t
FAIL: gdb.cp/anon-struct.exp: print type of X::t2
FAIL: gdb.cp/anon-struct.exp: print type of X::t2::t2
FAIL: gdb.cp/anon-struct.exp: print type of t3::~t3
...

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 20/20] Remove DW_UNSND
  2020-09-30  8:35   ` Tom de Vries
@ 2020-09-30 16:41     ` Tom Tromey
  0 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2020-09-30 16:41 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Tom Tromey, gdb-patches

Tom> This caused an internal-error:
Tom> https://sourceware.org/bugzilla/show_bug.cgi?id=26680 .

Thanks.  I ran into that this morning and pushed the fix a little while
ago.

Tom

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [committed][gdb] Fix regression in dwarf2_name
  2020-09-30 15:11   ` Tom de Vries
@ 2020-09-30 19:50     ` Tom de Vries
  0 siblings, 0 replies; 26+ messages in thread
From: Tom de Vries @ 2020-09-30 19:50 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

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

[ was: Re: [PATCH v2 04/20] Change some uses of DW_STRING to string method ]

On 9/30/20 5:11 PM, Tom de Vries wrote:
> On 4/4/20 4:43 PM, Tom Tromey wrote:
>> This changes some of the simpler spots to use attribute::string rather
>> than DW_STRING.
>>
>> gdb/ChangeLog
>> 2020-04-04  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::as_string.
> 
> This caused https://sourceware.org/bugzilla/show_bug.cgi?id=26683 :
> ...
> FAIL: gdb.base/info-types-c++.exp: info types
> FAIL: gdb.cp/anon-struct.exp: print type of t::t
> FAIL: gdb.cp/anon-struct.exp: print type of X::t2
> FAIL: gdb.cp/anon-struct.exp: print type of X::t2::t2
> FAIL: gdb.cp/anon-struct.exp: print type of t3::~t3
> ...
> 

Fixed by attached patch, committed.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Fix-regression-in-dwarf2_name.patch --]
[-- Type: text/x-patch, Size: 1428 bytes --]

[gdb] Fix regression in dwarf2_name

Since commit 2c830f5475 "Change some uses of DW_STRING to string method" we
have these regressions:
...
FAIL: gdb.base/info-types-c++.exp: info types
FAIL: gdb.cp/anon-struct.exp: print type of t::t
FAIL: gdb.cp/anon-struct.exp: print type of X::t2
FAIL: gdb.cp/anon-struct.exp: print type of X::t2::t2
FAIL: gdb.cp/anon-struct.exp: print type of t3::~t3
...

Fix these in dwarf2_name by updating attr_name each time attr is updated.

Tested on x86_64-linux.

gdb/ChangeLog:

2020-09-30  Tom de Vries  <tdevries@suse.de>

	PR symtab/26683
	* dwarf2/read.c (dwarf2_name): Update attr_name after attr is updated.

---
 gdb/dwarf2/read.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c0a89ecbe1..973a375641 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -22632,6 +22632,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
       if (!attr || attr_name == NULL)
 	{
 	  attr = dw2_linkage_name_attr (die, cu);
+	  attr_name = attr == nullptr ? nullptr : attr->as_string ();
 	  if (attr == NULL || attr_name == NULL)
 	    return NULL;
 
@@ -22645,6 +22646,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 		return nullptr;
 
 	      attr->set_string_canonical (objfile->intern (demangled.get ()));
+	      attr_name = attr->as_string ();
 	    }
 
 	  /* Strip any leading namespaces/classes, keep only the

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2020-09-30 19:51 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-04 14:43 [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey
2020-04-04 14:43 ` [PATCH v2 01/20] Add attribute::value_as_string method Tom Tromey
2020-04-04 14:43 ` [PATCH v2 02/20] Rename struct attribute accessors Tom Tromey
2020-04-04 14:43 ` [PATCH v2 03/20] Avoid using DW_* macros in dwarf2/attribute.c Tom Tromey
2020-04-04 14:43 ` [PATCH v2 04/20] Change some uses of DW_STRING to string method Tom Tromey
2020-09-30 15:11   ` Tom de Vries
2020-09-30 19:50     ` [committed][gdb] Fix regression in dwarf2_name Tom de Vries
2020-04-04 14:43 ` [PATCH v2 05/20] Remove some uses of DW_STRING_IS_CANONICAL Tom Tromey
2020-04-04 14:43 ` [PATCH v2 06/20] Remove DW_STRING and DW_STRING_IS_CANONICAL Tom Tromey
2020-04-04 14:43 ` [PATCH v2 07/20] Remove DW_BLOCK Tom Tromey
2020-04-04 14:43 ` [PATCH v2 08/20] Remove DW_SIGNATURE Tom Tromey
2020-04-04 14:43 ` [PATCH v2 09/20] Remove DW_SND Tom Tromey
2020-04-04 14:43 ` [PATCH v2 10/20] Use setter for attribute's unsigned value Tom Tromey
2020-04-04 14:43 ` [PATCH v2 11/20] Add reprocessing flag to struct attribute Tom Tromey
2020-04-04 14:43 ` [PATCH v2 12/20] Remove DW_ADDR Tom Tromey
2020-04-04 14:43 ` [PATCH v2 13/20] Change how reprocessing is done Tom Tromey
2020-04-04 14:43 ` [PATCH v2 14/20] Change how accessibility is handled in dwarf2/read.c Tom Tromey
2020-04-04 14:43 ` [PATCH v2 15/20] Add attribute::as_unsigned method Tom Tromey
2020-04-04 14:43 ` [PATCH v2 16/20] Change is_valid_DW_AT_defaulted to a method on attribute Tom Tromey
2020-04-04 14:43 ` [PATCH v2 17/20] Change die_info methods to check the attribute's form Tom Tromey
2020-04-04 14:43 ` [PATCH v2 18/20] Add attribute::as_virtuality method Tom Tromey
2020-04-04 14:43 ` [PATCH v2 19/20] Add attribute::as_boolean method Tom Tromey
2020-04-04 14:43 ` [PATCH v2 20/20] Remove DW_UNSND Tom Tromey
2020-09-30  8:35   ` Tom de Vries
2020-09-30 16:41     ` Tom Tromey
2020-09-30  2:28 ` [PATCH v2 00/20] Make DWARF attribute references safe Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).