Re: [PATCH 1/3] gdb/arm: Set the correct address to the FPU register on

[Luis Machado <luis.machado@arm.com>]

On 5/17/22 10:49, Torbjorn SVENSSON via Gdb-patches wrote:
> Hello,
> 
> 
> ST Restricted
> 
>> -----Original Message-----
>> From: Christophe Lyon <christophe.lyon@arm.com>
>> Sent: den 16 maj 2022 16:47
>> To: Yvan ROUX - foss <yvan.roux@foss.st.com>; gdb-
>> patches@sourceware.org
>> Cc: Torbjorn SVENSSON <torbjorn.svensson@st.com>
>> Subject: Re: [PATCH 1/3] gdb/arm: Set the correct address to the FPU
>> register on
>>
>> Hi!
>>
>>
>> On 5/16/22 15:58, Yvan Roux via Gdb-patches wrote:
>>> Registers offsets weren't computed from SP address.
>>>
>>> Signed-off-by: Torbj�rn SVENSSON <torbjorn.svensson@st.com>
>>> Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
>>> ---
>>>    gdb/arm-tdep.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>>> index 49664093f00..c37254c2ce1 100644
>>> --- a/gdb/arm-tdep.c
>>> +++ b/gdb/arm-tdep.c
>>> @@ -3475,7 +3475,7 @@ arm_m_exception_cache (struct frame_info
>> *this_frame)
>>>          if (tdep->have_sec_ext && !default_callee_register_stacking)
>>>    	{
>>>    	  /* Handle floating-point callee saved registers.  */
>>> -	  fpu_regs_stack_offset = 0x90;
>>> +	  fpu_regs_stack_offset = unwound_sp + 0x90;
>>
>> Sorry, it looks like this was my mistake when I committed ef273377587d.
>>
>> I haven't checked the manual, and I may have forgotten, but shouldn't
>> this be
>> fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x90?
> 
> As I see it, it can be either
> fpu_regs_stack_offset = unwound_sp + 0x90;
> or
> fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x68;
> The offset values (0x90 and 0x68) can be seen in the "Stack Frame" chapter at https://developer.arm.com/documentation/100701/0200/Exception-handlers

It would be nice to #define these constants and put some comments pointing at this documentation.

> 
> Based on the rest of the code, maybe "unwound_sp + sp_r0_offset + 0x68" is the preferred expression here as it removes the "integer callee saved" part from the equation.
>   
>> How did you test this? Does your patch actually work?
>   
> I've not tested the code on target (I don't know how to reliably trigger the code), I've just looked at how the rest of the register values are handled in the same block of code in GDB and they are all using the unwound_sp variable to identify the appropriate address.

Was this spotted on visual inspection only? Would it be possible to craft some code that runs into an exception and then forces GDB to go through the frame restoring the registers?