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

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

On 6/11/24 14:33, Luis Machado wrote:
> On 6/11/24 13:25, Tom de Vries wrote:
>> 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
>>
> 
> Excellent. Thanks Tom.

I've sent a v2 with that bit included ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/209918.html ).

Thanks,
- Tom