[PATCH v4] gdb/arm: Stop unwinding on error, but do not assert

[PATCH v4] gdb/arm: Stop unwinding on error, but do not assert

[Torbjörn SVENSSON]

When it's impossible to read the FPCCR and XPSR, the unwinding is
unpredictable as the it's not possible to determine the correct
frame size or padding.
The only sane thing to do in this condition is to stop the unwinding.

Example session wihtout this patch:

  (gdb) bt
  #0  SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
  .../gdb/arm-tdep.c:3594: internal-error: arm_m_exception_cache: Assertion `safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE, byte_order, &fpccr)' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  0x5583bfb2a157 gdb_internal_backtrace_1
  ...
  ---------------------

  This is a bug, please report it.  For instructions, see:
  <https://www.gnu.org/software/gdb/bugs/>.

  Aborted (core dumped)

Example session with this patch:

  (gdb) bt
  #0  SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
  warning: Could not fetch required FPCCR content.  Further unwind is impossible.
  #1  <signal handler called>
  (gdb)

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

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 041e6afefed..fa6b08e4a54 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3591,9 +3591,14 @@ arm_m_exception_cache (frame_info_ptr this_frame)
 	  ULONGEST fpcar;
 
 	  /* Read FPCCR register.  */
-	  gdb_assert (safe_read_memory_unsigned_integer (FPCCR,
-							 ARM_INT_REGISTER_SIZE,
-							 byte_order, &fpccr));
+	  if (!safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE,
+						 byte_order, &fpccr))
+	    {
+	      warning (_("Could not fetch required FPCCR content.  Further "
+			 "unwinding is impossible."));
+	      arm_cache_set_active_sp_value (cache, tdep, 0);
+	      return cache;
+	    }
 
 	  /* Read FPCAR register.  */
 	  if (!safe_read_memory_unsigned_integer (FPCAR, ARM_INT_REGISTER_SIZE,
@@ -3669,9 +3674,16 @@ arm_m_exception_cache (frame_info_ptr this_frame)
 	 aligner between the top of the 32-byte stack frame and the
 	 previous context's stack pointer.  */
       ULONGEST xpsr;
-      gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[
-						     ARM_PS_REGNUM].addr (), 4,
-						     byte_order, &xpsr));
+      if (!safe_read_memory_unsigned_integer (cache->saved_regs[ARM_PS_REGNUM]
+					      .addr (), ARM_INT_REGISTER_SIZE,
+					      byte_order, &xpsr))
+	{
+	  warning (_("Could not fetch required XPSR content.  Further "
+		     "unwinding is impossible."));
+	  arm_cache_set_active_sp_value (cache, tdep, 0);
+	  return cache;
+	}
+
       if (bit (xpsr, 9) != 0)
 	{
 	  CORE_ADDR new_sp = arm_cache_get_prev_sp_value (cache, tdep) + 4;
@@ -3689,6 +3701,27 @@ arm_m_exception_cache (frame_info_ptr this_frame)
 		  phex (lr, ARM_INT_REGISTER_SIZE));
 }
 
+/* Implementation of the stop_reason hook for arm_m_exception frames.  */
+
+static enum unwind_stop_reason
+arm_m_exception_frame_unwind_stop_reason (frame_info_ptr this_frame,
+					  void **this_cache)
+{
+  struct arm_prologue_cache *cache;
+  arm_gdbarch_tdep *tdep
+    = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));
+
+  if (*this_cache == NULL)
+    *this_cache = arm_m_exception_cache (this_frame);
+  cache = (struct arm_prologue_cache *) *this_cache;
+
+  /* If we've hit a wall, stop.  */
+  if (arm_cache_get_prev_sp_value (cache, tdep) == 0)
+    return UNWIND_OUTERMOST;
+
+  return UNWIND_NO_REASON;
+}
+
 /* Implementation of function hook 'this_id' in
    'struct frame_uwnind'.  */
 
@@ -3798,7 +3831,7 @@ struct frame_unwind arm_m_exception_unwind =
 {
   "arm m exception",
   SIGTRAMP_FRAME,
-  default_frame_unwind_stop_reason,
+  arm_m_exception_frame_unwind_stop_reason,
   arm_m_exception_this_id,
   arm_m_exception_prev_register,
   NULL,
-- 
2.25.1

Re: [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert

[Pedro Alves]

On 2022-10-13 7:11 p.m., Torbjörn SVENSSON wrote:
> When it's impossible to read the FPCCR and XPSR, the unwinding is
> unpredictable as the it's not possible to determine the correct
> frame size or padding.
> The only sane thing to do in this condition is to stop the unwinding.
> 
> Example session wihtout this patch:

wihtout -> without

Thank you, this version LGTM.  This way, there's a better chance gdb can
distinguish different "outermost" frames, e.g., for watchpoints on local vars,
or for stepping from a "broken" frame, to another "broken" frame.

I'll defer final approval to Luis.

Reviewed-by: Pedro Alves <pedro@palves.net>

Re: [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert

[Torbjorn SVENSSON]

On 2022-10-14 13:45, Pedro Alves wrote:
> On 2022-10-13 7:11 p.m., Torbjörn SVENSSON wrote:
>> When it's impossible to read the FPCCR and XPSR, the unwinding is
>> unpredictable as the it's not possible to determine the correct
>> frame size or padding.
>> The only sane thing to do in this condition is to stop the unwinding.
>>
>> Example session wihtout this patch:
> 
> wihtout -> without
> 
> Thank you, this version LGTM.  This way, there's a better chance gdb can
> distinguish different "outermost" frames, e.g., for watchpoints on local vars,
> or for stepping from a "broken" frame, to another "broken" frame.
> 
> I'll defer final approval to Luis.
> 
> Reviewed-by: Pedro Alves <pedro@palves.net>

Thanks Pedro!

After this patch has landed, I will clean up the other SP hacks to use 
the same solution to make it more clear what will happen.

Re: [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert

[Luis Machado]

On 10/14/22 12:45, Pedro Alves wrote:
> On 2022-10-13 7:11 p.m., Torbjörn SVENSSON wrote:
>> When it's impossible to read the FPCCR and XPSR, the unwinding is
>> unpredictable as the it's not possible to determine the correct
>> frame size or padding.
>> The only sane thing to do in this condition is to stop the unwinding.
>>
>> Example session wihtout this patch:
> 
> wihtout -> without
> 
> Thank you, this version LGTM.  This way, there's a better chance gdb can
> distinguish different "outermost" frames, e.g., for watchpoints on local vars,
> or for stepping from a "broken" frame, to another "broken" frame.
> 
> I'll defer final approval to Luis.
> 
> Reviewed-by: Pedro Alves <pedro@palves.net>

LGTM.

Re: [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert

[Torbjorn SVENSSON]

On 2022-10-14 14:03, Luis Machado wrote:
> On 10/14/22 12:45, Pedro Alves wrote:
>> On 2022-10-13 7:11 p.m., Torbjörn SVENSSON wrote:
>>> When it's impossible to read the FPCCR and XPSR, the unwinding is
>>> unpredictable as the it's not possible to determine the correct
>>> frame size or padding.
>>> The only sane thing to do in this condition is to stop the unwinding.
>>>
>>> Example session wihtout this patch:
>>
>> wihtout -> without
>>
>> Thank you, this version LGTM.  This way, there's a better chance gdb can
>> distinguish different "outermost" frames, e.g., for watchpoints on 
>> local vars,
>> or for stepping from a "broken" frame, to another "broken" frame.
>>
>> I'll defer final approval to Luis.
>>
>> Reviewed-by: Pedro Alves <pedro@palves.net>
> 
> LGTM.


Pushed.