[PATCH 00/20] Make DWARF attribute references safe

classic Classic list List threaded Threaded
47 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH 00/20] Make DWARF attribute references safe

Tom Tromey-2
This series changes the DWARF code to always check that the use of an
attribute's value is safe -- that is, that the requested type
corresponds to one of the forms that can construct a value of that
type.

This caught some minor bugs in the DWARF reader, though, IMO, nothing
very serious.

Attribute typing is still somewhat ad hoc.  That is, while the DWARF
standard specifies the type classes of forms, gdb largely doesn't
conform to this.  Instead, it is more lenient.  This could be changed,
but I didn't want to mix things too much in this series.  Also there's
an argument to be made that there's nothing wrong with the current
approach.

Regression tested by the buildbot.  Let me know what you think.

Tom


Reply | Threaded
Open this post in threaded view
|

[PATCH 01/20] Add attribute::value_as_string method

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

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

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

gdb/ChangeLog
2020-03-25  Tom Tromey  <[hidden email]>

        * 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 8c5046ef41c..98323f8920b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17896,7 +17896,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;
@@ -18949,17 +18949,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

Reply | Threaded
Open this post in threaded view
|

[PATCH 02/20] Rename struct attribute accessors

Tom Tromey-2
In reply to this post by Tom Tromey-2
This removes the "value_as_" prefix from the struct value accessors.
This seemed unnecessarily wordy to me.

gdb/ChangeLog
2020-03-28  Tom Tromey  <[hidden email]>

        * 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::address): Rename from
        value_as_address.
        (attribute::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..1bdd4cf7abb 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::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::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..cefd3c5541e 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 address () const;
 
   /* If the attribute has a string form, return the string value;
      otherwise return NULL.  */
-  const char *value_as_string () const;
+  const char *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 98323f8920b..d2b274a6e3a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5766,12 +5766,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->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->address ();
     }
 }
 
@@ -13022,7 +13022,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->address () + baseaddr;
   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
 
   if (cu->call_site_htab == NULL)
@@ -13724,8 +13724,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->address ();
+  high = attr_high->address ();
   if (cu->header.version >= 4 && attr_high->form_is_constant ())
     high += low;
  }
@@ -13897,8 +13897,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->address ();
+  CORE_ADDR high = attr_high->address ();
 
   if (cu->header.version >= 4 && attr_high->form_is_constant ())
     high += low;
@@ -17896,15 +17896,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.string ();
   break;
  case DW_AT_low_pc:
   has_low_pc_attr = 1;
-  lowpc = attr.value_as_address ();
+  lowpc = attr.address ();
   break;
  case DW_AT_high_pc:
   has_high_pc_attr = 1;
-  highpc = attr.value_as_address ();
+  highpc = attr.address ();
   if (cu->header.version >= 4 && attr.form_is_constant ())
  high_pc_relative = 1;
   break;
@@ -18949,7 +18949,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->string ();
       if (str == nullptr)
         complaint (_("string type expected for attribute %s for "
      "DIE at %s in module %s"),
@@ -20076,7 +20076,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
     {
       CORE_ADDR addr;
 
-      addr = attr->value_as_address ();
+      addr = attr->address ();
       addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
       SET_SYMBOL_VALUE_ADDRESS (sym, addr);
     }
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 03/20] Avoid using DW_* macros in dwarf2/attribute.c

Tom Tromey-2
In reply to this post by Tom Tromey-2
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-03-28  Tom Tromey  <[hidden email]>

        * 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 1bdd4cf7abb..634b7979143 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -51,10 +51,10 @@ attribute::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::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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-2
In reply to this post by Tom Tromey-2
This changes some of the simpler spots to use attribute::string rather
than DW_STRING.

2020-03-28  Tom Tromey  <[hidden email]>

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

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

Reply | Threaded
Open this post in threaded view
|

[PATCH 05/20] Remove some uses of DW_STRING_IS_CANONICAL

Tom Tromey-2
In reply to this post by Tom Tromey-2
This removes the rvalue uses of DW_STRING_IS_CANONICAL, replacing them
with an accessor method.

2020-03-28  Tom Tromey  <[hidden email]>

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

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index cefd3c5541e..f20540559aa 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -100,6 +100,12 @@ struct attribute
 
   LONGEST constant_value (int default_value) const;
 
