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

Message ID 20221011080025.567502-1-torbjorn.svensson@foss.st.com
State New
Headers
Series [v2] gdb/arm: Unwind S-registers for exception frames |

Commit Message

Torbjorn SVENSSON Oct. 11, 2022, 8 a.m. UTC
  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(-)
  

Comments

Luis Machado Oct. 11, 2022, 4:34 p.m. UTC | #1
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?
  

Patch

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;
 		    }
 		}