[PR,symtab/32658] Fix parsing .debug_aranges section for MIPS signed addresses
Checks
Commit Message
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).
Comments
>>>>> "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
>>>>> 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
>>>>> "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
>>>>> "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
@@ -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);
}