+  /* Return true if this attribute holds a canonical string.  */
+  bool canonical_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 eb5ee98de60..4b102e52e88 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20934,7 +20934,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_p ());
 
   /* Strip the base name, keep any leading namespaces/classes.  */
   base = strrchr (attr->string (), ':');
@@ -21262,7 +21262,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 
   /* Avoid demangling attr->string () the second time on a second
      call for the same DIE.  */
-  if (!DW_STRING_IS_CANONICAL (attr))
+  if (!attr->canonical_p ())
     {
       gdb::unique_xmalloc_ptr<char> demangled
  (gdb_demangle (attr->string (), DMGL_TYPES));
@@ -21287,7 +21287,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
       break;
     }
 
-  if (!DW_STRING_IS_CANONICAL (attr))
+  if (!attr->canonical_p ())
     {
       DW_STRING (attr) = dwarf2_canonicalize_name (attr->string (), cu,
    objfile);
@@ -21407,7 +21407,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_p () ? "is" : "not");
   break;
  case DW_FORM_flag:
   if (DW_UNSND (&die->attrs[i]))
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 06/20] Remove DW_STRING and DW_STRING_IS_CANONICAL

Tom Tromey-2
In reply to this post by Tom Tromey-2
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.

2020-03-28  Tom Tromey  <[hidden email]>

        * 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.
        <string_init, string_set>: New methods.
        <string_is_canonical>: Update comment.
        (DW_STRING, DW_STRING_IS_CANONICAL): Remove.
        * dwarf2/attribute.c (attribute::form_is_string): New method.
        (attribute::string): Use it.
---
 gdb/ChangeLog          | 13 +++++++++++
 gdb/dwarf2/attribute.c | 26 ++++++++++++++-------
 gdb/dwarf2/attribute.h | 27 +++++++++++++++++----
 gdb/dwarf2/read.c      | 53 ++++++++++++++++--------------------------
 4 files changed, 72 insertions(+), 47 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 634b7979143..06b3245e4b2 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -61,18 +61,26 @@ attribute::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::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 f20540559aa..49989211018 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.  */
 
@@ -106,13 +109,29 @@ struct attribute
     return string_is_canonical;
   }
 
+  /* Initialize this attribute to hold a string value.  */
+  void string_init (const char *str)
+  {
+    gdb_assert (form_is_string ());
+    u.str = str;
+    string_is_canonical = 0;
+  }
+
+  /* Set the canonical string value for this attribute.  */
+  void string_set (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
@@ -129,8 +148,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 4b102e52e88..e3223e92d43 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6474,8 +6474,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->string_init (stub_comp_dir);
     }
 
   /* Set up for reading the DWO CU/TU.  */
@@ -18321,16 +18320,11 @@ 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_p ());
   if (reader->dwo_file != NULL)
-    {
-      DW_STRING (attr) = read_dwo_str_index (reader, str_index);
-      DW_STRING_IS_CANONICAL (attr) = 0;
-    }
+    attr->string_init (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->string_init (read_stub_str_index (cu, str_index));
   break;
  }
       default:
@@ -18418,17 +18412,15 @@ 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->string_init (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,
+  attr->string_init (read_indirect_string (dwarf2_per_objfile,
    abfd, info_ptr, cu_header,
-   &bytes_read);
-  DW_STRING_IS_CANONICAL (attr) = 0;
+   &bytes_read));
   info_ptr += bytes_read;
   break;
  }
@@ -18436,10 +18428,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->string_init
+    (dwarf2_per_objfile->read_line_string (info_ptr, cu_header,
+   &bytes_read));
   info_ptr += bytes_read;
   break;
  }
@@ -18450,8 +18441,7 @@ 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->string_init (dwz->read_string (objfile, str_offset));
  info_ptr += bytes_read;
       }
       break;
@@ -18615,6 +18605,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);
@@ -20480,7 +20471,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->string ();
       break;
