[Bug gdb/28549] New: ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[Bug gdb/28549] New: ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

            Bug ID: 28549
           Summary: ARM/Cortex-M: improper stack unwinding when the target
                    is in lockup state
           Product: gdb
           Version: 9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: tarek.bouchkati at gmail dot com
  Target Milestone: ---

=== environment ====================================================
$ gdb-multiarch -v
GNU gdb (Ubuntu 9.2-0ubuntu1~20.04) 9.2

using STM32H723ZG MCU, containing a Cortex-M7 MCU
using OpenOCD as gdbserver

=== how to reproduce ===============================================
start debug session with empty flash
$ openocd -f interface/stlink-dap.cfg -f target/stm32h7x.cfg

since the flash is empty, the core is in lockup state,
ie: PC = 0xfffffffe, LR = 0xffffffff

now using gdb-multiarch, do:
(gdb) tar ext :3333

we get the following error in openocd:
Error: Failed to read memory at 0xfffffffe

=== analysis =======================================================
doing the same operation with higher verbosity, here is OpenOCD log:
Debug: gdb_server.c:384 gdb_log_incoming_packet(): received packet: mfffffffe,2
Debug: gdb_server.c:1495 gdb_read_memory_packet(): addr: 0x00000000fffffffe,
len: 0x00000002
Debug: target.c:2466 target_read_buffer(): reading buffer of 2 byte at
0xfffffffe
Debug: stlink_usb.c:1112 stlink_usb_error_check(): STLINK_SWD_AP_WDATA_ERROR
Error: arm_adi_v5.c:561 mem_ap_read(): Failed to read memory at 0xfffffffe

This means that gdb is requesting to read this address, most probably to unwind
the stack

=== expected behavior ==============================================
do not unwind the stack when PC value is "magic" = exception address

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #1 from Tarek BOCHKATI <tarek.bouchkati at gmail dot com> ---
reproduced using git/master/HEAD

GNU gdb (GDB) 12.0.50.20211102-git
...
This GDB was configured as "--host=x86_64-pc-linux-gnu --target=arm-none-eabi"

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #2 from Tarek BOCHKATI <tarek.bouchkati at gmail dot com> ---
according to ARM: ref https://developer.arm.com/documentation/ka001181/latest

quote:
 > During LOCKUP, the PC will be forced to 0xFFFFFFFx and will keep fetching
from that address

