Re: [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI

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

On 6/11/24 12:29, Luis Machado wrote:
> On 6/11/24 10:57, Tom de Vries wrote:
>> In commit 1a7d840a216 ("[gdb/tdep] Fix ARM_LINUX_JB_PC_EABI"), in absense of
>> osabi settings for newlib and uclibc for arm, I chose a best-effort approach
>> using ifdefs.
>>
>> Post-commit review [1] pointed out that this may be causing more problems than
>> it's worth.
>>
>> Fix this by removing the ifdefs and simply defining ARM_LINUX_JB_PC_EABI to 1.
>>
>> Rebuild on x86_64-linux with --enable-targets=all.
>>
>> Fixes: 1a7d840a216 ("[gdb/tdep] Fix ARM_LINUX_JB_PC_EABI")
>>
>> [1] https://sourceware.org/pipermail/gdb-patches/2024-June/209779.html
>> ---
>>   gdb/arm-linux-tdep.c | 29 ++++++++---------------------
>>   1 file changed, 8 insertions(+), 21 deletions(-)
>>
>> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
>> index b0b6f3646f1..c8a9936b5c1 100644
>> --- a/gdb/arm-linux-tdep.c
>> +++ b/gdb/arm-linux-tdep.c
>> @@ -98,29 +98,16 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
>>   
>>      The location of saved registers in this buffer (in particular the PC
>>      to use after longjmp is called) varies depending on the ABI (in
>> -   particular the FP model) and also (possibly) the C Library.
>> -
>> -   For glibc, eglibc, and uclibc the following holds:  If the FP model is
>> -   SoftVFP or VFP (which implies EABI) then the PC is at offset 1 or 9 in the
>> -   buffer.  This is also true for the SoftFPA model.  However, for the FPA
>> -   model the PC is at offset 21 in the buffer.  */
>> +   particular the FP model) and also (possibly) the C Library.  */
>>   #define ARM_LINUX_JB_ELEMENT_SIZE	ARM_INT_REGISTER_SIZE
>> +/* For the FPA model the PC is at offset 21 in the buffer.  */
>>   #define ARM_LINUX_JB_PC_FPA		21
>> -#ifdef __UCLIBC__
>> -# define ARM_LINUX_JB_PC_EABI		9
>> -#else
>> -# ifdef __GLIBC__
>> -#  if __GLIBC_PREREQ(2, 20)
>> -/* This has been 1 since glibc 2.20, see glibc commit 80a56cc3ee ("ARM: Add
>> -   SystemTap probes to longjmp and setjmp.").  */
>> -#   define ARM_LINUX_JB_PC_EABI		1
>> -#  else
>> -#   define ARM_LINUX_JB_PC_EABI		9
>> -#  endif
>> -# else
>> -#  define ARM_LINUX_JB_PC_EABI		9
>> -# endif
>> -#endif
>> +/* For glibc 2.20 and later the PC is at offset 1, see glibc commit 80a56cc3ee
>> +   ("ARM: Add SystemTap probes to longjmp and setjmp.").
>> +   For newlib and uclibc, this is not correct, we need osabi settings to deal
>> +   with those, see PR31854 and PR31856.  Likewise for older versions of
>> +   glibc.  */
>> +#define ARM_LINUX_JB_PC_EABI		1
>>   
>>   /*
>>      Dynamic Linking on ARM GNU/Linux
>>
>> base-commit: 60e221e9b7fae5ff40b79b93bdbff909989c2bae
> 
> Should we document somewhere that we dropped support for
> the old pre 2.20 glibc buffer offset though? This is a minor
> but breaking change after all.

Fine by me.  How about this, in NEWS:
...
* For ARM targets, the offset of the pc in the jmp_buf has been fixed to
   match glibc 2.20 and later.  This should only matter when not using
   libc probes.  This may cause breakage when using an incompatible libc,
   like uclibc or newlib, or an older glibc.
...

Thanks,
- Tom