@@ -21269,8 +21260,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->string_set (objfile->intern (demangled.get ()));
     }
 
   /* Strip any leading namespaces/classes, keep only the base name.
@@ -21288,11 +21278,8 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
     }
 
   if (!attr->canonical_p ())
-    {
-      DW_STRING (attr) = dwarf2_canonicalize_name (attr->string (), cu,
-   objfile);
-      DW_STRING_IS_CANONICAL (attr) = 1;
-    }
+    attr->string_set (dwarf2_canonicalize_name (attr->string (), cu,
+ objfile));
   return attr->string ();
 }
 
@@ -21405,9 +21392,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_p () ? "is" : "not");
+      die->attrs[i].string ()
+      ? die->attrs[i].string () : "",
+      die->attrs[i].canonical_p () ? "is" : "not");
   break;
  case DW_FORM_flag:
   if (DW_UNSND (&die->attrs[i]))
@@ -21799,7 +21786,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.  */
       result = (const gdb_byte *) attr->string ();
       *len = strlen (attr->string ());
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 07/20] Remove DW_BLOCK

Tom Tromey-2
In reply to this post by Tom Tromey-2
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.

2020-03-28  Tom Tromey  <[hidden email]>

        * 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) <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      | 102 ++++++++++++++++++++---------------------
 4 files changed, 82 insertions(+), 53 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 06b3245e4b2..fdf202033e9 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 49989211018..f59986f78a3 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -50,6 +50,13 @@ struct attribute
      otherwise return NULL.  */
   const char *string () const;
 
+  /* Return the block value.  The attribute must have block form.  */
+  dwarf_block *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.
@@ -125,6 +132,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;
@@ -149,7 +163,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 e3223e92d43..87bce7e51a9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -13127,15 +13127,15 @@ 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->block ()->size == 0))
     /* Keep NULL DWARF_BLOCK.  */;
   else if (attr->form_is_block ())
     {
       struct dwarf2_locexpr_baton *dlbaton;
 
       dlbaton = XOBNEW (&objfile->objfile_obstack, struct dwarf2_locexpr_baton);
-      dlbaton->data = DW_BLOCK (attr)->data;
-      dlbaton->size = DW_BLOCK (attr)->size;
+      dlbaton->data = attr->block ()->data;
+      dlbaton->size = attr->block ()->size;
       dlbaton->per_cu = cu->per_cu;
 
       SET_FIELD_DWARF_BLOCK (call_site->target, dlbaton);
@@ -13244,11 +13244,11 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       else
  {
   parameter->u.dwarf_reg = dwarf_block_to_dwarf_reg
-    (DW_BLOCK (loc)->data, &DW_BLOCK (loc)->data[DW_BLOCK (loc)->size]);
+    (loc->block ()->data, &loc->block ()->data[loc->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, loc->block ()->data,
+    &loc->block ()->data[loc->block ()->size],
      &parameter->u.fb_offset))
     parameter->kind = CALL_SITE_PARAMETER_FB_OFFSET;
   else
@@ -13274,8 +13274,8 @@ 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;
+      parameter->value = attr->block ()->data;
+      parameter->value_size = attr->block ()->size;
 
       /* Parameters are not pre-cleared by memset above.  */
       parameter->data_value = NULL;
@@ -13294,8 +13294,8 @@ 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;
+      parameter->data_value = attr->block ()->data;
+      parameter->data_value_size = attr->block ()->size;
     }
  }
     }
@@ -14057,7 +14057,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->block (), cu);
       else
  dwarf2_complex_location_expr_complaint ();
 
