[PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Tomas Vanek]

In the lockup state the PC value of the the outer frame is irreversibly
lost. The other registers are intact so LR likely contains
PC of some frame next to the outer one, but we cannot analyze
the nearest outer frame without knowing its PC
therefore we do not know SP fixup for this frame.

The frame unwinder possibly gets mad due to the wrong SP value.
To prevent problems terminate unwinding if PC contains the magic
value of the lockup state.

Example session wihtout this change,
Cortex-M33 CPU in lockup, gdb 13.0.50.20221016-git:
----------------
  (gdb) c
  Continuing.

  Program received signal SIGINT, Interrupt.
  0xeffffffe in ?? ()
  (gdb) bt
  #0  0xeffffffe in ?? ()
  #1  0x0c000a9c in HardFault_Handler ()
      at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
  #2  0x2002ffd8 in ?? ()
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb)
----------------
The frame #1 is at correct PC taken from LR, #2 is a total nonsense.

With the change:
----------------
  (gdb) c
  Continuing.

  Program received signal SIGINT, Interrupt.
  warning: ARM M in lockup state, stack unwinding terminated.
  <signal handler called>
  (gdb) bt
  #0  <signal handler called>
  (gdb)
----------------

There is a visible drawback of emitting a warning in a cache buildnig routine
as introduced in Torbjörn SVENSSON's
[PATCH v4] gdb/arm: Stop unwinding on error, but do not assert
The warning is printed just once and not repeated on each backtrace command.

v2 update: warning supressed for other frames than the innermost one.
v3 update: boolean values and comment fixes
v4 update: comment fixes

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
---
 gdb/arm-tdep.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index b5facae..b397ca3 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -724,9 +724,30 @@ class target_arm_instruction_reader : public arm_instruction_reader
   return 0;
 }
 
+static inline bool
+arm_m_addr_is_lockup (CORE_ADDR addr)
+{
+  switch (addr)
+    {
+      /* Values for lockup state.
+	 For more details see "B1.5.15 Unrecoverable exception cases" in
+	 both ARMv6-M and ARMv7-M Architecture Reference Manuals, or
+	 see "B4.32 Lockup" in ARMv8-M Architecture Reference Manual.  */
+      case 0xeffffffe:
+      case 0xfffffffe:
+      case 0xffffffff:
+	return true;
+
+      default:
+	/* Address is not lockup.  */
+	return false;
+    }
+}
+
 /* Determine if the address specified equals any of these magic return
    values, called EXC_RETURN, defined by the ARM v6-M, v7-M and v8-M
-   architectures.
+   architectures.  Also include lockup magic PC value.
+   Check also for FNC_RETURN if we have the v8-M security extension.
 
    From ARMv6-M Reference Manual B1.5.8
    Table B1-5 Exception return behavior
@@ -769,6 +790,9 @@ class target_arm_instruction_reader : public arm_instruction_reader
 static int
 arm_m_addr_is_magic (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
+  if (arm_m_addr_is_lockup (addr))
+    return 1;
+
   arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
   if (tdep->have_sec_ext)
     {
@@ -3355,6 +3379,31 @@ struct frame_unwind arm_stub_unwind = {
      describes which bits in LR that define which stack was used prior
      to the exception and if FPU is used (causing extended stack frame).  */
 
+  /* In the lockup state PC contains a lockup magic value.
+     The PC value of the the next outer frame is irreversibly
+     lost.  The other registers are intact so LR likely contains
+     PC of some frame next to the outer one, but we cannot analyze
+     the next outer frame without knowing its PC
+     therefore we do not know SP fixup for this frame.
+     Some heuristics to resynchronize SP might be possible.
+     For simplicity, just terminate the unwinding to prevent it going
+     astray and attempting to read data/addresses it shouldn't,
+     which may cause further issues due to side-effects.  */
+  CORE_ADDR pc = get_frame_pc (this_frame);
+  if (arm_m_addr_is_lockup (pc))
+    {
+      /* The lockup can be real just in the innermost frame
+	 as the CPU is stopped and cannot create more frames.
+	 If we hit lockup magic PC in the other frame, it is
+	 just a sentinel at the top of stack: do not warn then.  */
+      if (frame_relative_level (this_frame) == 0)
+	warning (_("ARM M in lockup state, stack unwinding terminated."));
+
+      /* Terminate any further stack unwinding.  */
+      arm_cache_set_active_sp_value (cache, tdep, 0);
+      return cache;
+    }
+
   CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
 
   /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers"