applying this minor change to test the theory seem to work perfectly:

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 5b60831bbe9..930eba7155e 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -543,6 +543,7 @@ arm_m_addr_is_magic (CORE_ADDR addr)
 {
   switch (addr)
     {
+      case 0xfffffffe:
       /* Values from ARMv8-M Architecture Technical Reference.  */
       case 0xffffffb0:
       case 0xffffffb8:



now gdb reports:

(gdb) tar ext :3333
Remote debugging using :3333
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
<signal handler called>
(gdb) c
Continuing.
stm32h7x.cpu0 -- clearing lockup after double fault

Program received signal SIGINT, Interrupt.
<signal handler called>
(gdb)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Tarek BOCHKATI <tarek.bouchkati at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fredrik.hederstierna@securi
                   |                            |tas-direct.com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Tarek BOCHKATI <tarek.bouchkati at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alan.hayward at arm dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[alahay01 at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Alan Hayward <alahay01 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alahay01 at gcc dot gnu.org,
                   |                            |luis.machado at linaro dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #3 from Luis Machado <luis.machado at linaro dot org> ---
It seems fairly obvious this magic PC needs to be handled, but is it right to
display something like <signal handler called>?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Luis Machado <luis.machado at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-11-05
             Status|UNCONFIRMED                 |WAITING
     Ever confirmed|0                           |1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Luis Machado <luis.machado at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |luis.machado at linaro dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #4 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
I agree it might be some mixups between LR and PC, though I think in practice
it might not cause any problems in these cases.

To cover also Cortex-M33 ARMv8M maybe also check for LR=0xFFFFFFFE and
PC=0xEFFFFFFE. The 'easy' fix is to add these values I guess:

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 857a52a9a51..33b03ead5a4 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -556,6 +556,13 @@ arm_m_addr_is_magic (CORE_ADDR addr)
       case 0xfffffff1:
       case 0xfffffff9:
       case 0xfffffffd:
+      /* 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:
        /* Address is magic.  */
        return 1;

Though maybe we would like to add some more info that actually detects more
explicit the LOCKUP state and make some printouts about that?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #5 from Tarek BOCHKATI <tarek.bouchkati at gmail dot com> ---
0xfffffffe is just one missing magic address.
according to arm: 
> During LOCKUP, the PC will be forced to 0xFFFFFFFx

so all 0xFFFFFFFx should be considered magic as well, and this hides several
EXC_RETURN

I'm wondering why we are linking this to EXC_RETURN, since this function is
used to check PC values and EXC_RETURN values are loaded into LR.

more literature:
according to ARM v6M Arch ref man (ARM DDI 0419E page B1-198),
section B1.5.8: Exception return behavior:
> An exception return occurs when the processor is in Handler mode
> and one of the following instructions loads a value of 0xFXXXXXXX into the PC

according to this all 0xfxxxxxxx values are magic
the same text is present in ARM v7M arch ref man

in ARM v8M ref man, section B3.22:
> ... loads an EXC_RETURN value, 0xFFXXXXXX, into the PC


============================================

regarding: 
is it right to display something like <signal handler called>?
it's fine for me, unless there is a way to display something similar to
"exception" 

but I agree, if we want to have a minor fix/change:
we can limit to adding the 3 magic addresses:
+      case 0xeffffffe:
+      case 0xfffffffe:
+      case 0xffffffff:

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #6 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Created attachment 13760
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13760&action=edit
Proposed patch

Proposed patch

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #7 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
This might started with patch from 2016-09-27:

@@ -472,7 +528,7 @@ arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR
val)
   /* On M-profile devices, do not strip the low bit from EXC_RETURN
      (the magic exception return address).  */
   if (gdbarch_tdep (gdbarch)->is_m
-      && (val & 0xfffffff0) == 0xfffffff0)
+      && arm_m_addr_is_magic (val))
     return val;

   if (arm_apcs_32)

Though still PC=0xEFFFFFFE is a Lockup state for ARMv8M to my understanding, so
just revert that wont do the full trick. Also I think it is good to have the
explicit EXC_RETURN values defined to possibly make way to future special
handling of the actual meaning of these values, like using different stack
pointers etc.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #8 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Created attachment 13761
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13761&action=edit
Proposed alternative patch

Proposed alternative more aggressive patch, catches all EXC_RETURN 0xFFxxxxxx.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #9 from Tarek BOCHKATI <tarek.bouchkati at gmail dot com> ---
Hi Fredrik,

I have checked both patches they seems to work fine !

I don't have a strong opinion on which one is more suitable, since IDK what
arm_addr_bits_remove is used for.

I let you decide which one is better.

Thanks again for your reactivity !

Kind Regards,
Tarek

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #10 from Luis Machado <luis.machado at linaro dot org> ---
I think we should keep the magic check function, but update it to do checks for
more cases of special PC's.

Do any of you have a copyright assignment in place? Or should I pursue this
upstream based on the manual entries?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #11 from Tarek BOCHKATI <tarek.bouchkati at gmail dot com> ---
IMO please proceed with manual entries. unless Fredrik has a different thought.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #12 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Either is fine by me, I have copyright assignment in place, actually it was me
who originally added this function 'arm_m_addr_is_magic()' in the first place,
so I guess you can blame me for the bug ;-)
BR Fredrik

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #13 from Luis Machado <luis.machado at linaro dot org> ---
Indeed. I see it now! :-)

In my opinion, the change to arm_addr_bits_remove should really go into
arm_m_addr_is_magic. And it would be nice to update the documentation (already
quite detailed) to include these previously unhandled magic PC's.

Otherwise, it looks OK to me.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Tarek BOCHKATI <tarek.bouchkati at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clyon at gcc dot gnu.org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

tomas.vanek at fbl dot cz changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tomas.vanek at fbl dot cz

--- Comment #14 from tomas.vanek at fbl dot cz ---
Fredrik,

I'm afraid that simply adding lockup addresses to the list of magic address is
not correct. The Cortex-M lockup differs from an exception! The lockup does not
stack anything, it just sets PC (and other special regs). So there is no point
in trying to unwind lockup frame. It does not seem possible, the previous PC is
lost.

Unwinding lockup state as it was an exception may result again in peeking not
existing memory (arm_m_exception_cache() reads word @ unwound_sp + 28)

So I would recommend splitting arm_m_addr_is_magic() to
arm_m_addr_is_exc_return()
and arm_m_addr_is_lockup().
Then introduce another Cortex-M specific unwinder "arm m lockup" with a trivial
implementation:
arm_m_lockup_this_id() will always return outer_frame_id
and
arm_m_lockup_unwind_sniffer() will check PC for arm_m_addr_is_lockup()

Tom

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #15 from Luis Machado <luis.machado at linaro dot org> ---
The custom unwinder that just stops seems to be a more elegant solution,
indeed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #16 from tomas.vanek at fbl dot cz ---
Luis,
TBH I didn't care about elegance ;)
The prevention of bogus unwinding is much more important.
Look at test results.
Just before real lockup:

Breakpoint 1, DoubleFaultGenerator () at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:
72
72            double_fault_dest= *double_fault_unreadable_src;
(gdb) backtrace
#0  DoubleFaultGenerator () at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:72
#1  0x0c000a50 in HardFault_Handler ()
    at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
#2  <signal handler called>
#3  0x0c000876 in HardFaultGenerator () at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:406
#4  0x0c0008c0 in HAL_SYSTICK_Callback () at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:427
#5  0x0c000b1e in SysTick_Handler ()
    at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:234
