[PATCH] gdb/arm: d0..d15 are 64-bit each, not 32-bit

[PATCH] gdb/arm: d0..d15 are 64-bit each, not 32-bit

[Yvan Roux]

Hi,

When unwinding the stack, the floating point registers d0 to d15
needs to be handled as double words, not words.

Only the first 8 registers has been confirmed fixed with this patch
on a STM32F407-DISC0 board, but the upper 8 registers on Cortex-M33
should be handled in the same way.

The test consisted of running a program compiled with float-abi=hard.
In the main function, a function took a double as an argument was
called. After the function call, a hardware timer was used to
trigger an interrupt.
In the debug session, a breakpoint was set in the function called
from main to verify the content of the registers using "info float"
and another breakpoint in the interrupt handler was used to check
the same registers using "info float" on frame 2 (the frame just
before the dummy frame created for the signal handler in gdb).

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com>
Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
---
 gdb/arm-tdep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 49664093f00..6c0aca274f5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3464,10 +3464,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
 
       /* Extended stack frame type used.  */
       fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
-      for (i = 0; i < 16; i++)
+      for (i = 0; i < 8; i++)
 	{
 	  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
-	  fpu_regs_stack_offset += 4;
+	  fpu_regs_stack_offset += 8;
 	}
       cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60);
       fpu_regs_stack_offset += 4;