@@ -3824,11 +3873,12 @@ struct frame_unwind arm_stub_unwind = {
   return arm_m_addr_is_magic (gdbarch, this_pc);
 }
 
-/* Frame unwinder for M-profile exceptions.  */
+/* Frame unwinder for M-profile exceptions (EXC_RETURN on stack),
+   lockup and secure/nonsecure interstate function calls (FNC_RETURN).  */
 
 struct frame_unwind arm_m_exception_unwind =
 {
-  "arm m exception",
+  "arm m exception lockup sec_fnc",
   SIGTRAMP_FRAME,
   arm_m_exception_frame_unwind_stop_reason,
   arm_m_exception_this_id,
-- 
1.9.1

[PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Tomas Vanek]

In the lockup state the PC value of the the outer frame is irreversibly
lost. The other registers are intact so LR likely contains
PC of some frame next to the outer one, but we cannot analyze
the nearest outer frame without knowing its PC
therefore we do not know SP fixup for this frame.

The frame unwinder possibly gets mad due to the wrong SP value.
To prevent problems terminate unwinding if PC contains the magic
value of the lockup state.

Example session wihtout this change,
Cortex-M33 CPU in lockup, gdb 13.0.50.20221016-git:
----------------
  (gdb) c
  Continuing.

  Program received signal SIGINT, Interrupt.
  0xeffffffe in ?? ()
  (gdb) bt
  #0  0xeffffffe in ?? ()
  #1  0x0c000a9c in HardFault_Handler ()
      at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
  #2  0x2002ffd8 in ?? ()
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb)
----------------
The frame #1 is at correct PC taken from LR, #2 is a total nonsense.

With the change:
----------------
  (gdb) c
  Continuing.

  Program received signal SIGINT, Interrupt.
  warning: ARM M in lockup state, stack unwinding terminated.
  <signal handler called>
  (gdb) bt
  #0  <signal handler called>
  (gdb)
----------------

There is a visible drawback of emitting a warning in a cache buildnig routine
as introduced in Torbjörn SVENSSON's
[PATCH v4] gdb/arm: Stop unwinding on error, but do not assert
The warning is printed just once and not repeated on each backtrace command.

v2 update: warning supressed for other frames than the innermost one.
v3 update: boolean values and comment fixes
v4 update: comment fixes

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
---
 gdb/arm-tdep.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index b5facae..b397ca3 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -724,9 +724,30 @@ class target_arm_instruction_reader : public arm_instruction_reader
   return 0;
 }
 
+static inline bool
+arm_m_addr_is_lockup (CORE_ADDR addr)
+{
+  switch (addr)
+    {
+      /* Values for lockup state.
+	 For more details see "B1.5.15 Unrecoverable exception cases" in
+	 both ARMv6-M and ARMv7-M Architecture Reference Manuals, or
+	 see "B4.32 Lockup" in ARMv8-M Architecture Reference Manual.  */
+      case 0xeffffffe:
+      case 0xfffffffe:
+      case 0xffffffff:
+	return true;
+
+      default:
+	/* Address is not lockup.  */
+	return false;
+    }
+}
+
 /* Determine if the address specified equals any of these magic return
    values, called EXC_RETURN, defined by the ARM v6-M, v7-M and v8-M
-   architectures.
+   architectures.  Also include lockup magic PC value.
+   Check also for FNC_RETURN if we have the v8-M security extension.
 
    From ARMv6-M Reference Manual B1.5.8
    Table B1-5 Exception return behavior
@@ -769,6 +790,9 @@ class target_arm_instruction_reader : public arm_instruction_reader
 static int
 arm_m_addr_is_magic (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
+  if (arm_m_addr_is_lockup (addr))
+    return 1;
+
   arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
   if (tdep->have_sec_ext)
     {
@@ -3355,6 +3379,31 @@ struct frame_unwind arm_stub_unwind = {
      describes which bits in LR that define which stack was used prior
      to the exception and if FPU is used (causing extended stack frame).  */
 