@@ -14632,19 +14632,19 @@ 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->block ()->size > 0)
         {
-  if (DW_BLOCK (attr)->data[0] == DW_OP_constu)
+  if (attr->block ()->data[0] == DW_OP_constu)
     {
       /* Old-style GCC.  */
-      fnp->voffset = decode_locdesc (DW_BLOCK (attr), cu) + 2;
+      fnp->voffset = decode_locdesc (attr->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 (attr->block ()->data[0] == DW_OP_deref
+   || (attr->block ()->size > 1
+       && attr->block ()->data[0] == DW_OP_deref_size
+       && attr->block ()->data[1] == cu->header.addr_size))
     {
-      fnp->voffset = decode_locdesc (DW_BLOCK (attr), cu);
+      fnp->voffset = decode_locdesc (attr->block (), cu);
       if ((fnp->voffset % cu->header.addr_size) != 0)
  dwarf2_complex_location_expr_complaint ();
       else
@@ -15858,7 +15858,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->block ()->size;
 
   ptr = (gdb_byte *) obstack_alloc (&objfile->objfile_obstack, baton->size);
   baton->data = ptr;
@@ -15878,8 +15878,8 @@ 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;
+      memcpy (ptr, member_loc->block ()->data, member_loc->block ()->size);
+      ptr += member_loc->block ()->size;
     }
 
   *ptr++ = DW_OP_plus;
@@ -16971,8 +16971,8 @@ 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;
+      baton->locexpr.size = attr->block ()->size;
+      baton->locexpr.data = attr->block ()->data;
       switch (attr->name)
  {
  case DW_AT_string_length:
@@ -17017,8 +17017,8 @@ 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;
+ baton->locexpr.size = target_attr->block ()->size;
+ baton->locexpr.data = target_attr->block ()->data;
  baton->locexpr.is_reference = true;
  prop->data.baton = baton;
  prop->kind = PROP_LOCEXPR;
@@ -17911,7 +17911,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.block ();
             }
           else if (attr.form_is_section_offset ())
             {
@@ -18378,7 +18378,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);
@@ -18386,7 +18386,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);
@@ -18405,7 +18405,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);
@@ -18452,7 +18452,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);
@@ -18460,7 +18460,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);
@@ -19920,7 +19920,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->block ()->size == 0)
     {
       SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
       return;
@@ -19932,23 +19932,23 @@ var_decode_location (struct attribute *attr, struct symbol *sym,
      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])))))
+      && ((attr->block ()->data[0] == DW_OP_addr
+   && attr->block ()->size == 1 + cu_header->addr_size)
+  || ((attr->block ()->data[0] == DW_OP_GNU_addr_index
+               || attr->block ()->data[0] == DW_OP_addrx)
+      && (attr->block ()->size
+  == 1 + leb128_size (&attr->block ()->data[1])))))
     {
       unsigned int dummy;
 
-      if (DW_BLOCK (attr)->data[0] == DW_OP_addr)
+      if (attr->block ()->data[0] == DW_OP_addr)
  SET_SYMBOL_VALUE_ADDRESS
   (sym, cu->header.read_address (objfile->obfd,
- DW_BLOCK (attr)->data + 1,
+ attr->block ()->data + 1,
  &dummy));
       else
  SET_SYMBOL_VALUE_ADDRESS
-  (sym, read_addr_index_from_leb128 (cu, DW_BLOCK (attr)->data + 1,
+  (sym, read_addr_index_from_leb128 (cu, attr->block ()->data + 1,
      &dummy));
       SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
       fixup_symbol_section (sym, objfile);
@@ -20481,7 +20481,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->block ();
       if (TYPE_LENGTH (type) != blk->size)
  dwarf2_const_value_length_mismatch_complaint (name, blk->size,
       TYPE_LENGTH (type));
@@ -21343,11 +21343,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].block ()->size));
   break;
  case DW_FORM_exprloc:
   fprintf_unfiltered (f, "expression: size %s",
-      pulongest (DW_BLOCK (&die->attrs[i])->size));
+      pulongest (die->attrs[i].block ()->size));
   break;
  case DW_FORM_data16:
   fprintf_unfiltered (f, "constant of 16 bytes");
@@ -21685,8 +21685,8 @@ 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;
+      retval.data = attr->block ()->data;
+      retval.size = attr->block ()->size;
     }
   retval.per_cu = cu->per_cu;
 