@@ -3476,10 +3476,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
 	{
 	  /* Handle floating-point callee saved registers.  */
 	  fpu_regs_stack_offset = 0x90;
-	  for (i = 16; i < 32; i++)
+	  for (i = 8; i < 16; i++)
 	    {
 	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
-	      fpu_regs_stack_offset += 4;
+	      fpu_regs_stack_offset += 8;
 	    }
 
 	  arm_cache_set_active_sp_value (cache, tdep, unwound_sp + sp_r0_offset + 0xD0);
-- 
2.17.1

Re: [PATCH] gdb/arm: d0..d15 are 64-bit each, not 32-bit

[Luis Machado]

On 5/25/22 16:14, Yvan Roux wrote:
> Hi,
> 
> When unwinding the stack, the floating point registers d0 to d15
> needs to be handled as double words, not words.

needs -> need

> 
> Only the first 8 registers has been confirmed fixed with this patch
> on a STM32F407-DISC0 board, but the upper 8 registers on Cortex-M33
> should be handled in the same way.
> 
> The test consisted of running a program compiled with float-abi=hard.
> In the main function, a function took a double as an argument was

function took a double -> function taking a double?

> called. After the function call, a hardware timer was used to
> trigger an interrupt.
> In the debug session, a breakpoint was set in the function called
> from main to verify the content of the registers using "info float"
> and another breakpoint in the interrupt handler was used to check
> the same registers using "info float" on frame 2 (the frame just
> before the dummy frame created for the signal handler in gdb).
> 
> Signed-off-by: Torbj�rn SVENSSON <torbjorn.svensson@st.com>
> Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> ---
>   gdb/arm-tdep.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 49664093f00..6c0aca274f5 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3464,10 +3464,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   
>         /* Extended stack frame type used.  */
>         fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
> -      for (i = 0; i < 16; i++)
> +      for (i = 0; i < 8; i++)
>   	{
>   	  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> -	  fpu_regs_stack_offset += 4;
> +	  fpu_regs_stack_offset += 8;
>   	}
>         cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60);
>         fpu_regs_stack_offset += 4;
> @@ -3476,10 +3476,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	{
>   	  /* Handle floating-point callee saved registers.  */
>   	  fpu_regs_stack_offset = 0x90;
> -	  for (i = 16; i < 32; i++)
> +	  for (i = 8; i < 16; i++)
>   	    {
>   	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> -	      fpu_regs_stack_offset += 4;
> +	      fpu_regs_stack_offset += 8;
>   	    }
>   
>   	  arm_cache_set_active_sp_value (cache, tdep, unwound_sp + sp_r0_offset + 0xD0);

Thanks. This is OK with the commit message nits fixed.

Re: [PATCH] gdb/arm: d0..d15 are 64-bit each, not 32-bit

[Christophe Lyon]

On 5/25/22 17:14, Yvan Roux via Gdb-patches wrote:
> Hi,
> 
> When unwinding the stack, the floating point registers d0 to d15
> needs to be handled as double words, not words.
> 
> Only the first 8 registers has been confirmed fixed with this patch
> on a STM32F407-DISC0 board, but the upper 8 registers on Cortex-M33
> should be handled in the same way.
> 
> The test consisted of running a program compiled with float-abi=hard.
> In the main function, a function took a double as an argument was
> called. After the function call, a hardware timer was used to
> trigger an interrupt.
> In the debug session, a breakpoint was set in the function called
> from main to verify the content of the registers using "info float"
> and another breakpoint in the interrupt handler was used to check
> the same registers using "info float" on frame 2 (the frame just
> before the dummy frame created for the signal handler in gdb).

I guess that's because 'info float' shows these registers as 
double-precision, and the confusion comes from the fact that the Arm 
v8-M ARM describes this stack frame in terms of single-precision 
registers (S0..S15, then S16..S32)?

> 
> Signed-off-by: Torbj�rn SVENSSON <torbjorn.svensson@st.com>
> Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> ---
>   gdb/arm-tdep.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 49664093f00..6c0aca274f5 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3464,10 +3464,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   
>         /* Extended stack frame type used.  */
>         fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
> -      for (i = 0; i < 16; i++)
> +      for (i = 0; i < 8; i++)
>   	{
>   	  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> -	  fpu_regs_stack_offset += 4;
> +	  fpu_regs_stack_offset += 8;
>   	}
>         cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60);
>         fpu_regs_stack_offset += 4;
> @@ -3476,10 +3476,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	{
>   	  /* Handle floating-point callee saved registers.  */
>   	  fpu_regs_stack_offset = 0x90;
> -	  for (i = 16; i < 32; i++)
> +	  for (i = 8; i < 16; i++)
>   	    {
>   	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> -	      fpu_regs_stack_offset += 4;
> +	      fpu_regs_stack_offset += 8;
>   	    }
>   
>   	  arm_cache_set_active_sp_value (cache, tdep, unwound_sp + sp_r0_offset + 0xD0);

Re: [PATCH] gdb/arm: d0..d15 are 64-bit each, not 32-bit

[Yvan Roux]

On Thu, May 26, 2022 at 08:39:05AM +0100, Luis Machado wrote:
> On 5/25/22 16:14, Yvan Roux wrote:
> > Hi,
> > 
> > When unwinding the stack, the floating point registers d0 to d15
> > needs to be handled as double words, not words.
> 
> needs -> need
> 
> > 
> > Only the first 8 registers has been confirmed fixed with this patch
> > on a STM32F407-DISC0 board, but the upper 8 registers on Cortex-M33
> > should be handled in the same way.
> > 
> > The test consisted of running a program compiled with float-abi=hard.
> > In the main function, a function took a double as an argument was
> 
> function took a double -> function taking a double?
> 
> > called. After the function call, a hardware timer was used to
> > trigger an interrupt.
> > In the debug session, a breakpoint was set in the function called
> > from main to verify the content of the registers using "info float"
> > and another breakpoint in the interrupt handler was used to check
> > the same registers using "info float" on frame 2 (the frame just
> > before the dummy frame created for the signal handler in gdb).
> > 
> > Signed-off-by: Torbj�rn SVENSSON <torbjorn.svensson@st.com>
> > Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> > ---
> >   gdb/arm-tdep.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> > index 49664093f00..6c0aca274f5 100644
> > --- a/gdb/arm-tdep.c
> > +++ b/gdb/arm-tdep.c
> > @@ -3464,10 +3464,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
> >         /* Extended stack frame type used.  */
> >         fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
> > -      for (i = 0; i < 16; i++)
> > +      for (i = 0; i < 8; i++)
> >   	{
> >   	  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> > -	  fpu_regs_stack_offset += 4;
> > +	  fpu_regs_stack_offset += 8;
> >   	}
> >         cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60);
> >         fpu_regs_stack_offset += 4;
> > @@ -3476,10 +3476,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
> >   	{
> >   	  /* Handle floating-point callee saved registers.  */
> >   	  fpu_regs_stack_offset = 0x90;
> > -	  for (i = 16; i < 32; i++)
> > +	  for (i = 8; i < 16; i++)
> >   	    {
> >   	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> > -	      fpu_regs_stack_offset += 4;
> > +	      fpu_regs_stack_offset += 8;
> >   	    }
> >   	  arm_cache_set_active_sp_value (cache, tdep, unwound_sp + sp_r0_offset + 0xD0);
> 
> Thanks. This is OK with the commit message nits fixed.

Thanks for the review, here is the updated version of the patch.
Can you apply it as well?

When unwinding the stack, the floating point registers d0 to d15
need to be handled as double words, not words.

Only the first 8 registers has been confirmed fixed with this patch
on a STM32F407-DISC0 board, but the upper 8 registers on Cortex-M33
should be handled in the same way.

The test consisted of running a program compiled with float-abi=hard.
In the main function, a function taking a double as an argument was
called. After the function call, a hardware timer was used to
trigger an interrupt.
In the debug session, a breakpoint was set in the function called
from main to verify the content of the registers using "info float"
and another breakpoint in the interrupt handler was used to check
the same registers using "info float" on frame 2 (the frame just
before the dummy frame created for the signal handler in gdb).

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com>
Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
---
 gdb/arm-tdep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 49664093f00..6c0aca274f5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3464,10 +3464,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
 
       /* Extended stack frame type used.  */
       fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
-      for (i = 0; i < 16; i++)
+      for (i = 0; i < 8; i++)
 	{
 	  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
-	  fpu_regs_stack_offset += 4;
+	  fpu_regs_stack_offset += 8;
 	}
       cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60);
       fpu_regs_stack_offset += 4;
@@ -3476,10 +3476,10 @@ arm_m_exception_cache (struct frame_info *this_frame)
 	{
 	  /* Handle floating-point callee saved registers.  */
 	  fpu_regs_stack_offset = 0x90;
-	  for (i = 16; i < 32; i++)
+	  for (i = 8; i < 16; i++)
 	    {
 	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
-	      fpu_regs_stack_offset += 4;
+	      fpu_regs_stack_offset += 8;
 	    }
 
 	  arm_cache_set_active_sp_value (cache, tdep, unwound_sp + sp_r0_offset + 0xD0);
-- 
2.17.1