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

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

Commit Message

Tom Tromey March 28, 2020, 7:22 p.m.
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  <tom@tromey.com>

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

-- 
2.17.2

Comments

Simon Marchi March 30, 2020, 3:50 p.m. | #1
On 2020-03-28 3:22 p.m., Tom Tromey wrote:
> 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  <tom@tromey.com>

> 

> 	* dwarf2/read.c (dwarf2_access_attribute): Rename from

> 	dwarf2_default_access_attribute.  Look up attribute.

> 	(dwarf2_add_field, dwarf2_add_type_defn, dwarf2_add_member_fn):

> 	Update.

> ---

>  gdb/ChangeLog     |  7 +++++++

>  gdb/dwarf2/read.c | 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.  */


Maybe I don't understand correctly, but I think the "if it is not overridden by
DW_AT_accessibility" part of the comment is no longer true.  It's more, return
the value of DW_AT_accessibility, or if it's not present, the default accessibility
value.

Otherwise, LGTM.

Simon

Patch

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: