public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/arm: Sync sp with other *sp registers
@ 2022-06-30  7:13 Yvan Roux
  2022-07-06 12:34 ` Luis Machado
  0 siblings, 1 reply; 3+ messages in thread
From: Yvan Roux @ 2022-06-30  7:13 UTC (permalink / raw)
  To: gdb-patches, Luis Machado; +Cc: Torbjorn SVENSSON

Hi,

For Arm Cortex-M33 with security extensions, there are 4 different
stack pointers (msp_s, msp_ns, psp_s, psp_ns), without security
extensions and for other Cortex-M targets, there are 2 different
stack pointers (msp and psp).

This patch allows arch dependent unwinding override register values
for registers not covered by the DWARF data.  That means that sp
will always be in sync with one of the real stack pointers on
Arm targets that contains more than one stack pointer.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
---
 gdb/arm-tdep.c     | 94 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/dwarf2/frame.c | 58 ++++++++++++++++++++++++----
 gdb/dwarf2/frame.h |  9 +++++
 3 files changed, 153 insertions(+), 8 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 3a1b52c2380..48711cb2a65 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -4956,6 +4956,98 @@ arm_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
     }
 }
 
+/* Return the value of the unspecified register REGNUM for THIS_FRAME.  */
+
+static struct value *
+arm_dwarf2_frame_prev_unspecified_reg (struct frame_info *this_frame,
+				       int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+  bool override_with_sp_value = false;
+
+  if (tdep->have_sec_ext)
+    {
+      CORE_ADDR sp, msp_s, msp_ns, psp_s, psp_ns;
+      sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+      msp_s = get_frame_register_unsigned (this_frame,
+					   tdep->m_profile_msp_s_regnum);
+      msp_ns = get_frame_register_unsigned (this_frame,
+					    tdep->m_profile_msp_ns_regnum);
+      psp_s = get_frame_register_unsigned (this_frame,
+					   tdep->m_profile_psp_s_regnum);
+      psp_ns = get_frame_register_unsigned (this_frame,
+					    tdep->m_profile_psp_ns_regnum);
+
+      if (regnum == tdep->m_profile_msp_regnum && (msp_s == sp || msp_ns == sp))
+	{
+	  /* Use value of SP for MSP register.  */
+	  override_with_sp_value = true;
+	}
+      else if (regnum == tdep->m_profile_msp_s_regnum && msp_s == sp)
+	{
+	  /* Use value of SP for MSP_S register.  */
+	  override_with_sp_value = true;
+	}
+      else if (regnum == tdep->m_profile_msp_ns_regnum && msp_ns == sp)
+	{
+	  /* Use value of SP for MSP_NS register.  */
+	  override_with_sp_value = true;
+	}
+      else if (regnum == tdep->m_profile_psp_regnum && (psp_s == sp
+							|| psp_ns == sp))
+	{
+	  /* Use value of SP for PSP register.  */
+	  override_with_sp_value = true;
+	}
+      else if (regnum == tdep->m_profile_psp_s_regnum && psp_s == sp)
+	{
+	  /* Use value of SP for PSP_S register.  */
+	  override_with_sp_value = true;
+	}
+      else if (regnum == tdep->m_profile_psp_ns_regnum && psp_ns == sp)
+	{
+	  /* Use value of SP for PSP_NS register.  */
+	  override_with_sp_value = true;
+	}
+    }
+  else if (tdep->is_m)
+    {
+      CORE_ADDR sp, msp, psp;
+      sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+      msp = get_frame_register_unsigned (this_frame,
+					 tdep->m_profile_msp_regnum);
+      psp = get_frame_register_unsigned (this_frame,
+					 tdep->m_profile_psp_regnum);
+
+      if (regnum == tdep->m_profile_msp_regnum && sp == msp)
+	{
+	  /* Use value of SP for MSP register.  */
+	  override_with_sp_value = true;
+	}
+      else if (regnum == tdep->m_profile_psp_regnum && sp == psp)
+	{
+	  /* Use value of SP for PSP register.  */
+	  override_with_sp_value = true;
+	}
+    }
+
+  if (override_with_sp_value)
+    {
+      CORE_ADDR prev_sp;
+      struct frame_info *prev_frame = get_prev_frame (this_frame);
+      if (prev_frame)
+	prev_sp = get_frame_register_unsigned (prev_frame, ARM_SP_REGNUM);
+      else
+	prev_sp = get_frame_base (this_frame);
+
+      return frame_unwind_got_constant (this_frame, regnum, prev_sp);
+    }
+
+  /* Don't know how to fetch this value, use generic method.  */
+  return NULL;
+}
+
 /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
    the buffer to be NEW_LEN bytes ending at ENDADDR.  Return
    NULL if an error occurs.  BUF is freed.  */
@@ -10438,6 +10530,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   gdbarch_init_osabi (info, gdbarch);
 
   dwarf2_frame_set_init_reg (gdbarch, arm_dwarf2_frame_init_reg);
+  dwarf2_frame_set_prev_unspecified_reg (gdbarch,
+					 arm_dwarf2_frame_prev_unspecified_reg);
 
   /* Add some default predicates.  */
   if (is_m)
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 5878d72f922..c4b15b2d240 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -595,6 +595,10 @@ struct dwarf2_frame_ops
   /* Convert .eh_frame register number to DWARF register number, or
      adjust .debug_frame register number.  */
   int (*adjust_regnum) (struct gdbarch *, int, int);
+
+  /* Return the value of the unspecified register REGNUM for THIS_FRAME. */
+  struct value * (*prev_unspecified_reg) (struct frame_info *this_frame,
+					  int regnum);
 };
 
 /* Default architecture-specific register state initialization
@@ -677,6 +681,50 @@ dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
   ops->init_reg (gdbarch, regnum, reg, this_frame);
 }
 
+/* Set the architecture-specific unspecified register unwinding
+   function for GDBARCH to PREV_UNSPECIFIED_REG.  */
+
+void
+dwarf2_frame_set_prev_unspecified_reg (struct gdbarch *gdbarch,
+				       struct value * (*prev_unspecified_reg)
+					      (struct frame_info *this_frame,
+					       int regnum))
+{
+  struct dwarf2_frame_ops *ops
+    = (struct dwarf2_frame_ops *) gdbarch_data (gdbarch, dwarf2_frame_data);
+
+  ops->prev_unspecified_reg = prev_unspecified_reg;
+}
+
+/* Return the value of the unspecified register REGNUM for THIS_FRAME.  */
+
+static struct value *
+dwarf2_frame_prev_unspecified_reg (struct gdbarch *gdbarch,
+				   struct dwarf2_frame_cache *cache,
+				   int regnum, struct frame_info *this_frame)
+{
+  struct dwarf2_frame_ops *ops
+    = (struct dwarf2_frame_ops *) gdbarch_data (gdbarch, dwarf2_frame_data);
+
+  if (ops->prev_unspecified_reg)
+    {
+      struct value * val = ops->prev_unspecified_reg (this_frame, regnum);
+
+      if (val)
+	return val;
+    }
+
+  /* GCC, in its infinite wisdom decided to not provide unwind
+     information for registers that are "same value".  Since
+     DWARF2 (3 draft 7) doesn't define such behavior, said
+     registers are actually undefined (which is different to CFI
+     "undefined").  Code above issues a complaint about this.
+     Here just fudge the books, assume GCC, and that the value is
+     more inner on the stack.  */
+  return frame_unwind_got_register (this_frame, regnum, regnum);
+}
+
+
 /* Set the architecture-specific signal trampoline recognition
    function for GDBARCH to SIGNAL_FRAME_P.  */
 
@@ -1190,14 +1238,8 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
       return frame_unwind_got_constant (this_frame, regnum, addr);
 
     case DWARF2_FRAME_REG_UNSPECIFIED:
-      /* GCC, in its infinite wisdom decided to not provide unwind
-	 information for registers that are "same value".  Since
-	 DWARF2 (3 draft 7) doesn't define such behavior, said
-	 registers are actually undefined (which is different to CFI
-	 "undefined").  Code above issues a complaint about this.
-	 Here just fudge the books, assume GCC, and that the value is
-	 more inner on the stack.  */
-      return frame_unwind_got_register (this_frame, regnum, regnum);
+      return dwarf2_frame_prev_unspecified_reg (gdbarch, cache, regnum,
+						this_frame);
 
     case DWARF2_FRAME_REG_SAME_VALUE:
       return frame_unwind_got_register (this_frame, regnum, regnum);
diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h
index 04ec1e06c48..7e34495a1a0 100644
--- a/gdb/dwarf2/frame.h
+++ b/gdb/dwarf2/frame.h
@@ -210,6 +210,15 @@ extern void dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
 					     struct dwarf2_frame_state_reg *,
 					     struct frame_info *));
 
+/* Set the architecture-specific unspecified register unwinding
+   function for GDBARCH to PREV_UNSPECIFIED_REG.  */
+
+extern void
+  dwarf2_frame_set_prev_unspecified_reg (struct gdbarch *gdbarch,
+					 struct value * (*prev_unspecified_reg)
+						(struct frame_info *this_frame,
+						 int regnum));
+
 /* Set the architecture-specific signal trampoline recognition
    function for GDBARCH to SIGNAL_FRAME_P.  */
 
-- 
2.17.1


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

* Re: [PATCH] gdb/arm: Sync sp with other *sp registers
  2022-06-30  7:13 [PATCH] gdb/arm: Sync sp with other *sp registers Yvan Roux
@ 2022-07-06 12:34 ` Luis Machado
  2022-07-07  9:11   ` Yvan Roux
  0 siblings, 1 reply; 3+ messages in thread
From: Luis Machado @ 2022-07-06 12:34 UTC (permalink / raw)
  To: Yvan Roux, gdb-patches; +Cc: Torbjorn SVENSSON

Hi,

On 6/30/22 08:13, Yvan Roux wrote:
> Hi,
> 
> For Arm Cortex-M33 with security extensions, there are 4 different
> stack pointers (msp_s, msp_ns, psp_s, psp_ns), without security
> extensions and for other Cortex-M targets, there are 2 different
> stack pointers (msp and psp).
> 
> This patch allows arch dependent unwinding override register values
> for registers not covered by the DWARF data.  That means that sp
> will always be in sync with one of the real stack pointers on
> Arm targets that contains more than one stack pointer.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> ---
>   gdb/arm-tdep.c     | 94 ++++++++++++++++++++++++++++++++++++++++++++++
>   gdb/dwarf2/frame.c | 58 ++++++++++++++++++++++++----
>   gdb/dwarf2/frame.h |  9 +++++
>   3 files changed, 153 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 3a1b52c2380..48711cb2a65 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -4956,6 +4956,98 @@ arm_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>       }
>   }
>   
> +/* Return the value of the unspecified register REGNUM for THIS_FRAME.  */
> +
> +static struct value *
> +arm_dwarf2_frame_prev_unspecified_reg (struct frame_info *this_frame,
> +				       int regnum)
> +{
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +  arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch);
> +  bool override_with_sp_value = false;
> +
> +  if (tdep->have_sec_ext)
> +    {
> +      CORE_ADDR sp, msp_s, msp_ns, psp_s, psp_ns;
> +      sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
> +      msp_s = get_frame_register_unsigned (this_frame,
> +					   tdep->m_profile_msp_s_regnum);
> +      msp_ns = get_frame_register_unsigned (this_frame,
> +					    tdep->m_profile_msp_ns_regnum);
> +      psp_s = get_frame_register_unsigned (this_frame,
> +					   tdep->m_profile_psp_s_regnum);
> +      psp_ns = get_frame_register_unsigned (this_frame,
> +					    tdep->m_profile_psp_ns_regnum);
> +
> +      if (regnum == tdep->m_profile_msp_regnum && (msp_s == sp || msp_ns == sp))
> +	{
> +	  /* Use value of SP for MSP register.  */
> +	  override_with_sp_value = true;
> +	}
> +      else if (regnum == tdep->m_profile_msp_s_regnum && msp_s == sp)
> +	{
> +	  /* Use value of SP for MSP_S register.  */
> +	  override_with_sp_value = true;
> +	}
> +      else if (regnum == tdep->m_profile_msp_ns_regnum && msp_ns == sp)
> +	{
> +	  /* Use value of SP for MSP_NS register.  */
> +	  override_with_sp_value = true;
> +	}
> +      else if (regnum == tdep->m_profile_psp_regnum && (psp_s == sp
> +							|| psp_ns == sp))
> +	{
> +	  /* Use value of SP for PSP register.  */
> +	  override_with_sp_value = true;
> +	}
> +      else if (regnum == tdep->m_profile_psp_s_regnum && psp_s == sp)
> +	{
> +	  /* Use value of SP for PSP_S register.  */
> +	  override_with_sp_value = true;
> +	}
> +      else if (regnum == tdep->m_profile_psp_ns_regnum && psp_ns == sp)
> +	{
> +	  /* Use value of SP for PSP_NS register.  */
> +	  override_with_sp_value = true;
> +	}
> +    }
> +  else if (tdep->is_m)
> +    {
> +      CORE_ADDR sp, msp, psp;
> +      sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
> +      msp = get_frame_register_unsigned (this_frame,
> +					 tdep->m_profile_msp_regnum);
> +      psp = get_frame_register_unsigned (this_frame,
> +					 tdep->m_profile_psp_regnum);
> +
> +      if (regnum == tdep->m_profile_msp_regnum && sp == msp)
> +	{
> +	  /* Use value of SP for MSP register.  */
> +	  override_with_sp_value = true;
> +	}
> +      else if (regnum == tdep->m_profile_psp_regnum && sp == psp)
> +	{
> +	  /* Use value of SP for PSP register.  */
> +	  override_with_sp_value = true;
> +	}
> +    }
> +
> +  if (override_with_sp_value)
> +    {
> +      CORE_ADDR prev_sp;
> +      struct frame_info *prev_frame = get_prev_frame (this_frame);
> +      if (prev_frame)
> +	prev_sp = get_frame_register_unsigned (prev_frame, ARM_SP_REGNUM);
> +      else
> +	prev_sp = get_frame_base (this_frame);
> +
> +      return frame_unwind_got_constant (this_frame, regnum, prev_sp);
> +    }
> +
> +  /* Don't know how to fetch this value, use generic method.  */
> +  return NULL;
> +}
> +
>   /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
>      the buffer to be NEW_LEN bytes ending at ENDADDR.  Return
>      NULL if an error occurs.  BUF is freed.  */
> @@ -10438,6 +10530,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     gdbarch_init_osabi (info, gdbarch);
>   
>     dwarf2_frame_set_init_reg (gdbarch, arm_dwarf2_frame_init_reg);
> +  dwarf2_frame_set_prev_unspecified_reg (gdbarch,
> +					 arm_dwarf2_frame_prev_unspecified_reg);
>   
>     /* Add some default predicates.  */
>     if (is_m)
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index 5878d72f922..c4b15b2d240 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -595,6 +595,10 @@ struct dwarf2_frame_ops
>     /* Convert .eh_frame register number to DWARF register number, or
>        adjust .debug_frame register number.  */
>     int (*adjust_regnum) (struct gdbarch *, int, int);
> +
> +  /* Return the value of the unspecified register REGNUM for THIS_FRAME. */
> +  struct value * (*prev_unspecified_reg) (struct frame_info *this_frame,
> +					  int regnum);
>   };
>   
>   /* Default architecture-specific register state initialization
> @@ -677,6 +681,50 @@ dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>     ops->init_reg (gdbarch, regnum, reg, this_frame);
>   }
>   
> +/* Set the architecture-specific unspecified register unwinding
> +   function for GDBARCH to PREV_UNSPECIFIED_REG.  */
> +
> +void
> +dwarf2_frame_set_prev_unspecified_reg (struct gdbarch *gdbarch,
> +				       struct value * (*prev_unspecified_reg)
> +					      (struct frame_info *this_frame,
> +					       int regnum))
> +{
> +  struct dwarf2_frame_ops *ops
> +    = (struct dwarf2_frame_ops *) gdbarch_data (gdbarch, dwarf2_frame_data);
> +
> +  ops->prev_unspecified_reg = prev_unspecified_reg;
> +}

I'm wondering if we really need to introduce a hook here. I understand we may need to sidestep the dwarf unwinder
to do custom handling of the msp*/psp* registers, but wouldn't we be able to achieve the same with the existing
dwarf hooks?

For example, we could register a hook with dwarf2_frame_set_init_reg (), like aarch64 (see gdb/aarch64-tdep.c:aarch64_dwarf2_frame_init_reg).

With such a hook, we could define our own rules for individual registers through DWARF2_FRAME_REG_FN.

Would that work?

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

* Re: [PATCH] gdb/arm: Sync sp with other *sp registers
  2022-07-06 12:34 ` Luis Machado
@ 2022-07-07  9:11   ` Yvan Roux
  0 siblings, 0 replies; 3+ messages in thread
From: Yvan Roux @ 2022-07-07  9:11 UTC (permalink / raw)
  To: gdb-patches, Luis Machado; +Cc: Torbjorn SVENSSON

On Wed, Jul 06, 2022 at 01:34:59PM +0100, Luis Machado wrote:
> Hi,
> 
> On 6/30/22 08:13, Yvan Roux wrote:
> > Hi,
> > 
> > For Arm Cortex-M33 with security extensions, there are 4 different
> > stack pointers (msp_s, msp_ns, psp_s, psp_ns), without security
> > extensions and for other Cortex-M targets, there are 2 different
> > stack pointers (msp and psp).
> > 
> > This patch allows arch dependent unwinding override register values
> > for registers not covered by the DWARF data.  That means that sp
> > will always be in sync with one of the real stack pointers on
> > Arm targets that contains more than one stack pointer.
> > 
> > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> > Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> > ---
> >   gdb/arm-tdep.c     | 94 ++++++++++++++++++++++++++++++++++++++++++++++
> >   gdb/dwarf2/frame.c | 58 ++++++++++++++++++++++++----
> >   gdb/dwarf2/frame.h |  9 +++++
> >   3 files changed, 153 insertions(+), 8 deletions(-)
> > 
> > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> > index 3a1b52c2380..48711cb2a65 100644
> > --- a/gdb/arm-tdep.c
> > +++ b/gdb/arm-tdep.c
> > @@ -4956,6 +4956,98 @@ arm_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> >       }
> >   }
> > +/* Return the value of the unspecified register REGNUM for THIS_FRAME.  */
> > +
> > +static struct value *
> > +arm_dwarf2_frame_prev_unspecified_reg (struct frame_info *this_frame,
> > +				       int regnum)
> > +{
> > +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> > +  arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch);
> > +  bool override_with_sp_value = false;
> > +
> > +  if (tdep->have_sec_ext)
> > +    {
> > +      CORE_ADDR sp, msp_s, msp_ns, psp_s, psp_ns;
> > +      sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
> > +      msp_s = get_frame_register_unsigned (this_frame,
> > +					   tdep->m_profile_msp_s_regnum);
> > +      msp_ns = get_frame_register_unsigned (this_frame,
> > +					    tdep->m_profile_msp_ns_regnum);
> > +      psp_s = get_frame_register_unsigned (this_frame,
> > +					   tdep->m_profile_psp_s_regnum);
> > +      psp_ns = get_frame_register_unsigned (this_frame,
> > +					    tdep->m_profile_psp_ns_regnum);
> > +
> > +      if (regnum == tdep->m_profile_msp_regnum && (msp_s == sp || msp_ns == sp))
> > +	{
> > +	  /* Use value of SP for MSP register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +      else if (regnum == tdep->m_profile_msp_s_regnum && msp_s == sp)
> > +	{
> > +	  /* Use value of SP for MSP_S register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +      else if (regnum == tdep->m_profile_msp_ns_regnum && msp_ns == sp)
> > +	{
> > +	  /* Use value of SP for MSP_NS register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +      else if (regnum == tdep->m_profile_psp_regnum && (psp_s == sp
> > +							|| psp_ns == sp))
> > +	{
> > +	  /* Use value of SP for PSP register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +      else if (regnum == tdep->m_profile_psp_s_regnum && psp_s == sp)
> > +	{
> > +	  /* Use value of SP for PSP_S register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +      else if (regnum == tdep->m_profile_psp_ns_regnum && psp_ns == sp)
> > +	{
> > +	  /* Use value of SP for PSP_NS register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +    }
> > +  else if (tdep->is_m)
> > +    {
> > +      CORE_ADDR sp, msp, psp;
> > +      sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
> > +      msp = get_frame_register_unsigned (this_frame,
> > +					 tdep->m_profile_msp_regnum);
> > +      psp = get_frame_register_unsigned (this_frame,
> > +					 tdep->m_profile_psp_regnum);
> > +
> > +      if (regnum == tdep->m_profile_msp_regnum && sp == msp)
> > +	{
> > +	  /* Use value of SP for MSP register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +      else if (regnum == tdep->m_profile_psp_regnum && sp == psp)
> > +	{
> > +	  /* Use value of SP for PSP register.  */
> > +	  override_with_sp_value = true;
> > +	}
> > +    }
> > +
> > +  if (override_with_sp_value)
> > +    {
> > +      CORE_ADDR prev_sp;
> > +      struct frame_info *prev_frame = get_prev_frame (this_frame);
> > +      if (prev_frame)
> > +	prev_sp = get_frame_register_unsigned (prev_frame, ARM_SP_REGNUM);
> > +      else
> > +	prev_sp = get_frame_base (this_frame);
> > +
> > +      return frame_unwind_got_constant (this_frame, regnum, prev_sp);
> > +    }
> > +
> > +  /* Don't know how to fetch this value, use generic method.  */
> > +  return NULL;
> > +}
> > +
> >   /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
> >      the buffer to be NEW_LEN bytes ending at ENDADDR.  Return
> >      NULL if an error occurs.  BUF is freed.  */
> > @@ -10438,6 +10530,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> >     gdbarch_init_osabi (info, gdbarch);
> >     dwarf2_frame_set_init_reg (gdbarch, arm_dwarf2_frame_init_reg);
> > +  dwarf2_frame_set_prev_unspecified_reg (gdbarch,
> > +					 arm_dwarf2_frame_prev_unspecified_reg);
> >     /* Add some default predicates.  */
> >     if (is_m)
> > diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> > index 5878d72f922..c4b15b2d240 100644
> > --- a/gdb/dwarf2/frame.c
> > +++ b/gdb/dwarf2/frame.c
> > @@ -595,6 +595,10 @@ struct dwarf2_frame_ops
> >     /* Convert .eh_frame register number to DWARF register number, or
> >        adjust .debug_frame register number.  */
> >     int (*adjust_regnum) (struct gdbarch *, int, int);
> > +
> > +  /* Return the value of the unspecified register REGNUM for THIS_FRAME. */
> > +  struct value * (*prev_unspecified_reg) (struct frame_info *this_frame,
> > +					  int regnum);
> >   };
> >   /* Default architecture-specific register state initialization
> > @@ -677,6 +681,50 @@ dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> >     ops->init_reg (gdbarch, regnum, reg, this_frame);
> >   }
> > +/* Set the architecture-specific unspecified register unwinding
> > +   function for GDBARCH to PREV_UNSPECIFIED_REG.  */
> > +
> > +void
> > +dwarf2_frame_set_prev_unspecified_reg (struct gdbarch *gdbarch,
> > +				       struct value * (*prev_unspecified_reg)
> > +					      (struct frame_info *this_frame,
> > +					       int regnum))
> > +{
> > +  struct dwarf2_frame_ops *ops
> > +    = (struct dwarf2_frame_ops *) gdbarch_data (gdbarch, dwarf2_frame_data);
> > +
> > +  ops->prev_unspecified_reg = prev_unspecified_reg;
> > +}
> 
> I'm wondering if we really need to introduce a hook here. I understand we may need to sidestep the dwarf unwinder
> to do custom handling of the msp*/psp* registers, but wouldn't we be able to achieve the same with the existing
> dwarf hooks?
> 
> For example, we could register a hook with dwarf2_frame_set_init_reg (), like aarch64 (see gdb/aarch64-tdep.c:aarch64_dwarf2_frame_init_reg).
> 
> With such a hook, we could define our own rules for individual registers through DWARF2_FRAME_REG_FN.
> 
> Would that work?

Yes, that's indeed a better and cleaner solution, we've tested it and it works
fine.  I'll submit a new patchset.

-- 
Y.

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

end of thread, other threads:[~2022-07-07  9:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-30  7:13 [PATCH] gdb/arm: Sync sp with other *sp registers Yvan Roux
2022-07-06 12:34 ` Luis Machado
2022-07-07  9:11   ` Yvan Roux

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).