Re: [PATCH] Fix: Sign extensions for DW_FORM_addrx were never considered

[Can Acar <canacar@imcan.dev>]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

CC'ing gdb-patches@sourceware.org as well this time. Sorry for the multiple
emails.

On Fri, Oct 24, 2025 at 6:43 AM Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Can" == Can Acar <canacar@imcan.dev> writes:
>
> Can> DW_FORM_addr is converted (sign-extended) to a signed value when
> Can> the dwarf size is less than the size of unrelocated_addr, if the
> Can> target architecture "naturally" sign extends an address (bfd.c).
>
> Thanks for the patch.
>
> Can>  static unrelocated_addr
> Can>  read_addr_index_1 (dwarf2_per_objfile *per_objfile, unsigned int
> addr_index,
> Can> -             std::optional<ULONGEST> addr_base, int addr_size)
> Can> +             std::optional<ULONGEST> addr_base, unit_head *cu_header)
>
> I think it'd be better to merge read_addr_index_1 and read_addr_index,
> i.e. have it be:
>
> static unrelocated_addr
> read_addr_index (struct dwarf2_cu *cu, unsigned int addr_index)
> ...
>
> Can> +  return cu_header->read_address(abfd, info_ptr, &ignore_bytes_read);
>
> gdb puts a space before "("
>
> Can>    else
> Can>      {
> Can>        cutu_reader reader (*per_cu, *per_objfile, nullptr, nullptr,
> false,
> Can>                      language_minimal);
> Can>        addr_base = reader.cu ()->addr_base;
> Can> -      addr_size = reader.cu ()->header.addr_size;
> Can> +      cu_header = &reader.cu ()->header;
>
> I'm not sure it is safe to refer to hold this pointer after cutu_reader
> is destroyed.
>
> Instead this could be refactored slightly to:
>
> std::optional<cutu_reader> reader;
> if ()
> ...
> else
>   {
>     reader.emplace (...)
>     cu = reader->cu ();
>   }
>
> thanks,
> Tom
>

I've implemented your suggestions, and will be sending in a new patch
version in a moment.

Best regards,
Can

[-- Attachment #2: Type: text/html, Size: 4187 bytes --]