#6  <signal handler called>
#7  HAL_GPIO_ReadPin (GPIOx=0x0, GPIO_Pin=0)
    at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_gpio.c:371
#8  0x0c0004ec in main () at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:165

Locked-up, gdb with Fredrik's alternative patch:

(gdb) c
Continuing.
stm32l5x.cpu -- clearing lockup after double fault

Program received signal SIGINT, Interrupt.
<signal handler called>
(gdb) backtrace
#0  <signal handler called>
#1  0x4fff0000 in ?? ()
#2  0x30000048 in pSecureErrorCallback ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

As expected, backtrace got completely lost.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[clyon at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #17 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I am looking at PR26613, which is probably partly related to this bug report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #18 from Luis Machado <luis.machado at linaro dot org> ---
What I meant by elegance is making GDB stop when it should and display the
right information (that we're locked up, instead of appearing like we stopped
in the middle of a signal handler).

If GDB attempts to backtrace further (as you showed in your example), that
sounds like a problem with the patch. It should've acted like the following...

--

(gdb) c
Continuing.
stm32h7x.cpu0 -- clearing lockup after double fault

Program received signal SIGINT, Interrupt.
<signal handler called>
(gdb)

--

... which, in my opinion, is not as informative as displaying a custom message
saying we're locked up.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #19 from Luis Machado <luis.machado at linaro dot org> ---
To complement, the backtrace produced should not go further than <signal
handler called>. This might be a limitation of how we currently handle magic
PC's/LR's.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #20 from tomas.vanek at fbl dot cz ---
Luis, Fredrik,

I wrote a draft of the proposed "lockup" custom unwinder. I'm not familiar with
gdb source so I'm asking for a little help.

I'm very unsure what arm_m_lockup_prev_register() should return.
I think it's not important as the lockup frame declare itself as outer.
Is it correct?

Is there an easy way to return something valid without generating a frame
cache?
Perhaps
 return frame_unwind_got_register (this_frame, prev_regnum, prev_regnum);
would be ok?

Tom

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #21 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Hi all,
would it be possible to handle Lockup inside function
  <arm_m_exception_cache (struct frame_info *this_frame)>

PSEUDO:

     if (pc == 0xFFFFFFFE)  <<== Lockup criterion checked in exception cache
       {
          warning (_("no thread stack unwinding supported in Lockup."));

          /* Terminate any further stack unwinding by refer to self.  */
          cache->prev_sp = sp;
          return cache;
       }

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #22 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Something like this (sketch not compiled):

@@ -2982,6 +2989,7 @@ arm_m_exception_cache (struct frame_info *this_frame)
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct arm_prologue_cache *cache;
+  CORE_ADDR pc;
   CORE_ADDR lr;
   CORE_ADDR sp;
   CORE_ADDR unwound_sp;
@@ -2998,12 +3006,26 @@ arm_m_exception_cache (struct frame_info *this_frame)
      describes which bits in LR that define which stack was used prior
      to the exception and if FPU is used (causing extended stack frame).  */

+  pc = get_frame_register_unsigned (this_frame, ARM_PC_REGNUM);
   lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
   sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);

   /* Check EXC_RETURN indicator bits.  */
   exc_return = (((lr >> 28) & 0xf) == 0xf);

+  /* Check if the core is in Lockup state.  */
+  if (exc_return && (pc == 0xFFFFFFFE))
+    {
+      /* Thread (process) stack could not be fetched in Lockup,
+        give warning and exit.  */
+
+      warning (_("no thread stack unwinding supported in Lockup."));
+
+      /* Terminate any further stack unwinding by refer to self.  */
+      cache->prev_sp = sp;
+      return cache;
+    }
+
   /* Check EXC_RETURN bit SPSEL if Main or Thread (process) stack used.  */
   process_stack_used = ((lr & (1 << 2)) != 0);
   if (exc_return && process_stack_used)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #23 from tomas.vanek at fbl dot cz ---
Fredrik,

it does not stop unwinding:

(gdb) bt
#0  <signal arm m exception>
#1  0x4fff0000 in ?? ()
#2  0x30000050 in pSecureErrorCallback ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

IMO returning outer_frame_id is crucial and yes, arm_m_exception_this_id()
can be modified to return it if lockup is detected. In comparison with this a
new custom unwinder sniffing for lockup seems me cleaner.

And with a small modification gdb 'backtrace' can print fancy unwinder name
instead of just <signal handler called>: 

(gdb) bt
#0  <signal arm m lockup>
(gdb)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #24 from tomas.vanek at fbl dot cz ---
Fredrik,

finally I found that exc_return is false in case of ARMv8m lockup (PC
0xeffffffe),
so your code also can stop unwinding (with twice printed warning):

(gdb) bt
warning: no thread stack unwinding supported in Lockup.
warning: no thread stack unwinding supported in Lockup.
#0  <signal handler called>
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb)

Hmmm, not neat...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #25 from tomas.vanek at fbl dot cz ---
One more test, this time Cortex-M4 just after reset (issued OpenOCD reset
halt).
Lockup didn't happen, just the CPU prepared LR=0xffffffff to catch eventual
erroneous return from the app:

(gdb) bt
warning: no thread stack unwinding supported in Lockup.
#0  0x080006b8 in Reset_Handler ()
(gdb) adv main
warning: no thread stack unwinding supported in Lockup.
Note: automatically using hardware breakpoints for read-only addresses.
main () at blink.c:164
164     {
(gdb)

Seems me kind of unnecessary noise although nothing wrong happened. Besides
that it works correctly, no nonsense unwinds observed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #26 from tomas.vanek at fbl dot cz ---
Created attachment 13852
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13852&action=edit
Fixes for ARM M exception frame unwinder

Reworked Proposed alternative patch (515 bytes, patch)
2021-11-06 00:20 UTC, Fredrik Hederstierna

- rename detection function to arm_m_addr_is_exc_return
- exclude lockups
- add all possible EXC_RETURN values in ARMv8m including Security Extension
- debug prints
- interstate exceptions (Secure->NonSecure and NonSecure->Secure)
  are not handled, unwinding is terminated

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #27 from tomas.vanek at fbl dot cz ---
Created attachment 13853
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13853&action=edit
ARM M lockup frame unwinder

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #28 from tomas.vanek at fbl dot cz ---
Created attachment 13854
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13854&action=edit
Show frame unwinder name in signal name

Shows more informative <signal arm m exception>
instead of <signal handler called>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Luis Machado <luis.machado at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |ASSIGNED

--- Comment #29 from Luis Machado <luis.machado at linaro dot org> ---
I think this is looking pretty good. Should we take it to the mailing list for
some feedback from others?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #30 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
I also think it looks good! Great job, smart idea with separate unwinder for
lockup. /Fredrik

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #31 from tomas.vanek at fbl dot cz ---
(In reply to Luis Machado from comment #29)
> Should we take it to the mailing list
> for some feedback from others?

Yes please, any feedback welcomed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarek.bouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #32 from Tarek BOCHKATI <tarek.bouchkati at gmail dot com> ---
Dear All,

FYI,

I have been informed that a colleague (christophe.lyon@foss.st.com) have posted
a PATCH series to address a similar issue.

I'm just pointing to the patch (since I haven't tested it yet):

[PATCH 4/5] gdb/arm: Add support for multiple stack pointers on Cortex-M
https://sourceware.org/pipermail/gdb-patches/2022-January/185130.html

[PATCH 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add
unwind-ns-to-s command
https://sourceware.org/pipermail/gdb-patches/2022-January/185131.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #33 from Luis Machado <luis.machado at linaro dot org> ---
Yeah. He has posted a comment previously. I'm going through his series at the
moment.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #34 from Luis Machado <luis.machado at linaro dot org> ---
Please let us know how it works on your end. I don't have a proper setup to
test this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[clyon at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #35 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Hi, I wanted to update this bug report but you have been faster than me :-)

Note that you'll need patches 1-3 too (well, maybe the prologue analysis is not
needed in your case, but skipping it may cause incorrect backtrace)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tarekbouchkati at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #36 from tarek-bochkati <tarekbouchkati at gmail dot com> ---
Hi Christophe,

Sure thing, previous changes are needed ;)
I have just pointed out the patches with subjects explicitly related to the
issue.

I will give them a try on real crash scenarios and let you know with my
findings.

Thanks !
Tarek


Le mer. 19 janv. 2022 à 12:40, clyon at gcc dot gnu.org <
sourceware-bugzilla@sourceware.org> a écrit :

> https://sourceware.org/bugzilla/show_bug.cgi?id=28549
>
> --- Comment #35 from Christophe Lyon <clyon at gcc dot gnu.org> ---
> Hi, I wanted to update this bug report but you have been faster than me :-)
>
> Note that you'll need patches 1-3 too (well, maybe the prologue analysis
> is not
> needed in your case, but skipping it may cause incorrect backtrace)
>
> --
> You are receiving this mail because:
> You reported the bug.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #37 from Luis Machado <luis.machado at arm dot com> ---
Was this ever addressed upstream? Is there an updated patch?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #38 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
I don't think Tomas have rebased these three great patches attached 2021-12-14,
and unfortunately I haven't had time to look further into it myself either.
Don't know if any of the developers working with similar functionalities at ST
have done any similar patch, or could help or support in trying to improve the
lockup state handling using any ideas from these attachments?
Thanks! Kindly, Fredrik

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #39 from Luis Machado <luis.machado at arm dot com> ---
There has been a recent round of contributions by ST around trustzone. Commit
ef273377587d touches arm_m_addr_is_magic, but it may not account for all
possible return magic codes.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #40 from tomas.vanek at fbl dot cz ---
Fredrik is right, I was too busy and completely forgot about this.

