Re: [PATCH] [gdb/symtab] Workaround PR gas/31115

[Tom de Vries <tdevries@suse.de>]

On 3/7/24 20:07, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> This is a regression due to PR gas/31115, which makes gas produce a low_pc
> Tom> with the thumb bit set (0x4d8 & 0x1):
> Tom> ...
> Tom>  <1><24>: Abbrev Number: 2 (DW_TAG_subprogram)
> Tom>     <25>   DW_AT_name        : main
> Tom>     <29>   DW_AT_external    : 1
> Tom>     <29>   DW_AT_type        : <0x2f>
> Tom>     <2a>   DW_AT_low_pc      : 0x4d9
> Tom>     <2e>   DW_AT_high_pc     : 12
> Tom> ...
> 
> Tom> The regression was introduced in 2.39, and is also present in 2.40 and 2.41,
> Tom> and hasn't been fixed yet.
> 
> Tom> Work around this in read_func_scope, by using gdbarch_addr_bits_remove on
> Tom> low_pc and high_pc.
> 
> Are there ever cases where this bit ought to be set here?
> 

I don't know.  The only information I have is that it never should be 
the case for arm, which is info I got out of the PR.

The target hook is present for other archs though (hppa, mips, rl78, 
s390, sparc64), and I don't know what the requirements for those are.

Which is why I limited the scope of the fix to the arm architecture.

I also limited the scope of the fix to the known wrong producer, that 
might not be necessary, it's just defensive programming.

> Tom>    lowpc = per_objfile->relocate (unrel_low);
> Tom>    highpc = per_objfile->relocate (unrel_high);
>   
> Tom> +  if (gdbarch_bfd_arch_info (gdbarch)->arch == bfd_arch_arm
> Tom> +      && producer_is_gas_ge_2_39 (cu))
> Tom> +    {
> Tom> +      /* Gas version 2.39 produces DWARF for a Thumb subprogram with a low_pc
> Tom> +	 attribute with the thumb bit set (PR gas/31115).  Work around this.  */
> Tom> +      lowpc = gdbarch_addr_bits_remove (gdbarch, lowpc);
> Tom> +      highpc = gdbarch_addr_bits_remove (gdbarch, highpc);
> 
> 'relocate' calls gdbarch_adjust_dwarf2_addr.  I wonder if that's a
> better approach.
> 

The only arch implementing that is mips, and looking at the 
implementation, it filters out the ISA bit (bit 0) and then sets back it 
if appropriate.

Which is not the thing we want to be doing here for arm.

> Right now there are a few gdbarch methods here and it's never been clear
> to me why they are different or when one should be used or not.  That
> is, there's also adjust_dwarf2_line, which is used in several other
> spots -- except dwarf_record_line_1 which for some reason calls
> gdbarch_addr_bits_remove.

As for adjust_dwarf2_line, well, the mips implementation (its only 
implementor) is a thin wrapper around adjust_dwarf2_addr that is 
stateful.  To me this looks like a candidate for removal.

Thanks,
- Tom