[PATCH v2] gdb/arm: Unwind S-registers for exception frames

[PATCH v2] gdb/arm: Unwind S-registers for exception frames

[Torbjörn SVENSSON]

After sending the v1 patch yesterday, I had an epiphany that the solution could be simplified.
The v2 of the patch is an alternative implementation that appears to work equally well.
Which do you prefer?


Save the address on the stack that contains the value for the
S-register when unwinding, just like already done for the
D-registers.

Signed-off-by: Torbjörn SVENSSON  <torbjorn.svensson@foss.st.com>
---
 gdb/arm-tdep.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d357066653b..f725d437825 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3625,10 +3625,13 @@ arm_m_exception_cache (struct frame_info *this_frame)
 	  if (read_fp_regs_from_stack)
 	    {
 	      CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x20;
-	      for (int i = 0; i < 8; i++)
+	      for (int i = 0; i < 16; i++)
 		{
-		  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
-		  addr += 8;
+		  if (tdep->have_s_pseudos)
+		    cache->saved_regs[tdep->s_pseudo_base + i].set_addr (addr);
+		  if (i % 2 == 0)
+		    cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
+		  addr += 4;
 		}
 	    }
 	  cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp
@@ -3641,10 +3644,14 @@ arm_m_exception_cache (struct frame_info *this_frame)
 	      if (read_fp_regs_from_stack)
 		{
 		  CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x68;
-		  for (int i = 8; i < 16; i++)
+		  for (int i = 16; i < 32; i++)
 		    {
-		      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
-		      addr += 8;
+		      if (tdep->have_s_pseudos)
+			cache->saved_regs[tdep->s_pseudo_base + i]
+			  .set_addr (addr);
+		      if (i % 2 == 0)
+			cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
+		      addr += 4;
 		    }
 		}
 
-- 
2.25.1

Re: [PATCH v2] gdb/arm: Unwind S-registers for exception frames

[Luis Machado]

Hi Torbjörn,

On 10/11/22 09:00, Torbjörn SVENSSON wrote:
> After sending the v1 patch yesterday, I had an epiphany that the solution could be simplified.
> The v2 of the patch is an alternative implementation that appears to work equally well.
> Which do you prefer?
> 
> 
> Save the address on the stack that contains the value for the
> S-register when unwinding, just like already done for the
> D-registers.
> 
> Signed-off-by: Torbjörn SVENSSON  <torbjorn.svensson@foss.st.com>
> ---
>   gdb/arm-tdep.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index d357066653b..f725d437825 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3625,10 +3625,13 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	  if (read_fp_regs_from_stack)
>   	    {
>   	      CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x20;
> -	      for (int i = 0; i < 8; i++)
> +	      for (int i = 0; i < 16; i++)
>   		{
> -		  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
> -		  addr += 8;
> +		  if (tdep->have_s_pseudos)
> +		    cache->saved_regs[tdep->s_pseudo_base + i].set_addr (addr);
> +		  if (i % 2 == 0)
> +		    cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
> +		  addr += 4;
>   		}
>   	    }
>   	  cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp
> @@ -3641,10 +3644,14 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	      if (read_fp_regs_from_stack)
>   		{
>   		  CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x68;
> -		  for (int i = 8; i < 16; i++)
> +		  for (int i = 16; i < 32; i++)
>   		    {
> -		      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
> -		      addr += 8;
> +		      if (tdep->have_s_pseudos)
> +			cache->saved_regs[tdep->s_pseudo_base + i]
> +			  .set_addr (addr);
> +		      if (i % 2 == 0)
> +			cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
> +		      addr += 4;
>   		    }
>   		}
>   

Although this is a working solution to the problem, I'm afraid the proper way to address this
issue is to support pseudo-registers based on frame registers. But from past discussions, there
doesn't seem to be consensus about the right way to do it or even if we should do it.

See https://sourceware.org/pipermail/gdb-patches/2018-May/149243.html.

Simon, was this ever pursued after this thread?