Luis, what am I expected to do to get the patch upstreamed?
And could you point to the conflicting changes in frame unwinder code if there
is more recent work than that one pointed by Tarek?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #41 from Luis Machado <luis.machado at arm dot com> ---
Hi Tomas,

You can refer to https://sourceware.org/gdb/wiki/ContributionChecklist for some
information on how to contribute. You'll need a FSF copyright assignment in
place to be able to contribute non-trivial changes. Do you have one?

As for the change, here's what git shows for that commit:

@@ -695,28 +730,43 @@ arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR
memaddr)
    0xFFFFFFBC    Return to Thread mode using the process stack.  */

 static int
-arm_m_addr_is_magic (CORE_ADDR addr)
-{
-  switch (addr)
-    {
-      /* Values from ARMv8-M Architecture Technical Reference.  */
-      case 0xffffffb0:
-      case 0xffffffb8:
-      case 0xffffffbc:
-      /* Values from Tables in B1.5.8 the EXC_RETURN definitions of
-        the exception return behavior.  */
-      case 0xffffffe1:
-      case 0xffffffe9:
-      case 0xffffffed:
-      case 0xfffffff1:
-      case 0xfffffff9:
-      case 0xfffffffd:
-       /* Address is magic.  */
-       return 1;
+arm_m_addr_is_magic (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+  if (tdep->have_sec_ext)
+    {
+      switch ((addr & 0xff000000))
+       {
+       case 0xff000000: /* EXC_RETURN pattern.  */
+       case 0xfe000000: /* FNC_RETURN pattern.  */
+         return 1;
+       default:
+         return 0;
+       }
+    }
+  else
+    {
+      switch (addr)
+       {
+         /* Values from ARMv8-M Architecture Technical Reference.  */
+       case 0xffffffb0:
+       case 0xffffffb8:
+       case 0xffffffbc:
+         /* Values from Tables in B1.5.8 the EXC_RETURN definitions of
+            the exception return behavior.  */
+       case 0xffffffe1:
+       case 0xffffffe9:
+       case 0xffffffed:
+       case 0xfffffff1:
+       case 0xfffffff9:
+       case 0xfffffffd:
+         /* Address is magic.  */
+         return 1;

-      default:
-       /* Address is not magic.  */
-       return 0;
+       default:
+         /* Address is not magic.  */
+         return 0;
+       }
     }
 }

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #42 from tomas.vanek at fbl dot cz ---
(In reply to Luis Machado from comment #41)

> You'll need a FSF copyright
> assignment in place to be able to contribute non-trivial changes. Do you
> have one?

No, I do not.

> As for the change, here's what git shows for that commit:

Thanks, I've already seen the code in the current git master.
I asked to find if there is some new pending work not yet merged to master
branch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #43 from Luis Machado <luis.machado at arm dot com> ---
(In reply to tomas.vanek from comment #42)
> (In reply to Luis Machado from comment #41)
> 
> > You'll need a FSF copyright
> > assignment in place to be able to contribute non-trivial changes. Do you
> > have one?
> 
> No, I do not.
> 

You will need to get one (or be covered by one by your employer, if there is
one).

More information here: https://sourceware.org/gdb/wiki/ContributionChecklist,
item 6.

If you don't want to do it, someone else with copyright assignment in place
will need to write their own code to implement this change.

> > As for the change, here's what git shows for that commit:
> 
> Thanks, I've already seen the code in the current git master.
> I asked to find if there is some new pending work not yet merged to master
> branch.

Ah, sorry. I misunderstood. No, I don't think there are further changes planned
for this particular piece of code.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[torbjorn.svensson at st dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Torbjörn SVENSSON <torbjorn.svensson at st dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |torbjorn.svensson at st dot com

--- Comment #44 from Torbjörn SVENSSON <torbjorn.svensson at st dot com> ---
(In reply to tomOas.vanek from comment #42)
> (In reply to Luis Machado from comment #41)
> 
> > You'll need a FSF copyright
> > assignment in place to be able to contribute non-trivial changes. Do you
> > have one?
> 
> No, I do not.

Any progress on the FSF copyright?

> > As for the change, here's what git shows for that commit:
> 
> Thanks, I've already seen the code in the current git master.
> I asked to find if there is some new pending work not yet merged to master
> branch.

I have some more pending patches for Cortex-M unwinding, but nothing that
touches the arm_m_addr_is_magic. However, I don't think you need to modify this
function.
I did try attachment 13853 a few weeks ago on top of the other changes that
I've submitted so far and it worked. I have some minor comments on it, that I
shared in the private mail-thread with subject "Cortex-M lockup patch question"
that Luis CCed you on.
If you send attachment 13853 to gdb-patches@soureware.org, I can reply with my
comments inline in you patch and we can take it from there?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #45 from tomas.vanek at fbl dot cz ---
(In reply to Torbjörn SVENSSON from comment #44)
> 
> Any progress on the FSF copyright?

I sent the signed form 2 weeks ago.
I'm still waiting for a reply with approval indication.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #46 from Luis Machado <luis.machado at arm dot com> ---
Hi Tomas,

Could you please ping copyright-clerk@fsf.org about the status of your
assignment?

I checked this morning and it seems your copyright isn't in place yet.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[torbjorn.svensson at st dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #47 from Torbjörn SVENSSON <torbjorn.svensson at st dot com> ---
(In reply to tomas.vanek from comment #45)
> (In reply to Torbjörn SVENSSON from comment #44)
> > 
> > Any progress on the FSF copyright?
> 
> I sent the signed form 2 weeks ago.
> I'm still waiting for a reply with approval indication.

From what I heard, the FSF copyright process has now completed.

Tomas, can you please send your patch to the gdb-patches ML in order to get it
merged after a review process? If it helps, I can help with adapting the patch
for the review comments, but you need to send the initial version.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[torbjorn.svensson at st dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #48 from tomas.vanek at fbl dot cz ---
(In reply to Torbjörn SVENSSON from comment #47)
> 
> From what I heard, the FSF copyright process has now completed.

Yes, it's finally done. Sorry, it was my fault: I replied to
copyright-clerk@fsf.org first instead of sending the signed form to
assign@gnu.org

> Tomas, can you please send your patch to the gdb-patches ML in order to get
> it merged after a review process? If it helps, I can help with adapting the
> patch for the review comments, but you need to send the initial version.

Will do.

The discussion here narrowed to 'ARM M lockup frame unwinder' patch. I
originally attached 3 patches, see above. TBH I never intended to use just
the unwinder without accompanying changes. 
'Show frame unwinder name in signal name' gives user little bit better info
than <signal handler called> - it might be mergeable (I didn't check, will do).
The rest of work in 'Fixes for ARM M exception frame unwinder' is probably
definitely unusable because of merging ARMv8 profile M security extension
support.
I need some time to check the new code and see if some parts of my old changes
are applicable.

Back to 'ARM M lockup frame unwinder' and Torbjörn's comments Fredrik forwarded
me by mail:

> 1. arm_m_lockup_prev_register
> I think arm_m_lockup_prev_register should have an assert statement as it
> should never be reached unless the sniffer is incorrectly implemented.

I think the same but I was not able to find the part of the general frame code
(frame-unwind.c, frame.c ...) which ensures it. That's why I wanted to avoid
gdb fail or assert because post-mortem debugging of frame related structures
seemed me almost impossible.
So if you confirm that prev_register method cannot be called when this_id
method returned outer_frame in any corner case like sniffing of corrupted data,
then ok, let's just assert(0) in prev_register

> 2. arm_m_lockup_unwind_sniffer
> I would have inlined the arm_m_addr_is_lockup function here as there is
> only one call to it and it would make it easier to see what is happening.

I just kept the format of original arm_m_addr_is_magic().
Maybe keeping all tests for 'special' addresses together has some advantage in
code readability.

> 3. registering the lockup unwinder
> I think it makes sense to have the lockup unwinder as early in the list
> as possible to avoid any of the other unwinders to preserve the state of
> the target as much as possible

Well the lockup unwinder gets registered with the exception unwinder as the
first two unwinders. You might miss the code from 'Fixes for ARM M exception
frame unwinder' - with this patch, lockup and exception are sniffed as mutually
exclusive, no matter which one is sniffed first. I agree, without the second
patch the lockup unwinder registration should precede the exception unwinder,
especially with ARMv8 profile M security extension with arm_m_addr_is_magic()
catching lockup magic codes.

I looked to the new code to support ARMv8 profile M security extension just
very shortly.
I wonder why arm_m_addr_is_magic() decoding differs so much in branches without
and with the security extension. The former decodes just the used special
addresses, the latter whole pages with lot of 'undefined behaviour' codes.
IMO the frame unwinding should proceed as long as sniffers encounter frames
possibly saved by the processor and stop as soon as a sniffer hits something
suspicious like a special address described as 'undefined behaviour' in
architecture manual. Stopping frame unwinding seems me much better than
generating nonsense frames from corrupted data.
Considering this I think we need another meta unwinder - or better said unwind
stopper for 'undefined behaviour'. Registered as the last one and catching
everything what all other sniffers refuse.
What do you think?

--- Comment #49 from Torbjörn SVENSSON <torbjorn.svensson at st dot com> ---
(In reply to tomas.vanek from comment #48)
> The discussion here narrowed to 'ARM M lockup frame unwinder' patch. I
> originally attached 3 patches, see above. TBH I never intended to use just
> the unwinder without accompanying changes. 

Right now, I'm more concerned about the endless(?) loop when the target is
in lockup state. I think we should first solve that problem and then look
into if there is improvements to do to make it more obvious for the user
what's going on.

> 'Show frame unwinder name in signal name' gives user little bit better info
> than <signal handler called> - it might be mergeable (I didn't check, will
> do).

I think this patch could be an improvement, but not as important as the
previous one. Lets focus on getting the other one in first, okay?

> The rest of work in 'Fixes for ARM M exception frame unwinder' is probably
> definitely unusable because of merging ARMv8 profile M security extension
> support.

I don't think any of the other changes are needed anymore.

> I need some time to check the new code and see if some parts of my old
> changes are applicable.
> 
> Back to 'ARM M lockup frame unwinder' and Torbjörn's comments Fredrik
> forwarded me by mail:
> 
> > 1. arm_m_lockup_prev_register
> > I think arm_m_lockup_prev_register should have an assert statement as it
> > should never be reached unless the sniffer is incorrectly implemented.
> 
> I think the same but I was not able to find the part of the general frame
> code (frame-unwind.c, frame.c ...) which ensures it. That's why I wanted to
> avoid gdb fail or assert because post-mortem debugging of frame related
> structures seemed me almost impossible.
> So if you confirm that prev_register method cannot be called when this_id
> method returned outer_frame in any corner case like sniffing of corrupted
> data, then ok, let's just assert(0) in prev_register

I just landed a patch that shows how you can use the 
frame_unwind_stop_reason hook to stop the unwinding. I think the same logic
could be used in your patch too.
Regarding the assurance, please see Pedro's reply to the very same question
here: https://sourceware.org/pipermail/gdb-patches/2022-October/192655.html

> > 2. arm_m_lockup_unwind_sniffer
> > I would have inlined the arm_m_addr_is_lockup function here as there is
> > only one call to it and it would make it easier to see what is happening.
> 
> I just kept the format of original arm_m_addr_is_magic().
> Maybe keeping all tests for 'special' addresses together has some advantage
> in code readability.
> 
> > 3. registering the lockup unwinder
> > I think it makes sense to have the lockup unwinder as early in the list
> > as possible to avoid any of the other unwinders to preserve the state of
> > the target as much as possible
> 
> Well the lockup unwinder gets registered with the exception unwinder as the
> first two unwinders. You might miss the code from 'Fixes for ARM M exception
> frame unwinder' - with this patch, lockup and exception are sniffed as
> mutually exclusive, no matter which one is sniffed first. I agree, without
> the second patch the lockup unwinder registration should precede the
> exception unwinder,
> especially with ARMv8 profile M security extension with
> arm_m_addr_is_magic() catching lockup magic codes.
> 
> I looked to the new code to support ARMv8 profile M security extension just
> very shortly.
> I wonder why arm_m_addr_is_magic() decoding differs so much in branches
> without and with the security extension. The former decodes just the used
> special addresses, the latter whole pages with lot of 'undefined behaviour'
> codes.

On https://developer.arm.com/documentation/100701/0200/The-EXC-RETURN-register,
it's written as that 0xffxxxxxx is an EXC_RETURN pattern, even if there are
combinations within that that is illegal.

> IMO the frame unwinding should proceed as long as sniffers encounter frames
> possibly saved by the processor and stop as soon as a sniffer hits something
> suspicious like a special address described as 'undefined behaviour' in
> architecture manual. Stopping frame unwinding seems me much better than
> generating nonsense frames from corrupted data.

I agree completely with this statement.

> Considering this I think we need another meta unwinder - or better said
> unwind stopper for 'undefined behaviour'. Registered as the last one and
> catching everything what all other sniffers refuse.
> What do you think?

I don't think we need another unwinder, but we might need to have some more
checks in the arm_m_exception_cache function to stop the unwind in case an
unsupported state is detected.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[tomas.vanek at fbl dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

--- Comment #50 from tomas.vanek at fbl dot cz ---
I finally submitted
https://sourceware.org/pipermail/gdb-patches/2022-October/192716.html

(In reply to Torbjörn SVENSSON from comment #49)
> 
> Right now, I'm more concerned about the endless(?) loop when the target is
> in lockup state.

Could it be explained by unwinding with wrong SP value because the size
of the next outer frame cannot be determined because of unknown pre-lockup PC?

> > 'Show frame unwinder name in signal name' gives user little bit better info
> > than <signal handler called> - it might be mergeable (I didn't check, will
> > do).
> 
> I think this patch could be an improvement, but not as important as the
> previous one. Lets focus on getting the other one in first, okay?

Because you doesn't seem to be happy with
'Show frame unwinder name in signal name'...

> I just landed a patch that shows how you can use the 
> frame_unwind_stop_reason hook to stop the unwinding. I think the same logic
> could be used in your patch too.

... and your patch made much easier to stop unwinding without misleading
messages, I simplified the change. Dedicated lockup unwinder is dropped,
I added the equal functionality to the exception unwinder.

TBH Personally I would prefer 3 dedicated unwinders: exception (EXC_RETURN),
FNC_RETURN and lockup. I think that performance penalty would be negligible
as the sniffers are simple and fast and the code would be cleaner. However I'm
not so familiar with gdb code so I respect your preference.

BTW Thanks for the pointer to
> Regarding the assurance, please see Pedro's reply to the very same question
> here: https://sourceware.org/pipermail/gdb-patches/2022-October/192655.html

and submitting your
'[PATCH v4] gdb/arm: Stop unwinding on error, but do not assert'
and
'[PATCH] gdb/arm: Don't rely on loop detection to stop unwind'

They really make a difference.
But there is a small drawback: as the warning messages are printed out in time
of building cache, they appear just once and if user e.g. repeats 'backtrace'
command he gets no more messages.


> > I wonder why arm_m_addr_is_magic() decoding differs so much in branches
> > without and with the security extension. The former decodes just the used
> > special addresses, the latter whole pages with lot of 'undefined behaviour'
> > codes.
> 
> On
> https://developer.arm.com/documentation/100701/0200/The-EXC-RETURN-register,
> it's written as that 0xffxxxxxx is an EXC_RETURN pattern, even if there are
> combinations within that that is illegal.

Hmm, this document really looks like the devices without sec ext have just
3 listed EXC_RETURNs. 'Arm®v8-M Architecture Reference Manual', D1.2.95
'EXC_RETURN, Exception Return Payload' makes clear that the payload format and
bits assignment are same for both with and without sec ext, just some bits are
reserved in the case 'without sec'.
So IMO detecting EXC_RETURN on a device with security extension should treat
reserved bits same way as doing so on dev w/o sec, not to do the quite
opposite.

And one more suspicious point: ARMv8-M RM describes the FType bit 4
as it does not depend on sec ext. Correct me if I'm wrong it would mean
that ARMv8-M device with FP and without security used also 0xFFFFFFA0,
0xFFFFFFA8 and 0xFFFFFFAC EXC_RETURNs. I don't have such device
neither I'm not aware of any, it even does not exist at all.

BTW it's quite easy to trigger the internal error at the end of
arm_m_exception_cache(). Just set PC register to any of magic codes
(I admit that doing so has little sense except showing another reason to check
the reserved bits):

On STM32L5 ARMv8-M with sec ext:

(gdb) set $pc=0xff000000
../../gdb/arm-tdep.c:3684: internal-error: While unwinding an exception frame,
found unexpected Link Register value 0c0002cb.  This should not happen and
maybe caused by corrupt data or a bug in GDB.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0x8119852 gdb_internal_backtrace_1
        ../../gdb/bt-utils.c:122
0x8119852 _Z22gdb_internal_backtracev
        ../../gdb/bt-utils.c:168
0x843a40c internal_vproblem
        ../../gdb/utils.c:396
0x843a79e _Z15internal_verrorPKciS0_Pc
        ../../gdb/utils.c:476
0x8528326 _Z14internal_errorPKciS0_z
        ../../gdbsupport/errors.cc:58
0x80cf181 arm_m_exception_cache
        ../../gdb/arm-tdep.c:3689
0x80cff77 arm_m_exception_this_id
        ../../gdb/arm-tdep.c:3703
0x8257bbe compute_frame_id
        ../../gdb/frame.c:594
0x8257e49 _Z12get_frame_id14frame_info_ptr
        ../../gdb/frame.c:637
0x825b185 _Z16frame_find_by_id8frame_id
        ../../gdb/frame.c:906
0x8447bbf _Z12value_assignP5valueS0_
        ../../gdb/valops.c:1304
0x8226a46 _ZN10expression8evaluateEP4type6noside
        ../../gdb/eval.c:101
0x8226e8e _Z19evaluate_expressionP10expressionP4type
        ../../gdb/eval.c:115
0x8328a4e set_command
        ../../gdb/printcmd.c:1519
0x814b1aa _Z8cmd_funcP16cmd_list_elementPKci
        ../../gdb/cli/cli-decode.c:2543
0x84001bb _Z15execute_commandPKci
        ../../gdb/top.c:692
0x822dff4 _Z15command_handlerPKc
        ../../gdb/event-top.c:616
0x822e366 _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE
        ../../gdb/event-top.c:860

On Kinetis K22, Cortex-M4 FP:

(gdb) set $pc=0xFFFFFFBC
the same error follows...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |WAITING

--- Comment #51 from Luis Machado <luis.machado at arm dot com> ---
Patch under review.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[brobecker at gnat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Joel Brobecker <brobecker at gnat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |brobecker at gnat dot com

--- Comment #52 from Joel Brobecker <brobecker at gnat dot com> ---
IIUC, is the following commit fixing this PR allowing us to close it?

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8b73ee207c9c4b2d692a8
2a29d1cee2dcfa07394

According to Torbjorn's email
(https://sourceware.org/pipermail/gdb-patches/2022-October/193182.html), he
said all his items for the GDB 13 release have been handled, and this was one
of them, so hoping I found the right patch to close this PR...

Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28549] ARM/Cortex-M: improper stack unwinding when the target is in lockup state

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28549

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|WAITING                     |RESOLVED

--- Comment #53 from Luis Machado <luis.machado at arm dot com> ---
Joel,

Tomas' patch was pushed (although unfortunately incorrectly authored due to me
missing the --author tweak) to fix this. It should be safe to close it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.