@@ -21797,8 +21797,8 @@ 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;
+      result = attr->block ()->data;
+      *len = attr->block ()->size;
       break;
 
       /* The DW_AT_const_value attributes are supposed to carry the
@@ -22578,8 +22578,8 @@ 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;
+  baton->size = attr->block ()->size;
+  baton->data = attr->block ()->data;
  }
       else
  {
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 08/20] Remove DW_SIGNATURE

Tom Tromey-2
In reply to this post by Tom Tromey-2
This removes DW_SIGNATURE in favor of methods on struct attribute.  As
usual, the methods check the form, which DW_SIGNATURE did not do.

2020-03-28  Tom Tromey  <[hidden email]>

        * 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) <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 f59986f78a3..8a7aab84eb2 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 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.
@@ -139,6 +147,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;
@@ -165,6 +180,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 87bce7e51a9..cecdc41a0eb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18503,7 +18503,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:
@@ -20693,7 +20693,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->signature ();
 
       return get_signatured_type (die, signature, cu);
     }
@@ -21383,7 +21383,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].signature ()));
   break;
  case DW_FORM_string:
  case DW_FORM_strp:
@@ -21950,7 +21950,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->signature ();
   struct signatured_type *sig_type;
   struct die_info *die;
 
@@ -22057,7 +22057,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->signature (), cu);
     }
   else
     {
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 09/20] Remove DW_SND

Tom Tromey-2
In reply to this post by Tom Tromey-2
This removes DW_SND in favor of accessors on struct attribute.
These accessors check that the form is appropriate.

gdb/ChangeLog
2020-03-28  Tom Tromey  <[hidden email]>

        * 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) <get_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 8a7aab84eb2..a08c5a13e44 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 get_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.
@@ -154,6 +162,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;
@@ -178,7 +193,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 cecdc41a0eb..6eecf051c84 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14812,7 +14812,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->get_signed ();
       if (val < 0)
  {
   complaint (_("DW_AT_alignment value must not be negative"
@@ -15764,7 +15764,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
@@ -18474,7 +18478,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:
@@ -18523,7 +18527,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:
@@ -20508,7 +20512,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->get_signed ();
       break;
 
     case DW_FORM_udata:
@@ -21373,7 +21377,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;
@@ -21411,9 +21414,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].get_signed ()));
   break;
  default:
   fprintf_unfiltered (f, "unsupported attribute form: %d.",
@@ -21839,7 +21843,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->get_signed (), len);
       break;
 
     case DW_FORM_udata:
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 10/20] Use setter for attribute's unsigned value

Tom Tromey-2
In reply to this post by Tom Tromey-2
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-03-28  Tom Tromey  <[hidden email]>

        * 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 fdf202033e9..72ec13c11f9 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 a08c5a13e44..0a4c8647f6e 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.  */
 
@@ -169,6 +172,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 6eecf051c84..75ac56efc02 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18360,15 +18360,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:
@@ -18393,15 +18394,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:
@@ -18412,7 +18413,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:
@@ -18467,15 +18469,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));
@@ -18483,27 +18482,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:
@@ -18511,8 +18510,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:
@@ -18595,7 +18595,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

Reply | Threaded
Open this post in threaded view
|

[PATCH 11/20] Add reprocessing flag to struct attribute

Tom Tromey-2
In reply to this post by Tom Tromey-2
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-03-28  Tom Tromey  <[hidden email]>

        * 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) <get_unsigned_reprocess,
        form_is_reprocessed>: 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_is_reprocessed): New
        method.
---
 gdb/ChangeLog          | 14 ++++++++++++++
 gdb/dwarf2/attribute.c | 14 ++++++++++++++
 gdb/dwarf2/attribute.h | 30 +++++++++++++++++++++++++++++-
 gdb/dwarf2/read.c      | 12 +++++++-----
 4 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 72ec13c11f9..73c1ef9f792 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_is_reprocessed () 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 0a4c8647f6e..b96fdac5201 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 get_unsigned_reprocess () const
+  {
+    gdb_assert (form_is_reprocessed ());
+    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_is_reprocessed () const;
+
   /* Return DIE offset of this attribute.  Return 0 with complaint if
      the attribute is not of the required kind.  */
 
@@ -141,6 +154,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.  */
@@ -179,8 +193,22 @@ struct attribute
     u.unsnd = unsnd;
   }
 
+  /* Temporarily 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_is_reprocessed ());
+    u.unsnd = unsnd;
+    requires_reprocessing = 1;
+  }
+
+
+  ENUM_BITFIELD(dwarf_attribute) name : 15;
+
+  /* If this requires reprocessing, is it in its final form, or is it
+     still stored as an unsigned?  */
+  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 75ac56efc02..a5c2c52375d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6471,7 +6471,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->string_init (stub_comp_dir);
@@ -18314,7 +18314,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->get_unsigned_reprocess ());
         break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
@@ -18323,7 +18323,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->get_unsigned_reprocess ();
   gdb_assert (!attr->canonical_p ());
   if (reader->dwo_file != NULL)
     attr->string_init (read_dwo_str_index (reader, str_index));
@@ -18532,7 +18532,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:
@@ -18569,7 +18570,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:
@@ -18610,6 +18611,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

Reply | Threaded
Open this post in threaded view
|

[PATCH 12/20] Remove DW_ADDR

Tom Tromey-2
In reply to this post by Tom Tromey-2
This removes DW_ADDR in favor of accessor methods.

