[gdb/symtab] Workaround PR gas/31115
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
On arm-linux, with gas 2.40, I run into:
...
(gdb) x /i main+8^M
0x4e1 <main+7>: vrhadd.u16 d14, d14, d31^M
(gdb) FAIL: gdb.arch/pr25124.exp: disassemble thumb instruction (1st try)
...
This is a regression due to PR gas/31115, which makes gas produce a low_pc
with the thumb bit set (0x4d8 & 0x1):
...
<1><24>: Abbrev Number: 2 (DW_TAG_subprogram)
<25> DW_AT_name : main
<29> DW_AT_external : 1
<29> DW_AT_type : <0x2f>
<2a> DW_AT_low_pc : 0x4d9
<2e> DW_AT_high_pc : 12
...
The regression was introduced in 2.39, and is also present in 2.40 and 2.41,
and hasn't been fixed yet.
Work around this in read_func_scope, by using gdbarch_addr_bits_remove on
low_pc and high_pc.
Tested on arm-linux and x86_64-linux.
PR tdep/31453
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31453
---
gdb/dwarf2/cu.c | 1 +
gdb/dwarf2/cu.h | 1 +
gdb/dwarf2/read.c | 22 ++++++++++++++++++++++
3 files changed, 24 insertions(+)
base-commit: 164cc86b81de7ab25682a234b978134d08fb3009
Comments
>>>>> "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?
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.
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.
Tom
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
On 3/7/24 22:09, Tom de Vries wrote:
> 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.
>
I'm planning to commit this tomorrow, any objects?
Thanks,
- Tom
On 3/19/24 09:39, Tom de Vries wrote:
> On 3/7/24 22:09, Tom de Vries wrote:
>> 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.
>>
>
> I'm planning to commit this tomorrow, any objects?
None from me.
@@ -42,6 +42,7 @@ dwarf2_cu::dwarf2_cu (dwarf2_per_cu_data *per_cu,
producer_is_clang (false),
producer_is_gas_lt_2_38 (false),
producer_is_gas_2_39 (false),
+ producer_is_gas_ge_2_40 (false),
processing_has_namespace_info (false)
{
}
@@ -263,6 +263,7 @@ struct dwarf2_cu
bool producer_is_clang : 1;
bool producer_is_gas_lt_2_38 : 1;
bool producer_is_gas_2_39 : 1;
+ bool producer_is_gas_ge_2_40 : 1;
/* When true, the file that we're processing is known to have
debugging info for C++ namespaces. GCC 3.3.x did not produce
@@ -148,6 +148,7 @@ static int dwarf2_loclist_block_index;
static int ada_block_index;
static bool producer_is_gas_lt_2_38 (struct dwarf2_cu *cu);
+static bool producer_is_gas_ge_2_39 (struct dwarf2_cu *cu);
/* Size of .debug_loclists section header for 32-bit DWARF format. */
#define LOCLIST_HEADER_SIZE32 12
@@ -9974,6 +9975,15 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
lowpc = per_objfile->relocate (unrel_low);
highpc = per_objfile->relocate (unrel_high);
+ if (gdbarch_bfd_arch_info (gdbarch)->arch == bfd_arch_arm
+ && producer_is_gas_ge_2_39 (cu))
+ {
+ /* Gas version 2.39 produces DWARF for a Thumb subprogram with a low_pc
+ attribute with the thumb bit set (PR gas/31115). Work around this. */
+ lowpc = gdbarch_addr_bits_remove (gdbarch, lowpc);
+ highpc = gdbarch_addr_bits_remove (gdbarch, highpc);
+ }
+
/* If we have any template arguments, then we must allocate a
different sort of symbol. */
for (child_die = die->child; child_die; child_die = child_die->sibling)
@@ -11326,6 +11336,7 @@ check_producer (struct dwarf2_cu *cu)
{
cu->producer_is_gas_lt_2_38 = major < 2 || (major == 2 && minor < 38);
cu->producer_is_gas_2_39 = major == 2 && minor == 39;
+ cu->producer_is_gas_ge_2_40 = major > 2 || (major == 2 && minor >= 40);
}
else
{
@@ -11380,6 +11391,17 @@ producer_is_gas_2_39 (struct dwarf2_cu *cu)
return cu->producer_is_gas_2_39;
}
+/* Return true if CU is produced by GAS 2.39 or later. */
+
+static bool
+producer_is_gas_ge_2_39 (struct dwarf2_cu *cu)
+{
+ if (!cu->checked_producer)
+ check_producer (cu);
+
+ return cu->producer_is_gas_2_39 || cu->producer_is_gas_ge_2_40;
+}
+
/* Return the accessibility of DIE, as given by DW_AT_accessibility.
If that attribute is not available, return the appropriate
default. */