[PATCH] [gdb/tdep] Fix ARM_LINUX_JB_PC_EABI

[PATCH] [gdb/tdep] Fix ARM_LINUX_JB_PC_EABI

[Tom de Vries]

In arm-linux-tdep.c, ARM_LINUX_JB_PC_EABI is defined as 9, but it's been 1
since glibc 2.20.

See glibc commit 80a56cc3ee ("ARM: Add SystemTap probes to longjmp and
setjmp.").

Update it, allowing us to run into the gdb/26967 kfail.

Tested on arm-linux.

PR arm/tdep
Bug: https://www.sourceware.org/bugzilla/show_bug.cgi?id=31089
---
 gdb/arm-linux-tdep.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 43869e4fcfe..b0b6f3646f1 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -101,12 +101,26 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
    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 9 in the 
+   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.  */
 #define ARM_LINUX_JB_ELEMENT_SIZE	ARM_INT_REGISTER_SIZE
 #define ARM_LINUX_JB_PC_FPA		21
-#define ARM_LINUX_JB_PC_EABI		9
+#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
 
 /*
    Dynamic Linking on ARM GNU/Linux

base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d
-- 
2.35.3

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

[Luis Machado]

On 6/7/24 09:14, Tom de Vries wrote:
> In arm-linux-tdep.c, ARM_LINUX_JB_PC_EABI is defined as 9, but it's been 1
> since glibc 2.20.
> 
> See glibc commit 80a56cc3ee ("ARM: Add SystemTap probes to longjmp and
> setjmp.").
> 
> Update it, allowing us to run into the gdb/26967 kfail.
> 
> Tested on arm-linux.
> 
> PR arm/tdep
> Bug: https://www.sourceware.org/bugzilla/show_bug.cgi?id=31089
> ---
>  gdb/arm-linux-tdep.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
> index 43869e4fcfe..b0b6f3646f1 100644
> --- a/gdb/arm-linux-tdep.c
> +++ b/gdb/arm-linux-tdep.c
> @@ -101,12 +101,26 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
>     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 9 in the 
> +   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.  */
>  #define ARM_LINUX_JB_ELEMENT_SIZE	ARM_INT_REGISTER_SIZE
>  #define ARM_LINUX_JB_PC_FPA		21
> -#define ARM_LINUX_JB_PC_EABI		9
> +#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
>  
>  /*
>     Dynamic Linking on ARM GNU/Linux
> 
> base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d

Aren't those offsets supposed to be discovered at runtime, as opposed
to compile-time? We're looking at whether the inferior is built with
uclibc/glibc, and not gdb's own library versions, right?

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

[Tom de Vries]

On 6/7/24 10:40, Luis Machado wrote:
> On 6/7/24 09:14, Tom de Vries wrote:
>> In arm-linux-tdep.c, ARM_LINUX_JB_PC_EABI is defined as 9, but it's been 1
>> since glibc 2.20.
>>
>> See glibc commit 80a56cc3ee ("ARM: Add SystemTap probes to longjmp and
>> setjmp.").
>>
>> Update it, allowing us to run into the gdb/26967 kfail.
>>
>> Tested on arm-linux.
>>
>> PR arm/tdep
>> Bug: https://www.sourceware.org/bugzilla/show_bug.cgi?id=31089
>> ---
>>   gdb/arm-linux-tdep.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
>> index 43869e4fcfe..b0b6f3646f1 100644
>> --- a/gdb/arm-linux-tdep.c
>> +++ b/gdb/arm-linux-tdep.c
>> @@ -101,12 +101,26 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
>>      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 9 in the
>> +   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.  */
>>   #define ARM_LINUX_JB_ELEMENT_SIZE	ARM_INT_REGISTER_SIZE
>>   #define ARM_LINUX_JB_PC_FPA		21
>> -#define ARM_LINUX_JB_PC_EABI		9
>> +#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
>>   
>>   /*
>>      Dynamic Linking on ARM GNU/Linux
>>
>> base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d
> 
> Aren't those offsets supposed to be discovered at runtime, as opposed
> to compile-time? We're looking at whether the inferior is built with
> uclibc/glibc, and not gdb's own library versions, right?

This is a best effort, in absence of implementation of these enhancement 
PRs:
- add uclibc osabi (PR31854), and
- newlibc osabi enablement on arm (PR31856).

Using the compile time discovery ensures that at least a gdb build 
against a system uclibc or newlib will get the correct values to use on 
that system.

Likewise, there's no versioning in the osabi, so we use compile time 
discovery for the glibc version.  This is not correct, but it's the best 
we can do in absence of such versioning.

Thanks,
- Tom

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

[Luis Machado]

On 6/7/24 10:01, Tom de Vries wrote:
> On 6/7/24 10:40, Luis Machado wrote:
>> On 6/7/24 09:14, Tom de Vries wrote:
>>> In arm-linux-tdep.c, ARM_LINUX_JB_PC_EABI is defined as 9, but it's been 1
>>> since glibc 2.20.
>>>
>>> See glibc commit 80a56cc3ee ("ARM: Add SystemTap probes to longjmp and
>>> setjmp.").
>>>
>>> Update it, allowing us to run into the gdb/26967 kfail.
>>>
>>> Tested on arm-linux.
>>>
>>> PR arm/tdep
>>> Bug: https://www.sourceware.org/bugzilla/show_bug.cgi?id=31089
>>> ---
>>>   gdb/arm-linux-tdep.c | 18 ++++++++++++++++--
>>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
>>> index 43869e4fcfe..b0b6f3646f1 100644
>>> --- a/gdb/arm-linux-tdep.c
>>> +++ b/gdb/arm-linux-tdep.c
>>> @@ -101,12 +101,26 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
>>>      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 9 in the
>>> +   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.  */
>>>   #define ARM_LINUX_JB_ELEMENT_SIZE    ARM_INT_REGISTER_SIZE
>>>   #define ARM_LINUX_JB_PC_FPA        21
>>> -#define ARM_LINUX_JB_PC_EABI        9
>>> +#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
>>>     /*
>>>      Dynamic Linking on ARM GNU/Linux
>>>
>>> base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d
>>
>> Aren't those offsets supposed to be discovered at runtime, as opposed
>> to compile-time? We're looking at whether the inferior is built with
>> uclibc/glibc, and not gdb's own library versions, right?
> 
> This is a best effort, in absence of implementation of these enhancement PRs:
> - add uclibc osabi (PR31854), and
> - newlibc osabi enablement on arm (PR31856).
> 
> Using the compile time discovery ensures that at least a gdb build against a system uclibc or newlib will get the correct values to use on that system.
> 
> Likewise, there's no versioning in the osabi, so we use compile time discovery for the glibc version.  This is not correct, but it's the best we can do in absence of such versioning.
> 
> Thanks,
> - Tom

Ok, thanks for clarifying it. Sounds reasonable. And thanks for the patch.

Approved-By: Luis Machado <luis.machado@arm.com>

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

[Tom de Vries]

On 6/7/24 18:15, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> This is a best effort, in absence of implementation of these
> Tom> enhancement PRs:
> Tom> - add uclibc osabi (PR31854), and
> Tom> - newlibc osabi enablement on arm (PR31856).
> 
> I think it's a mistake to put host-dependent hacks into a tdep file,
> especially for a feature as minor as this one.

Hi Tom,

thanks for the post-commit review.

[ Like your other review, doesn't seem to have made it to the mail 
archive sofar for some reason. ]

So, your suggestion is to just hardcode it to 1?

Thanks,
- Tom

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

[Tom de Vries]

On 6/8/24 10:31, Tom de Vries wrote:
> On 6/7/24 18:15, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> This is a best effort, in absence of implementation of these
>> Tom> enhancement PRs:
>> Tom> - add uclibc osabi (PR31854), and
>> Tom> - newlibc osabi enablement on arm (PR31856).
>>
>> I think it's a mistake to put host-dependent hacks into a tdep file,
>> especially for a feature as minor as this one.
> 
> Hi Tom,
> 
> thanks for the post-commit review.
> 
> [ Like your other review, doesn't seem to have made it to the mail 
> archive sofar for some reason. ]
> 
> So, your suggestion is to just hardcode it to 1?
> 

Posted a patch implementing that approach here ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/209849.html ).

Thanks,
- Tom