gdb/ChangeLog
2020-03-28  Tom Tromey  <[hidden email]>

        * 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.
---
 gdb/ChangeLog          | 11 +++++++++++
 gdb/dwarf2/attribute.c |  3 +--
 gdb/dwarf2/attribute.h | 17 ++++++++++++++---
 gdb/dwarf2/read.c      | 18 +++++++++++-------
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 73c1ef9f792..94bedf6dbd3 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -128,8 +128,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 b96fdac5201..972beef9825 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;
 
@@ -202,6 +203,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;
 
@@ -231,6 +243,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 a5c2c52375d..b010c9c3b01 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18314,7 +18314,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->get_unsigned_reprocess ());
+ attr->set_address (read_addr_index (cu,
+    attr->get_unsigned_reprocess ()));
         break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
@@ -18373,9 +18374,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);
@@ -20468,7 +20472,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->address ());
  data[cu_header->addr_size + 1] = DW_OP_stack_value;
       }
       break;
@@ -21342,7 +21346,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].address ()), f);
   break;
  case DW_FORM_block2:
  case DW_FORM_block4:
@@ -21783,7 +21787,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->address ());
  result = tem;
       }
       break;
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 13/20] Change how reprocessing is done

Tom Tromey-2
In reply to this post by Tom Tromey-2
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-03-28  Tom Tromey  <[hidden email]>

        * 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) <reprocessing_p>: New
        method.
---
 gdb/ChangeLog          | 10 +++++++++
 gdb/dwarf2/attribute.h |  6 ++++++
 gdb/dwarf2/read.c      | 48 +++++++++++++++++++-----------------------
 3 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 972beef9825..9b387e5df05 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -214,6 +214,12 @@ struct attribute
     requires_reprocessing = 0;
   }
 
+  /* True if this attribute requires reprocessing.  */
+  bool 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 b010c9c3b01..d3897ac2198 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);
@@ -8464,9 +8464,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
@@ -17484,15 +17482,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].reprocessing_p ())
+ any_need_reprocess = true;
     }
 
   struct attribute *attr = die->attr (DW_AT_str_offsets_base);
@@ -17502,8 +17498,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].reprocessing_p ())
+    read_attribute_reprocess (reader, &die->attrs[i]);
+ }
+    }
   *diep = die;
   return info_ptr;
 }
@@ -17857,13 +17859,12 @@ partial_die_info::read (const struct die_reader_specs *reader,
   std::vector<struct attribute> attr_vec (abbrev.num_attrs);
   for (i = 0; i < abbrev.num_attrs; ++i)
     {
-      bool need_reprocess;
       info_ptr = read_attribute (reader, &attr_vec[i], &abbrev.attrs[i],
- info_ptr, &need_reprocess);
+ 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_vec[i].reprocessing_p ())
  read_attribute_reprocess (reader, &attr_vec[i]);
       attribute &attr = attr_vec[i];
       /* Store the data if it is of an attribute we want to keep in a
@@ -18342,8 +18343,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
@@ -18354,7 +18354,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)
@@ -18528,14 +18527,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;
@@ -18573,9 +18571,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]"),
@@ -18611,14 +18608,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

Reply | Threaded
Open this post in threaded view
|

[PATCH 14/20] Change how accessibility is handled in dwarf2/read.c

Tom Tromey-2
In reply to this post by Tom Tromey-2
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-03-28  Tom Tromey  <[hidden email]>

        * 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 | 38 +++++++++++++++++---------------------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d3897ac2198..efa59fcab4d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14001,12 +14001,24 @@ producer_is_codewarrior (struct dwarf2_cu *cu)
   return cu->producer_is_codewarrior;
 }
 
-/* Return the default accessibility type if it is not overridden by
+/* Return the accessibility type if it is not overridden by
    DW_AT_accessibility.  */
 
 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
@@ -14089,11 +14101,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;
 
@@ -14315,12 +14323,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:
@@ -14332,8 +14335,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)
@@ -14510,7 +14511,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"));
@@ -14589,11 +14589,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

Reply | Threaded
Open this post in threaded view
|

[PATCH 15/20] Add attribute::get_unsigned method

Tom Tromey-2
In reply to this post by Tom Tromey-2
This introduces a new attribute::get_unsigned method and changes a few
spots to use it.

