[2/7] Introduce and use attribute::unsigned_constant

Message ID 20250311-attribute-unsigned-initial-v1-2-833f35aa0f29@adacore.com
State New
Headers
Series Handle unsigned DW_FORM_data* constants more nicely |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Tom Tromey March 11, 2025, 5 p.m. UTC
  This introduces a new 'unsigned_constant' method on attribute.  This
method can be used to get the value as an unsigned number.  Unsigned
scalar forms are handled, and signed scalar forms are handled as well
provided that the value is non-negative.

Several spots in the reader that expect small DWARF-defined constants
are updated to use this new method.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32680
---
 gdb/dwarf2/attribute.c      |  71 +++++++++++++++++--------
 gdb/dwarf2/attribute.h      |  12 +++++
 gdb/dwarf2/cooked-indexer.c |   7 ++-
 gdb/dwarf2/read.c           | 125 ++++++++++++++++++++++++++++----------------
 gdb/dwarf2/read.h           |   2 +-
 5 files changed, 147 insertions(+), 70 deletions(-)
  

Comments

Simon Marchi March 18, 2025, 4:15 p.m. UTC | #1
On 3/11/25 1:00 PM, Tom Tromey wrote:
> @@ -19828,8 +19856,15 @@ cutu_reader::prepare_one_comp_unit (struct dwarf2_cu *cu,
>      }
>    else if (attr != nullptr)
>      {
> -      lang = dwarf_lang_to_enum_language (attr->constant_value (0));
> -      dw_lang = (dwarf_source_language) attr->constant_value (0);
> +      std::optional<ULONGEST> lang_val = attr->unsigned_constant ();
> +      if (lang_val.has_value ())
> +	{
> +	  lang = dwarf_lang_to_enum_language (*lang_val);
> +	  if (lang_val <= DW_LANG_hi_user)
> +	    dw_lang = (dwarf_source_language) *lang_val;
> +	}
> +      else
> +	lang = language_minimal;

I don't know if it matters, but I think this changes the behavior for
dw_lang.  If lang_val is > DW_LANG_hi_user, previously, dw_lang would
have that value.  With the patch, it would stay 0.  Is it on purpose?

Simon
  
Tom Tromey March 18, 2025, 6:43 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I don't know if it matters, but I think this changes the behavior for
Simon> dw_lang.  If lang_val is > DW_LANG_hi_user, previously, dw_lang would
Simon> have that value.  With the patch, it would stay 0.  Is it on purpose?

I think values greater than DW_LANG_hi_user aren't defined in DWARF, so
this "non-value" is just as good here.

Tom
  

Patch

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 3eb32b65f1406bfc67edf5134c73874d8adbb490..49c0bc07d75dd23f5b4f952f4ccb7c46cebc5265 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -164,6 +164,27 @@  attribute::constant_value (int default_value) const
 
 /* See attribute.h.  */
 
+std::optional<ULONGEST>
+attribute::unsigned_constant () const
+{
+  if (form_is_strictly_signed ())
+    {
+      if (u.snd >= 0)
+	return u.snd;
+      complaint (_("Attribute value is not unsigned (%s)"),
+		 dwarf_form_name (form));
+    }
+  else if (form_is_constant ())
+    return u.unsnd;
+
+  /* For DW_FORM_data16 see attribute::form_is_constant.  */
+  complaint (_("Attribute value is not a constant (%s)"),
+	     dwarf_form_name (form));
+  return {};
+}
+
+/* See attribute.h.  */
+
 bool
 attribute::form_is_unsigned () const
 {
@@ -216,21 +237,24 @@  attribute::form_requires_reprocessing () const
 dwarf_defaulted_attribute
 attribute::defaulted () const
 {
-  LONGEST value = constant_value (-1);
+  std::optional<ULONGEST> value = unsigned_constant ();
 
-  switch (value)
+  if (value.has_value ())
     {
-    case DW_DEFAULTED_no:
-    case DW_DEFAULTED_in_class:
-    case DW_DEFAULTED_out_of_class:
-      return (dwarf_defaulted_attribute) value;
+      switch (*value)
+	{
+	case DW_DEFAULTED_no:
+	case DW_DEFAULTED_in_class:
+	case DW_DEFAULTED_out_of_class:
+	  return (dwarf_defaulted_attribute) *value;
+
+	default:
+	  complaint (_("unrecognized DW_AT_defaulted value (%s)"),
+		     plongest (*value));
+	  break;
+	}
     }
 
-  /* If the form was not constant, we already complained in
-     constant_value, so there's no need to complain again.  */
-  if (form_is_constant ())
-    complaint (_("unrecognized DW_AT_defaulted value (%s)"),
-	       plongest (value));
   return DW_DEFAULTED_no;
 }
 
@@ -239,21 +263,24 @@  attribute::defaulted () const
 dwarf_virtuality_attribute
 attribute::as_virtuality () const
 {
-  LONGEST value = constant_value (-1);
+  std::optional<ULONGEST> value = unsigned_constant ();
 
-  switch (value)
+  if (value.has_value ())
     {
-    case DW_VIRTUALITY_none:
-    case DW_VIRTUALITY_virtual:
-    case DW_VIRTUALITY_pure_virtual:
-      return (dwarf_virtuality_attribute) value;
+      switch (*value)
+	{
+	case DW_VIRTUALITY_none:
+	case DW_VIRTUALITY_virtual:
+	case DW_VIRTUALITY_pure_virtual:
+	  return (dwarf_virtuality_attribute) *value;
+
+	default:
+	  complaint (_("unrecognized DW_AT_virtuality value (%s)"),
+		     plongest (*value));
+	  break;
+	}
     }
 
-  /* If the form was not constant, we already complained in
-     constant_value, so there's no need to complain again.  */
-  if (form_is_constant ())
-    complaint (_("unrecognized DW_AT_virtuality value (%s)"),
-	       plongest (value));
   return DW_VIRTUALITY_none;
 }
 
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 6332b399c6d33b575cdb53582978201961235248..824e92bf9cf991817e4ba0f48aebc211c2106036 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -124,6 +124,18 @@  struct attribute
 
   bool form_is_section_offset () const;
 
+  /* Return an unsigned constant value.  This only handles constant
+     forms (i.e., form_is_constant -- and not the extended list of
+     "unsigned" forms) and assumes an unsigned value is desired.  This
+     can intended for use with DWARF-defined enumerations like DW_CC_*
+     or DW_INL_*, but also in situations where a nonnegative constant
+     integer is specified by DWARF.
+
+     If a signed form and negative value is used, or if a non-constant
+     form is used, then complaint is issued and an empty value is
+     returned.  */
+  std::optional<ULONGEST> unsigned_constant () const;
+
   /* Return non-zero if ATTR's value falls in the 'constant' class, or
      zero otherwise.  When this function returns true, you can apply
      the constant_value method to it.
diff --git a/gdb/dwarf2/cooked-indexer.c b/gdb/dwarf2/cooked-indexer.c
index 18c9c3b2459f2ddf6c849f8f06dbbde18abe1930..5413ce749e52d15cbdc19e8eddc358ed32c34487 100644
--- a/gdb/dwarf2/cooked-indexer.c
+++ b/gdb/dwarf2/cooked-indexer.c
@@ -202,8 +202,11 @@  cooked_indexer::scan_attributes (dwarf2_per_cu *scanning_per_cu,
 	   the DW_AT_calling_convention attribute was used instead as
 	   the only means available.  We handle both variants then.  */
 	case DW_AT_calling_convention:
-	  if (attr.constant_value (DW_CC_normal) == DW_CC_program)
-	    *flags |= IS_MAIN;
+	  {
+	    std::optional<ULONGEST> value = attr.unsigned_constant ();
+	    if (value.has_value () && *value == DW_CC_program)
+	      *flags |= IS_MAIN;
+	  }
 	  break;
 
 	case DW_AT_declaration:
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a25daa36d38caca8f5651ea7bc0a1b4d94b492a7..85ad432192a207ccd35b4dbc79e7b6e7ba790edf 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -8567,8 +8567,10 @@  dwarf2_func_is_main_p (struct die_info *die, struct dwarf2_cu *cu)
   if (dwarf2_flag_true_p (die, DW_AT_main_subprogram, cu))
     return true;
   struct attribute *attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
-  return (attr != nullptr
-	  && attr->constant_value (DW_CC_normal) == DW_CC_program);
+  if (attr == nullptr)
+    return false;
+  std::optional<ULONGEST> value = attr->unsigned_constant ();
+  return value.has_value () && *value == DW_CC_program;
 }
 
 /* A helper to handle Ada's "Pragma Import" feature when it is applied
@@ -8688,11 +8690,14 @@  read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
 	  attr = dwarf2_attr (die, DW_AT_external, cu);
 	  bool external_p = attr != nullptr && attr->as_boolean ();
 	  attr = dwarf2_attr (die, DW_AT_inline, cu);
-	  bool inlined_p
-	    = (attr != nullptr
-	       && attr->is_nonnegative ()
-	       && (attr->as_nonnegative () == DW_INL_inlined
-		   || attr->as_nonnegative () == DW_INL_declared_inlined));
+	  bool inlined_p = false;
+	  if (attr != nullptr)
+	    {
+	      std::optional<ULONGEST> value = attr->unsigned_constant ();
+	      inlined_p = (value.has_value ()
+			   && (*value == DW_INL_inlined
+			       || *value == DW_INL_declared_inlined));
+	    }
 	  attr = dwarf2_attr (die, DW_AT_declaration, cu);
 	  bool decl_p = attr != nullptr && attr->as_boolean ();
 	  if (!external_p && !inlined_p && !decl_p)
@@ -10161,13 +10166,16 @@  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));
+      std::optional<ULONGEST> value = attr->unsigned_constant ();
+      if (value.has_value ())
+	{
+	  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)"),
+		     pulongest (*value));
+	}
     }
 
   if (cu->header.version < 3 || cu->producer_is_gxx_lt_4_6 ())
@@ -11370,12 +11378,16 @@  read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
      the die.  Otherwise the calling convention remains set to
      the default value DW_CC_normal.  */
   attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
-  if (attr != nullptr
-      && is_valid_DW_AT_calling_convention_for_type (attr->constant_value (0)))
+  if (attr != nullptr)
     {
-      ALLOCATE_CPLUS_STRUCT_TYPE (type);
-      TYPE_CPLUS_CALLING_CONVENTION (type)
-	= (enum dwarf_calling_convention) (attr->constant_value (0));
+      std::optional<ULONGEST> value = attr->unsigned_constant ();
+      if (value.has_value ()
+	  && is_valid_DW_AT_calling_convention_for_type (*value))
+	{
+	  ALLOCATE_CPLUS_STRUCT_TYPE (type);
+	  TYPE_CPLUS_CALLING_CONVENTION (type)
+	    = (enum dwarf_calling_convention) *value;
+	}
     }
 
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
@@ -11841,19 +11853,23 @@  die_byte_order (die_info *die, dwarf2_cu *cu, enum bfd_endian *byte_order)
   attribute *attr = dwarf2_attr (die, DW_AT_endianity, cu);
   if (attr != nullptr && attr->form_is_constant ())
     {
-      int endianity = attr->constant_value (0);
+      std::optional<ULONGEST> endianity = attr->unsigned_constant ();
 
-      switch (endianity)
+      if (endianity.has_value ())
 	{
-	case DW_END_big:
-	  new_order = BFD_ENDIAN_BIG;
-	  break;
-	case DW_END_little:
-	  new_order = BFD_ENDIAN_LITTLE;
-	  break;
-	default:
-	  complaint (_("DW_AT_endianity has unrecognized value %d"), endianity);
-	  break;
+	  switch (*endianity)
+	    {
+	    case DW_END_big:
+	      new_order = BFD_ENDIAN_BIG;
+	      break;
+	    case DW_END_little:
+	      new_order = BFD_ENDIAN_LITTLE;
+	      break;
+	    default:
+	      complaint (_("DW_AT_endianity has unrecognized value %s"),
+			 pulongest (*endianity));
+	      break;
+	    }
 	}
     }
 
@@ -12507,9 +12523,10 @@  read_array_order (struct die_info *die, struct dwarf2_cu *cu)
 
   if (attr != nullptr)
     {
-      LONGEST val = attr->constant_value (-1);
-      if (val == DW_ORD_row_major || val == DW_ORD_col_major)
-	return (enum dwarf_array_dim_ordering) val;
+      std::optional<ULONGEST> val = attr->unsigned_constant ();
+      if (val.has_value () &&
+	  (*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
@@ -12915,7 +12932,7 @@  read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *type;
   struct attribute *attr_byte_size;
   struct attribute *attr_address_class;
-  int byte_size, addr_class;
+  int byte_size;
   struct type *target_type;
 
   target_type = die_type (die, cu);
@@ -12934,8 +12951,10 @@  read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
     byte_size = cu_header->addr_size;
 
   attr_address_class = dwarf2_attr (die, DW_AT_address_class, cu);
+  ULONGEST addr_class;
   if (attr_address_class)
-    addr_class = attr_address_class->constant_value (DW_ADDR_none);
+    addr_class = (attr_address_class->unsigned_constant ()
+		  .value_or (DW_ADDR_none));
   else
     addr_class = DW_ADDR_none;
 
@@ -13337,10 +13356,14 @@  read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
      the subroutine die.  Otherwise set the calling convention to
      the default value DW_CC_normal.  */
   attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
-  if (attr != nullptr
-      && is_valid_DW_AT_calling_convention_for_subroutine (attr->constant_value (0)))
-    TYPE_CALLING_CONVENTION (ftype)
-      = (enum dwarf_calling_convention) attr->constant_value (0);
+  if (attr != nullptr)
+    {
+      std::optional<ULONGEST> value = attr->unsigned_constant ();
+      if (value.has_value ()
+	  && is_valid_DW_AT_calling_convention_for_subroutine (*value))
+	TYPE_CALLING_CONVENTION (ftype)
+	  = (enum dwarf_calling_convention) *value;
+    }
   else if (cu->producer_is_xlc_opencl ())
     TYPE_CALLING_CONVENTION (ftype) = DW_CC_GDB_IBM_OpenCL;
   else
@@ -13931,12 +13954,17 @@  read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   struct objfile *objfile = cu->per_objfile->objfile;
   struct type *type;
   struct attribute *attr;
-  int encoding = 0, bits = 0;
+  ULONGEST encoding = 0;
+  int bits = 0;
   const char *name;
 
   attr = dwarf2_attr (die, DW_AT_encoding, cu);
-  if (attr != nullptr && attr->form_is_constant ())
-    encoding = attr->constant_value (0);
+  if (attr != nullptr)
+    {
+      std::optional<ULONGEST> value = attr->unsigned_constant ();
+      if (value.has_value ())
+	encoding = *value;
+    }
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr != nullptr)
     bits = attr->constant_value (0) * TARGET_CHAR_BIT;
@@ -15788,7 +15816,7 @@  leb128_size (const gdb_byte *buf)
 /* Converts DWARF language names to GDB language names.  */
 
 enum language
-dwarf_lang_to_enum_language (unsigned int lang)
+dwarf_lang_to_enum_language (ULONGEST lang)
 {
   enum language language;
 
@@ -19828,8 +19856,15 @@  cutu_reader::prepare_one_comp_unit (struct dwarf2_cu *cu,
     }
   else if (attr != nullptr)
     {
-      lang = dwarf_lang_to_enum_language (attr->constant_value (0));
-      dw_lang = (dwarf_source_language) attr->constant_value (0);
+      std::optional<ULONGEST> lang_val = attr->unsigned_constant ();
+      if (lang_val.has_value ())
+	{
+	  lang = dwarf_lang_to_enum_language (*lang_val);
+	  if (lang_val <= DW_LANG_hi_user)
+	    dw_lang = (dwarf_source_language) *lang_val;
+	}
+      else
+	lang = language_minimal;
     }
   else
     lang = pretend_language;
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index e178c60509eaa1ce58bd1da4d6b6845d9d840786..1e9574d8188140e357197f704b0d75c61b6b9782 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -1055,7 +1055,7 @@  class cutu_reader
 
 /* Converts DWARF language names to GDB language names.  */
 
-enum language dwarf_lang_to_enum_language (unsigned int lang);
+enum language dwarf_lang_to_enum_language (ULONGEST lang);
 
 /* Get the dwarf2_per_objfile associated to OBJFILE.  */