public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI
@ 2024-06-11  9:57 Tom de Vries
  2024-06-11 10:29 ` Luis Machado
  0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2024-06-11  9:57 UTC (permalink / raw)
  To: gdb-patches

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
-- 
2.35.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI
  2024-06-11  9:57 [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI Tom de Vries
@ 2024-06-11 10:29 ` Luis Machado
  2024-06-11 12:25   ` Tom de Vries
  0 siblings, 1 reply; 5+ messages in thread
From: Luis Machado @ 2024-06-11 10:29 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI
  2024-06-11 10:29 ` Luis Machado
@ 2024-06-11 12:25   ` Tom de Vries
  2024-06-11 12:33     ` Luis Machado
  0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2024-06-11 12:25 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI
  2024-06-11 12:25   ` Tom de Vries
@ 2024-06-11 12:33     ` Luis Machado
  2024-06-12 17:08       ` Tom de Vries
  0 siblings, 1 reply; 5+ messages in thread
From: Luis Machado @ 2024-06-11 12:33 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI
  2024-06-11 12:33     ` Luis Machado
@ 2024-06-12 17:08       ` Tom de Vries
  0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2024-06-12 17:08 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-06-12 17:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-11  9:57 [PATCH] [gdb/tdep] Simplify ARM_LINUX_JB_PC_EABI Tom de Vries
2024-06-11 10:29 ` Luis Machado
2024-06-11 12:25   ` Tom de Vries
2024-06-11 12:33     ` Luis Machado
2024-06-12 17:08       ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).