diff mbox series

[2/2] gdb, dwarf: update complaint logic in read_tag_pointer_type

Message ID 20260728170746.1037942-2-tankutbaris.aktemur@amd.com
State New
Headers
Series [1/2] gdb, dwarf: update code style in read_tag_pointer_type |

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-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply

Commit Message

Aktemur, Baris July 28, 2026, 5:07 p.m. UTC 
  There is nested branching in `read_tag_pointer_type` with non-trivial
conditions.  I think what is meant there is if there is a non-default
address class attribute for the type, alignment and size changes are
acceptable.  Otherwise we should check for unexpected size and
alignment, and complain about them.  This patch updates the logic.

In particular:

 - If addr_class is default, byte_size does not match the expectation,
   and the architecture defines the address_class_dwarf_to_id hook
   method, code before the patch does not complain about pointer size
   whereas the new code complains.

 - If addr_class is non-default, byte_size does not match the
   expectation, and the architecture does not define the
   address_class_dwarf_to_id hook method, code before the patch
   complains about pointer size whereas the code after does not
   complain.

(Similar cases for alignment mismatch instead of type size, too.)

I think the new behavior is what was intended and it yields simpler
code.
---
 gdb/dwarf2/read.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

Comments

Tom Tromey July 30, 2026, 4:38 p.m. UTC | #1 
>>>>> Tankut Baris Aktemur <tankutbaris.aktemur@amd.com> writes:

> I think the new behavior is what was intended and it yields simpler
> code.

I agree, thanks for doing this.

Approved-By: Tom Tromey <tom@tromey.com>

This code in general seems a bit ill-considered.  Like why are the
complaints conditional on previous decisions?  However it doesn't
matter, IMO, since complaints aren't actually useful.

Tom
diff mbox series

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7db76140319..ca475f53745 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12043,10 +12043,7 @@  read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
   /* If the pointer size, alignment, or address class is different
      than the default, create a type variant marked as such and set
      the length accordingly.  */
-  if (type->length () != byte_size
-      || (alignment != 0 && TYPE_RAW_ALIGN (type) != 0
-	  && alignment != TYPE_RAW_ALIGN (type))
-      || addr_class != DW_ADDR_none)
+  if (addr_class != DW_ADDR_none)
     {
       if (gdbarch_address_class_dwarf_to_id_p (gdbarch))
 	{
@@ -12055,22 +12052,19 @@  read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
 						 addr_class);
 	  type = make_type_with_address_class (type, aclass);
 	}
-      else if (type->length () != byte_size)
-	{
-	  complaint (_("invalid pointer size %s"), pulongest (byte_size));
-	}
-      else if (TYPE_RAW_ALIGN (type) != alignment)
-	{
-	  complaint (_("Invalid DW_AT_alignment"
-		       " - DIE at %s [in module %s]"),
-		     sect_offset_str (die->sect_off),
-		     objfile_name (cu->per_objfile->objfile));
-	}
       else
 	{
 	  /* Should we also complain about unhandled address classes?  */
 	}
     }
+  else if (type->length () != byte_size)
+    complaint (_("invalid pointer size %s"), pulongest (byte_size));
+  else if (alignment != 0 && TYPE_RAW_ALIGN (type) != 0
+	   && TYPE_RAW_ALIGN (type) != alignment)
+    complaint (_("Invalid DW_AT_alignment"
+		 " - DIE at %s [in module %s]"),
+	       sect_offset_str (die->sect_off),
+	       objfile_name (cu->per_objfile->objfile));
 
   type->set_length (byte_size);
   set_type_align (type, alignment);