gdb/ChangeLog
2020-03-28  Tom Tromey  <[hidden email]>

        * 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 get_unsigned.
        * dwarf2/attribute.h (struct attribute) <get_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 9b387e5df05..546156283b3 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 get_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 get_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 efa59fcab4d..abeb0e1a4e9 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->get_unsigned ();
 
       /* We may have already read in this line header (TU line header sharing).
  If we have we're done.  */
@@ -5892,8 +5892,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->get_unsigned (), cu);
   if (lh == NULL)
     return;  /* No linetable, so no includes.  */
 
@@ -10560,10 +10560,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->get_unsigned ();
 
   /* The line header hash table is only created if needed (it exists to
      prevent redundant reading of the line table for partial_units).
@@ -10752,9 +10752,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->get_unsigned ();
       lh = dwarf_decode_line_header (line_offset, this);
     }
   if (lh == NULL)
@@ -22526,8 +22526,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->get_unsigned ();
+  baton->data = section->buffer + attr->get_unsigned ();
   if (cu->base_address.has_value ())
     baton->base_address = *cu->base_address;
   else
@@ -22548,7 +22548,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->get_unsigned () < section->get_size (objfile))
     {
       struct dwarf2_loclist_baton *baton;
 
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 16/20] Change is_valid_DW_AT_defaulted to a method on attribute

Tom Tromey-2
In reply to this post by Tom Tromey-2
This changes is_valid_DW_AT_defaulted to be a method on struct attribute.
Now it correctly respects the form of the attribute.

2020-03-28  Tom Tromey  <[hidden email]>

        * 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 | 21 +++++++++++++++++++++
 gdb/dwarf2/attribute.h |  8 ++++++++
 gdb/dwarf2/read.c      | 23 ++---------------------
 4 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 94bedf6dbd3..ee5e6321bde 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -219,3 +219,24 @@ attribute::form_is_reprocessed () 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 (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 546156283b3..dce93b154a8 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
@@ -229,6 +230,13 @@ 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 abeb0e1a4e9..ec4e6e2e3b4 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14479,25 +14479,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
@@ -14607,8 +14588,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

Reply | Threaded
Open this post in threaded view
|

[PATCH 17/20] Change die_info methods to check the attribute's form

Tom Tromey-2
In reply to this post by Tom Tromey-2
This changes two die_info methods to check the form of the attribute
before using it.

2020-03-28  Tom Tromey  <[hidden email]>

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

diff --git a/gdb/dwarf2/die.h b/gdb/dwarf2/die.h
index 5522ebdf311..37f83a45a50 100644
--- a/gdb/dwarf2/die.h
+++ b/gdb/dwarf2/die.h
@@ -39,11 +39,12 @@ struct die_info
   gdb::optional<ULONGEST> addr_base ()
   {
     for (unsigned i = 0; i < num_attrs; ++i)
-      if (attrs[i].name == DW_AT_addr_base
-  || attrs[i].name == DW_AT_GNU_addr_base)
+      if ((attrs[i].name == DW_AT_addr_base
+   || attrs[i].name == DW_AT_GNU_addr_base)
+  && attrs[i].form_is_unsigned ())
  {
   /* If both exist, just use the first one.  */
-  return DW_UNSND (&attrs[i]);
+  return attrs[i].get_unsigned ();
  }
     return gdb::optional<ULONGEST> ();
   }
@@ -54,11 +55,12 @@ struct die_info
   ULONGEST ranges_base ()
   {
     for (unsigned i = 0; i < num_attrs; ++i)
-      if (attrs[i].name == DW_AT_rnglists_base
-  || attrs[i].name == DW_AT_GNU_ranges_base)
+      if ((attrs[i].name == DW_AT_rnglists_base
+   || attrs[i].name == DW_AT_GNU_ranges_base)
+  && attrs[i].form_is_unsigned ())
  {
   /* If both exist, just use the first one.  */
-  return DW_UNSND (&attrs[i]);
+  return attrs[i].get_unsigned ();
  }
     return 0;
   }
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 18/20] Add attribute::virtuality method

Tom Tromey-2
In reply to this post by Tom Tromey-2
This adds a new attribute::virtuality method and changes the DWARF
reader to use it.  This also ensures that the attibute's form will now
be respected.

