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

Message ID 20200328192208.11324-17-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.
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  <tom@tromey.com>

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

-- 
2.17.2

Comments

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

> 

> 	* dwarf2/read.c (is_valid_DW_AT_defaulted): Move to attribute.c.

> 	(dwarf2_add_member_fn): Update.

> 	* dwarf2/attribute.h (struct attribute) <defaulted>: Declare.

> 	* dwarf2/attribute.c (attribute::defaulted): New method, from

> 	is_valid_DW_AT_defaulted.

> ---

>  gdb/ChangeLog          |  8 ++++++++

>  gdb/dwarf2/attribute.c | 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;


I don't quite understand the logic here, we check form_is_constant after having called
constant_value?  If the form is not a constant, shouldn't we emit a warning to that
effect (and return _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;

> +

>  


There's an extra empty line here, might as well remove it.

Simon
Tom Tromey April 4, 2020, 2:23 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


>> +  if (form_is_constant ())

>> +    complaint (_("unrecognized DW_AT_defaulted value (%s)"),

>> +	       plongest (value));

>> +  return DW_DEFAULTED_no;


Simon> I don't quite understand the logic here, we check
Simon> form_is_constant after having called constant_value?  If the form
Simon> is not a constant, shouldn't we emit a warning to that effect
Simon> (and return _no)?

In this case the complaint is issued by constant_value.  This avoids
duplicate complaints.  I will make a note to this effect.

>> +  /* 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;

>> +


Simon> There's an extra empty line here, might as well remove it.

I often leave space between the methods and the data members, but I
suppose I don't really care that much.

It would be better to add "private:" but that requires another round of
patching.

Tom

Patch

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);