+  /* In the lockup state PC contains a lockup magic value.
+     The PC value of the the next outer frame is irreversibly
+     lost.  The other registers are intact so LR likely contains
+     PC of some frame next to the outer one, but we cannot analyze
+     the next outer frame without knowing its PC
+     therefore we do not know SP fixup for this frame.
+     Some heuristics to resynchronize SP might be possible.
+     For simplicity, just terminate the unwinding to prevent it going
+     astray and attempting to read data/addresses it shouldn't,
+     which may cause further issues due to side-effects.  */
+  CORE_ADDR pc = get_frame_pc (this_frame);
+  if (arm_m_addr_is_lockup (pc))
+    {
+      /* The lockup can be real just in the innermost frame
+	 as the CPU is stopped and cannot create more frames.
+	 If we hit lockup magic PC in the other frame, it is
+	 just a sentinel at the top of stack: do not warn then.  */
+      if (frame_relative_level (this_frame) == 0)
+	warning (_("ARM M in lockup state, stack unwinding terminated."));
+
+      /* Terminate any further stack unwinding.  */
+      arm_cache_set_active_sp_value (cache, tdep, 0);
+      return cache;
+    }
+
   CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
 
   /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers"
@@ -3824,11 +3873,12 @@ struct frame_unwind arm_stub_unwind = {
   return arm_m_addr_is_magic (gdbarch, this_pc);
 }
 
