[PATCH][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[PATCH][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Martin Simmons]

This patch fixes https://sourceware.org/bugzilla/show_bug.cgi?id=32658 by sign
extending the addresses read from the .debug_aranges section as required for
MIPS.  I made it explicitly sign extend the addresses after they are read
(instead of using extract_signed_integer) because I don't know if the
address_size in the section will always be equal to the gdb arch_size.

I've tested the patch with the MIPS example from PR 32658 and also with a few
x86_64 binaries (where it has no effect, as expected).

diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
index 1be67852b43..a95e93aed27 100644
--- a/gdb/dwarf2/aranges.c
+++ b/gdb/dwarf2/aranges.c
@@ -59,6 +59,15 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 
   std::set<sect_offset> debug_info_offset_seen;
   const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
+  ULONGEST sign_extend_bit = 0;
+  if (bfd_get_sign_extend_vma (abfd))
+    {
+      int arch_size = bfd_get_arch_size (abfd);
+      if (arch_size < 8 * sizeof(ULONGEST))
+	{
+	  sign_extend_bit = (ULONGEST) 1 << (arch_size - 1);
+	}
+    }
   const gdb_byte *addr = section->buffer;
   while (addr < section->buffer + section->size)
     {
@@ -187,6 +196,13 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 	      /* Symbol was eliminated due to a COMDAT group.  */
 	      continue;
 	    }
+	  if (sign_extend_bit != 0)
+	    {
+	      /* Sign extend from the arch size. */
+	      start = (((start & ((sign_extend_bit << 1) - 1))
+			^ sign_extend_bit)
+		       - sign_extend_bit);
+	    }
 	  ULONGEST end = start + length;
 	  mutable_map->set_empty (start, end - 1, per_cu);
 	}

Re: [PATCH][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Tom Tromey]

>>>>> "Martin" == Martin Simmons <qqxnjvamvxwx@dyxyl.com> writes:

Martin> This patch fixes https://sourceware.org/bugzilla/show_bug.cgi?id=32658 by sign
Martin> extending the addresses read from the .debug_aranges section as required for
Martin> MIPS.  I made it explicitly sign extend the addresses after they are read
Martin> (instead of using extract_signed_integer) because I don't know if the
Martin> address_size in the section will always be equal to the gdb arch_size.

I wonder if perhaps the relevant gdbarch method should be used instead.

Tom

{Spam?} Re: [PATCH][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Martin Simmons]

>>>>> On Sat, 08 Mar 2025 09:39:06 -0700, Tom Tromey said:
> 
>>>>> "Martin" == Martin Simmons <qqxnjvamvxwx@dyxyl.com> writes:
> 
> Martin> This patch fixes https://sourceware.org/bugzilla/show_bug.cgi?id=32658 by sign
> Martin> extending the addresses read from the .debug_aranges section as required for
> Martin> MIPS.  I made it explicitly sign extend the addresses after they are read
> Martin> (instead of using extract_signed_integer) because I don't know if the
> Martin> address_size in the section will always be equal to the gdb arch_size.
> 
> I wonder if perhaps the relevant gdbarch method should be used instead.

Do you mean gdbarch_integer_to_address like in
dwarf_expr_context::fetch_address?  I considered that, but it looked
like a kludge and it wasn't clear to me if that would be good for avr
(calling avr_integer_to_address).

Also, the old parser used comp_unit_head::read_address (called from
dwarf_decode_lines_1) so that didn't do anything with gdbarch.

__Martin

Re: {Spam?} Re: [PATCH][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Tom Tromey]

>>>>> "Martin" == Martin Simmons <qqxnjvamvxwx@dyxyl.com> writes:

>> I wonder if perhaps the relevant gdbarch method should be used instead.

Martin> Do you mean gdbarch_integer_to_address like in
Martin> dwarf_expr_context::fetch_address?

Yeah.

Martin>  I considered that, but it looked
Martin> like a kludge and it wasn't clear to me if that would be good for avr
Martin> (calling avr_integer_to_address).

Alright.  I'll look at it again.

Tom

Re: [PATCH][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Tom Tromey]

>>>>> "Martin" == Martin Simmons <qqxnjvamvxwx@dyxyl.com> writes:

Martin> This patch fixes https://sourceware.org/bugzilla/show_bug.cgi?id=32658 by sign

There should be a 'Bug: ...' trailer in the comment with this link.

FWIW I see now that other spots in the DWARF reader do use the BFD
approach:

  int signed_addr_p = bfd_get_sign_extend_vma (objfile->obfd.get ());

Martin> I made it explicitly sign extend the addresses after they are read
Martin> (instead of using extract_signed_integer) because I don't know if the
Martin> address_size in the section will always be equal to the gdb arch_size.

I don't think this should be a concern, unless you have an example where
it actually happens.

The .debug_aranges header spec explicitly says "size of an address in
bytes on the target architecture".

Tom

[PATCH v2][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Martin Simmons]

>>>>> On Wed, 19 Mar 2025 10:48:09 -0600, Tom Tromey said:
> 
>>>>> "Martin" == Martin Simmons <qqxnjvamvxwx@dyxyl.com> writes:
> 
> Martin> This patch fixes https://sourceware.org/bugzilla/show_bug.cgi?id=32658 by sign
> 
> There should be a 'Bug: ...' trailer in the comment with this link.
> 
> FWIW I see now that other spots in the DWARF reader do use the BFD
> approach:
> 
>   int signed_addr_p = bfd_get_sign_extend_vma (objfile->obfd.get ());
> 
> Martin> I made it explicitly sign extend the addresses after they are read
> Martin> (instead of using extract_signed_integer) because I don't know if the
> Martin> address_size in the section will always be equal to the gdb arch_size.
> 
> I don't think this should be a concern, unless you have an example where
> it actually happens.
> 
> The .debug_aranges header spec explicitly says "size of an address in
> bytes on the target architecture".

I've not found an example (and both gcc and clang explicitly write them
using the target's pointer size).

Here is v2 of the patch addressing the above.


Fix parsing .debug_aranges section for signed addresses.

Some architectures, such as MIPS, have signed addresses and this changes
read_addrmap_from_aranges to record them as signed when required.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32658

diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
index 7016eee557c..ac530abd6b3 100644
--- a/gdb/dwarf2/aranges.c
+++ b/gdb/dwarf2/aranges.c
@@ -59,6 +59,7 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 
   gdb::unordered_set<sect_offset> debug_info_offset_seen;
   const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
+  const int signed_addr_p = bfd_get_sign_extend_vma (abfd);
   const gdb_byte *addr = section->buffer;
   while (addr < section->buffer + section->size)
     {
@@ -167,8 +168,13 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 			  plongest (entry_addr - section->buffer));
 	      return false;
 	    }
-	  ULONGEST start = extract_unsigned_integer (addr, address_size,
-						     dwarf5_byte_order);
+	  ULONGEST start;
+	  if (signed_addr_p)
+	    start = extract_signed_integer (addr, address_size,
+					    dwarf5_byte_order);
+	  else
+	    start = extract_unsigned_integer (addr, address_size,
+					      dwarf5_byte_order);
 	  addr += address_size;
 	  ULONGEST length = extract_unsigned_integer (addr, address_size,
 						      dwarf5_byte_order);

Re: [PATCH v2][PR symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses

[Tom Tromey]

>>>>> "Martin" == Martin Simmons <qqxnjvamvxwx@dyxyl.com> writes:

Martin> I've not found an example (and both gcc and clang explicitly write them
Martin> using the target's pointer size).

Martin> Here is v2 of the patch addressing the above.

Thanks.  I'll check it in shortly.

Tom