2020-03-28  Tom Tromey  <[hidden email]>

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

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index ee5e6321bde..7f428279757 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -240,3 +240,24 @@ attribute::defaulted () const
        plongest (value));
   return DW_DEFAULTED_no;
 }
+
+/* See attribute.h.  */
+
+dwarf_virtuality_attribute
+attribute::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 (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 dce93b154a8..7c0ce1615cf 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -237,6 +237,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 virtuality () const;
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ec4e6e2e3b4..66a508c95ea 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14107,7 +14107,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->virtuality ();
   else
     new_field->virtuality = DW_VIRTUALITY_none;
 
@@ -14661,7 +14661,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->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

Reply | Threaded
Open this post in threaded view
|

[PATCH 19/20] Add attribute::boolean method

Tom Tromey-2
In reply to this post by Tom Tromey-2
This adds a new attribute::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.

2020-03-28  Tom Tromey  <[hidden email]>

        * 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) <boolean>: Declare.
        * dwarf2/attribute.c (attribute::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 7f428279757..db01f8ac0ca 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -261,3 +261,15 @@ attribute::virtuality () const
        plongest (value));
   return DW_VIRTUALITY_none;
 }
+
+/* See attribute.h.  */
+
+bool
+attribute::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 7c0ce1615cf..20947a11642 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -242,6 +242,10 @@ struct attribute
      issue a complaint and return DW_VIRTUALITY_none.  */
   dwarf_virtuality_attribute virtuality () const;
 
+  /* Return the attribute's value as a boolean.  An unrecognized form
+     will issue a complaint and return false.  */
+  bool boolean () const;
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 66a508c95ea..cf4a2a333ef 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12768,7 +12768,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->boolean ())
  complaint (_("cannot get low and high bounds "
      "for subprogram DIE at %s"),
    sect_offset_str (die->sect_off));
@@ -14583,7 +14583,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->boolean ())
     fnp->is_artificial = 1;
 
   /* Check for defaulted methods.  */
@@ -14593,7 +14593,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->boolean ())
     fnp->is_deleted = 1;
 
   fnp->is_constructor = dwarf2_is_constructor (die, cu);
@@ -16486,7 +16486,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->boolean ())
     return 1;
 
   /* The DWARF standard implies that the DW_AT_prototyped attribute
@@ -16555,7 +16555,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->boolean ())
     TYPE_NO_RETURN (ftype) = 1;
 
   /* We need to add the subroutine type to the die immediately so
@@ -16612,7 +16612,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->boolean ();
       else
  TYPE_FIELD_ARTIFICIAL (ftype, iparams) = 0;
       arg_type = die_type (child_die, cu);
@@ -17906,10 +17906,10 @@ partial_die_info::read (const struct die_reader_specs *reader,
             }
   break;
  case DW_AT_external:
-  is_external = DW_UNSND (&attr);
+  is_external = attr.boolean ();
   break;
  case DW_AT_declaration:
-  is_declaration = DW_UNSND (&attr);
+  is_declaration = attr.boolean ();
   break;
  case DW_AT_type:
   has_type = 1;
@@ -17982,7 +17982,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.boolean ();
   break;
 
  case DW_AT_ranges:
@@ -18954,7 +18954,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->boolean ();
 }
 
 static int
@@ -20064,7 +20064,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->boolean ())
       || cu->language == language_ada
       || cu->language == language_fortran)
     {
@@ -20116,7 +20116,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->boolean ())
     list_to_add = cu->get_builder ()->get_global_symbols ();
   else
     list_to_add = cu->list_in_scope;
@@ -20144,7 +20144,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->boolean ())
  {
   if (SYMBOL_CLASS (sym) == LOC_STATIC
       && (objfile->flags & OBJF_MAINLINE) == 0
@@ -20193,7 +20193,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->boolean ()
        && dwarf2_attr (die, DW_AT_type, cu) != NULL)
  {
   /* A variable with DW_AT_external is never static, but it
@@ -21379,7 +21379,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
       die->attrs[i].canonical_p () ? "is" : "not");
   break;
  case DW_FORM_flag:
-  if (DW_UNSND (&die->attrs[i]))
+  if (die->attrs[i].boolean ())
     fprintf_unfiltered (f, "flag: TRUE");
   else
     fprintf_unfiltered (f, "flag: FALSE");
--
2.17.2

123