-/* Frame unwinder for M-profile exceptions.  */
+/* Frame unwinder for M-profile exceptions (EXC_RETURN on stack),
+   lockup and secure/nonsecure interstate function calls (FNC_RETURN).  */
 
 struct frame_unwind arm_m_exception_unwind =
 {
-  "arm m exception",
+  "arm m exception lockup sec_fnc",
   SIGTRAMP_FRAME,
   arm_m_exception_frame_unwind_stop_reason,
   arm_m_exception_this_id,
-- 
1.9.1

Re: [PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Luis Machado]

Hi Tomas,

No need to send a v5 for this, but do reference the bugzilla entry: https://sourceware.org/bugzilla/show_bug.cgi?id=28549
in the commit message.

Let's wait for Torbjörn's feedback.

On 10/21/22 10:39, Tomas Vanek wrote:
> In the lockup state the PC value of the the outer frame is irreversibly
> lost. The other registers are intact so LR likely contains
> PC of some frame next to the outer one, but we cannot analyze
> the nearest outer frame without knowing its PC
> therefore we do not know SP fixup for this frame.
> 
> The frame unwinder possibly gets mad due to the wrong SP value.
> To prevent problems terminate unwinding if PC contains the magic
> value of the lockup state.
> 
> Example session wihtout this change,
> Cortex-M33 CPU in lockup, gdb 13.0.50.20221016-git:
> ----------------
>    (gdb) c
>    Continuing.
> 
>    Program received signal SIGINT, Interrupt.
>    0xeffffffe in ?? ()
>    (gdb) bt
>    #0  0xeffffffe in ?? ()
>    #1  0x0c000a9c in HardFault_Handler ()
>        at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
>    #2  0x2002ffd8 in ?? ()
>    Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>    (gdb)
> ----------------
> The frame #1 is at correct PC taken from LR, #2 is a total nonsense.
> 
> With the change:
> ----------------
>    (gdb) c
>    Continuing.
> 
>    Program received signal SIGINT, Interrupt.
>    warning: ARM M in lockup state, stack unwinding terminated.
>    <signal handler called>
>    (gdb) bt
>    #0  <signal handler called>
>    (gdb)
> ----------------
> 
> There is a visible drawback of emitting a warning in a cache buildnig routine
> as introduced in Torbjörn SVENSSON's
> [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert
> The warning is printed just once and not repeated on each backtrace command.
> 
> v2 update: warning supressed for other frames than the innermost one.
> v3 update: boolean values and comment fixes
> v4 update: comment fixes
> 
> Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
> ---
>   gdb/arm-tdep.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index b5facae..b397ca3 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -724,9 +724,30 @@ class target_arm_instruction_reader : public arm_instruction_reader
>     return 0;
>   }
>   
> +static inline bool
> +arm_m_addr_is_lockup (CORE_ADDR addr)
> +{
> +  switch (addr)
> +    {
> +      /* Values for lockup state.
> +	 For more details see "B1.5.15 Unrecoverable exception cases" in
> +	 both ARMv6-M and ARMv7-M Architecture Reference Manuals, or
> +	 see "B4.32 Lockup" in ARMv8-M Architecture Reference Manual.  */
> +      case 0xeffffffe:
> +      case 0xfffffffe:
> +      case 0xffffffff:
> +	return true;
> +
> +      default:
> +	/* Address is not lockup.  */
> +	return false;
> +    }
> +}
> +
>   /* Determine if the address specified equals any of these magic return
>      values, called EXC_RETURN, defined by the ARM v6-M, v7-M and v8-M
> -   architectures.
> +   architectures.  Also include lockup magic PC value.
> +   Check also for FNC_RETURN if we have the v8-M security extension.
>   
>      From ARMv6-M Reference Manual B1.5.8
>      Table B1-5 Exception return behavior
> @@ -769,6 +790,9 @@ class target_arm_instruction_reader : public arm_instruction_reader
>   static int
>   arm_m_addr_is_magic (struct gdbarch *gdbarch, CORE_ADDR addr)
>   {
> +  if (arm_m_addr_is_lockup (addr))
> +    return 1;
> +
>     arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
>     if (tdep->have_sec_ext)
>       {
> @@ -3355,6 +3379,31 @@ struct frame_unwind arm_stub_unwind = {
>        describes which bits in LR that define which stack was used prior
>        to the exception and if FPU is used (causing extended stack frame).  */
>   
> +  /* In the lockup state PC contains a lockup magic value.
> +     The PC value of the the next outer frame is irreversibly
> +     lost.  The other registers are intact so LR likely contains
> +     PC of some frame next to the outer one, but we cannot analyze
> +     the next outer frame without knowing its PC
> +     therefore we do not know SP fixup for this frame.
> +     Some heuristics to resynchronize SP might be possible.
> +     For simplicity, just terminate the unwinding to prevent it going
> +     astray and attempting to read data/addresses it shouldn't,
> +     which may cause further issues due to side-effects.  */
> +  CORE_ADDR pc = get_frame_pc (this_frame);
> +  if (arm_m_addr_is_lockup (pc))
> +    {
> +      /* The lockup can be real just in the innermost frame
> +	 as the CPU is stopped and cannot create more frames.
> +	 If we hit lockup magic PC in the other frame, it is
> +	 just a sentinel at the top of stack: do not warn then.  */
> +      if (frame_relative_level (this_frame) == 0)
> +	warning (_("ARM M in lockup state, stack unwinding terminated."));
> +
> +      /* Terminate any further stack unwinding.  */
> +      arm_cache_set_active_sp_value (cache, tdep, 0);
> +      return cache;
> +    }
> +
>     CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
>   
>     /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers"
> @@ -3824,11 +3873,12 @@ struct frame_unwind arm_stub_unwind = {
>     return arm_m_addr_is_magic (gdbarch, this_pc);
>   }
>   
> -/* Frame unwinder for M-profile exceptions.  */
> +/* Frame unwinder for M-profile exceptions (EXC_RETURN on stack),
> +   lockup and secure/nonsecure interstate function calls (FNC_RETURN).  */
>   
>   struct frame_unwind arm_m_exception_unwind =
>   {
> -  "arm m exception",
> +  "arm m exception lockup sec_fnc",
>     SIGTRAMP_FRAME,
>     arm_m_exception_frame_unwind_stop_reason,
>     arm_m_exception_this_id,

Re: [PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Tom Tromey]

>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

Luis> No need to send a v5 for this, but do reference the bugzilla
Luis> entry: https://sourceware.org/bugzilla/show_bug.cgi?id=28549 in
Luis> the commit message.

If you add text of the form "PR category/28549" to the commit message,
it will also be logged in bugzilla.  (Sadly the script still doesn't
seem to understand the Bug: convention.)

Tom

Re: [PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Tom Tromey]

>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

Luis> No need to send a v5 for this, but do reference the bugzilla
Luis> entry: https://sourceware.org/bugzilla/show_bug.cgi?id=28549 in
Luis> the commit message.

If you add text of the form "PR category/28549" to the commit message,
it will also be logged in bugzilla.  (Sadly the script still doesn't
seem to understand the Bug: convention.)

Tom

Re: [PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Luis Machado]

Tomas,

On 10/21/22 10:53, Luis Machado via Gdb-patches wrote:
> Hi Tomas,
> 
> No need to send a v5 for this, but do reference the bugzilla entry: https://sourceware.org/bugzilla/show_bug.cgi?id=28549
> in the commit message.
> 
> Let's wait for Torbjörn's feedback.

Torbjörn confirmed this works on a STM32F4-Discovery board.

The patch LGTM.

Thanks!

> 
> On 10/21/22 10:39, Tomas Vanek wrote:
>> In the lockup state the PC value of the the outer frame is irreversibly
>> lost. The other registers are intact so LR likely contains
>> PC of some frame next to the outer one, but we cannot analyze
>> the nearest outer frame without knowing its PC
>> therefore we do not know SP fixup for this frame.
>>
>> The frame unwinder possibly gets mad due to the wrong SP value.
>> To prevent problems terminate unwinding if PC contains the magic
>> value of the lockup state.
>>
>> Example session wihtout this change,
>> Cortex-M33 CPU in lockup, gdb 13.0.50.20221016-git:
>> ----------------
>>    (gdb) c
>>    Continuing.
>>
>>    Program received signal SIGINT, Interrupt.
>>    0xeffffffe in ?? ()
>>    (gdb) bt
>>    #0  0xeffffffe in ?? ()
>>    #1  0x0c000a9c in HardFault_Handler ()
>>        at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
>>    #2  0x2002ffd8 in ?? ()
>>    Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>>    (gdb)
>> ----------------
>> The frame #1 is at correct PC taken from LR, #2 is a total nonsense.
>>
>> With the change:
>> ----------------
>>    (gdb) c
>>    Continuing.
>>
>>    Program received signal SIGINT, Interrupt.
>>    warning: ARM M in lockup state, stack unwinding terminated.
>>    <signal handler called>
>>    (gdb) bt
>>    #0  <signal handler called>
>>    (gdb)
>> ----------------
>>
>> There is a visible drawback of emitting a warning in a cache buildnig routine
>> as introduced in Torbjörn SVENSSON's
>> [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert
>> The warning is printed just once and not repeated on each backtrace command.
>>
>> v2 update: warning supressed for other frames than the innermost one.
>> v3 update: boolean values and comment fixes
>> v4 update: comment fixes
>>
>> Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
>> ---
>>   gdb/arm-tdep.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> index b5facae..b397ca3 100644
>> --- a/gdb/arm-tdep.c
>> +++ b/gdb/arm-tdep.c
>> @@ -724,9 +724,30 @@ class target_arm_instruction_reader : public arm_instruction_reader
>>     return 0;
>>   }
>> +static inline bool
>> +arm_m_addr_is_lockup (CORE_ADDR addr)
>> +{
>> +  switch (addr)
>> +    {
>> +      /* Values for lockup state.
>> +     For more details see "B1.5.15 Unrecoverable exception cases" in
>> +     both ARMv6-M and ARMv7-M Architecture Reference Manuals, or
>> +     see "B4.32 Lockup" in ARMv8-M Architecture Reference Manual.  */
>> +      case 0xeffffffe:
>> +      case 0xfffffffe:
>> +      case 0xffffffff:
>> +    return true;
>> +
>> +      default:
>> +    /* Address is not lockup.  */
>> +    return false;
>> +    }
>> +}
>> +
>>   /* Determine if the address specified equals any of these magic return
>>      values, called EXC_RETURN, defined by the ARM v6-M, v7-M and v8-M
>> -   architectures.
>> +   architectures.  Also include lockup magic PC value.
>> +   Check also for FNC_RETURN if we have the v8-M security extension.
>>      From ARMv6-M Reference Manual B1.5.8
>>      Table B1-5 Exception return behavior
>> @@ -769,6 +790,9 @@ class target_arm_instruction_reader : public arm_instruction_reader
>>   static int
>>   arm_m_addr_is_magic (struct gdbarch *gdbarch, CORE_ADDR addr)
>>   {
>> +  if (arm_m_addr_is_lockup (addr))
>> +    return 1;
>> +
>>     arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
>>     if (tdep->have_sec_ext)
>>       {
>> @@ -3355,6 +3379,31 @@ struct frame_unwind arm_stub_unwind = {
>>        describes which bits in LR that define which stack was used prior
>>        to the exception and if FPU is used (causing extended stack frame).  */
>> +  /* In the lockup state PC contains a lockup magic value.
>> +     The PC value of the the next outer frame is irreversibly
>> +     lost.  The other registers are intact so LR likely contains
>> +     PC of some frame next to the outer one, but we cannot analyze
>> +     the next outer frame without knowing its PC
>> +     therefore we do not know SP fixup for this frame.
>> +     Some heuristics to resynchronize SP might be possible.
>> +     For simplicity, just terminate the unwinding to prevent it going
>> +     astray and attempting to read data/addresses it shouldn't,
>> +     which may cause further issues due to side-effects.  */
>> +  CORE_ADDR pc = get_frame_pc (this_frame);
>> +  if (arm_m_addr_is_lockup (pc))
>> +    {
>> +      /* The lockup can be real just in the innermost frame
>> +     as the CPU is stopped and cannot create more frames.
>> +     If we hit lockup magic PC in the other frame, it is
>> +     just a sentinel at the top of stack: do not warn then.  */
>> +      if (frame_relative_level (this_frame) == 0)
>> +    warning (_("ARM M in lockup state, stack unwinding terminated."));
>> +
>> +      /* Terminate any further stack unwinding.  */
>> +      arm_cache_set_active_sp_value (cache, tdep, 0);
>> +      return cache;
>> +    }
>> +
>>     CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
>>     /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers"
>> @@ -3824,11 +3873,12 @@ struct frame_unwind arm_stub_unwind = {
>>     return arm_m_addr_is_magic (gdbarch, this_pc);
>>   }
>> -/* Frame unwinder for M-profile exceptions.  */
>> +/* Frame unwinder for M-profile exceptions (EXC_RETURN on stack),
>> +   lockup and secure/nonsecure interstate function calls (FNC_RETURN).  */
>>   struct frame_unwind arm_m_exception_unwind =
>>   {
>> -  "arm m exception",
>> +  "arm m exception lockup sec_fnc",
>>     SIGTRAMP_FRAME,
>>     arm_m_exception_frame_unwind_stop_reason,
>>     arm_m_exception_this_id,
> 

Re: [PATCH v4] gdb/arm: Terminate frame unwinding in M-profile lockup state

[Luis Machado]

On 10/25/22 14:44, Luis Machado via Gdb-patches wrote:
> Tomas,
> 
> On 10/21/22 10:53, Luis Machado via Gdb-patches wrote:
>> Hi Tomas,
>>
>> No need to send a v5 for this, but do reference the bugzilla entry: https://sourceware.org/bugzilla/show_bug.cgi?id=28549
>> in the commit message.
>>
>> Let's wait for Torbjörn's feedback.
> 
> Torbjörn confirmed this works on a STM32F4-Discovery board.
> 
> The patch LGTM.
> 
> Thanks!
> 
>>
>> On 10/21/22 10:39, Tomas Vanek wrote:
>>> In the lockup state the PC value of the the outer frame is irreversibly
>>> lost. The other registers are intact so LR likely contains
>>> PC of some frame next to the outer one, but we cannot analyze
>>> the nearest outer frame without knowing its PC
>>> therefore we do not know SP fixup for this frame.
>>>
>>> The frame unwinder possibly gets mad due to the wrong SP value.
>>> To prevent problems terminate unwinding if PC contains the magic
>>> value of the lockup state.
>>>
>>> Example session wihtout this change,
>>> Cortex-M33 CPU in lockup, gdb 13.0.50.20221016-git:
>>> ----------------
>>>    (gdb) c
>>>    Continuing.
>>>
>>>    Program received signal SIGINT, Interrupt.
>>>    0xeffffffe in ?? ()
>>>    (gdb) bt
>>>    #0  0xeffffffe in ?? ()
>>>    #1  0x0c000a9c in HardFault_Handler ()
>>>        at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
>>>    #2  0x2002ffd8 in ?? ()
>>>    Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>>>    (gdb)
>>> ----------------
>>> The frame #1 is at correct PC taken from LR, #2 is a total nonsense.
>>>
>>> With the change:
>>> ----------------
>>>    (gdb) c
>>>    Continuing.
>>>
>>>    Program received signal SIGINT, Interrupt.
>>>    warning: ARM M in lockup state, stack unwinding terminated.
>>>    <signal handler called>
>>>    (gdb) bt
>>>    #0  <signal handler called>
>>>    (gdb)
>>> ----------------
>>>
>>> There is a visible drawback of emitting a warning in a cache buildnig routine
>>> as introduced in Torbjörn SVENSSON's
>>> [PATCH v4] gdb/arm: Stop unwinding on error, but do not assert
>>> The warning is printed just once and not repeated on each backtrace command.
>>>
>>> v2 update: warning supressed for other frames than the innermost one.
>>> v3 update: boolean values and comment fixes
>>> v4 update: comment fixes
>>>
>>> Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
>>> ---
>>>   gdb/arm-tdep.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>>   1 file changed, 53 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>>> index b5facae..b397ca3 100644
>>> --- a/gdb/arm-tdep.c
>>> +++ b/gdb/arm-tdep.c
>>> @@ -724,9 +724,30 @@ class target_arm_instruction_reader : public arm_instruction_reader
>>>     return 0;
>>>   }
>>> +static inline bool
>>> +arm_m_addr_is_lockup (CORE_ADDR addr)
>>> +{
>>> +  switch (addr)
>>> +    {
>>> +      /* Values for lockup state.
>>> +     For more details see "B1.5.15 Unrecoverable exception cases" in
>>> +     both ARMv6-M and ARMv7-M Architecture Reference Manuals, or
>>> +     see "B4.32 Lockup" in ARMv8-M Architecture Reference Manual.  */
>>> +      case 0xeffffffe:
>>> +      case 0xfffffffe:
>>> +      case 0xffffffff:
>>> +    return true;
>>> +
>>> +      default:
>>> +    /* Address is not lockup.  */
>>> +    return false;
>>> +    }
>>> +}
>>> +
>>>   /* Determine if the address specified equals any of these magic return
>>>      values, called EXC_RETURN, defined by the ARM v6-M, v7-M and v8-M
>>> -   architectures.
>>> +   architectures.  Also include lockup magic PC value.
>>> +   Check also for FNC_RETURN if we have the v8-M security extension.
>>>      From ARMv6-M Reference Manual B1.5.8
>>>      Table B1-5 Exception return behavior
>>> @@ -769,6 +790,9 @@ class target_arm_instruction_reader : public arm_instruction_reader
>>>   static int
>>>   arm_m_addr_is_magic (struct gdbarch *gdbarch, CORE_ADDR addr)
>>>   {
>>> +  if (arm_m_addr_is_lockup (addr))
>>> +    return 1;
>>> +
>>>     arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
>>>     if (tdep->have_sec_ext)
>>>       {
>>> @@ -3355,6 +3379,31 @@ struct frame_unwind arm_stub_unwind = {
>>>        describes which bits in LR that define which stack was used prior
>>>        to the exception and if FPU is used (causing extended stack frame).  */
>>> +  /* In the lockup state PC contains a lockup magic value.
>>> +     The PC value of the the next outer frame is irreversibly
>>> +     lost.  The other registers are intact so LR likely contains
>>> +     PC of some frame next to the outer one, but we cannot analyze
>>> +     the next outer frame without knowing its PC
>>> +     therefore we do not know SP fixup for this frame.
>>> +     Some heuristics to resynchronize SP might be possible.
>>> +     For simplicity, just terminate the unwinding to prevent it going
>>> +     astray and attempting to read data/addresses it shouldn't,
>>> +     which may cause further issues due to side-effects.  */
>>> +  CORE_ADDR pc = get_frame_pc (this_frame);
>>> +  if (arm_m_addr_is_lockup (pc))
>>> +    {
>>> +      /* The lockup can be real just in the innermost frame
>>> +     as the CPU is stopped and cannot create more frames.
>>> +     If we hit lockup magic PC in the other frame, it is
>>> +     just a sentinel at the top of stack: do not warn then.  */
>>> +      if (frame_relative_level (this_frame) == 0)
>>> +    warning (_("ARM M in lockup state, stack unwinding terminated."));
>>> +
>>> +      /* Terminate any further stack unwinding.  */
>>> +      arm_cache_set_active_sp_value (cache, tdep, 0);
>>> +      return cache;
>>> +    }
>>> +
>>>     CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
>>>     /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers"
>>> @@ -3824,11 +3873,12 @@ struct frame_unwind arm_stub_unwind = {
>>>     return arm_m_addr_is_magic (gdbarch, this_pc);
>>>   }
>>> -/* Frame unwinder for M-profile exceptions.  */
>>> +/* Frame unwinder for M-profile exceptions (EXC_RETURN on stack),
>>> +   lockup and secure/nonsecure interstate function calls (FNC_RETURN).  */
>>>   struct frame_unwind arm_m_exception_unwind =
>>>   {
>>> -  "arm m exception",
>>> +  "arm m exception lockup sec_fnc",
>>>     SIGTRAMP_FRAME,
>>>     arm_m_exception_frame_unwind_stop_reason,
>>>     arm_m_exception_this_id,
>>
> 

Pushed now.