[Bug tdep/29716] New: Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[Bug tdep/29716] New: Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

            Bug ID: 29716
           Summary: Arm v8 M-profile FNC_RETURN unwinder uses wrong stack
           Product: gdb
           Version: HEAD
            Status: UNCONFIRMED
          Severity: minor
          Priority: P2
         Component: tdep
          Assignee: unassigned at sourceware dot org
          Reporter: tomas.vanek at fbl dot cz
  Target Milestone: ---

Discovered on STM32L552 with Cortex-M33, should be same for all ARMv8-M devices
with the security extension.

A secure code runs in thread mode and uses MSP_S because CONTROL_S bit SPSEL =
0
A non-secure function is called from secure code.
Command 'backtrace' does not show the correct stack frames:

 (gdb) bt
 #0  NonSecureFc ()
 #1  <signal handler called>
 #2  0x00000000 in ?? ()
 Backtrace stopped: previous frame identical to this frame (corrupt stack?)

The problem is in arm-tdep.c:3394, function arm_m_exception_cache (),
block if (fnc_return):

  ULONGEST xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM);
  if ((xpsr & 0xff) != 0)
    /* Handler mode: This is the mode that exceptions are handled in.  */
    arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum);
  else
    /* Thread mode: This is the normal mode that programs run in.  */
    arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum);

This code does not comply
Arm®v8-M Architecture Reference Manual
B3.8 Stack pointer
IDMLS "In Thread mode, CONTROL.SPSEL determines whether the PE uses the main or
process stack"

and uses process stack regardless of SPSEL.

The test should check SPSEL bit of CONTROL_S register.
Unfortunately the current arm-tdep is not aware of this register, we need to
look up the register number in arm_gdbarch_init().

Moreover stack unwinders probably do not keep track of CONTROL_S updates.
Even if we neglect usually very seldom updates of SPSEL and use the value from
the innermost frame we have better chance to see correct stack frames.

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

--- Comment #1 from tomas.vanek at fbl dot cz ---
Another problem in unwinding of interstate returns:

A secure procedure with cmse_nonsecure_entry attribute is compiled with
an epilogue ending by
 bxns lr

When a non-secure context called such procedure, the stack unwinder (usually
dwarf2 unwinder) does not know about cmse_nonsecure_entry attribute, does not
see 'bxns' at the return and therefore assumes normal return keeping security
state unchanged. This causes incorrect unwinding of following frames as the
secure stack is used instead of insecure.

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

--- Comment #2 from tomas.vanek at fbl dot cz ---
And for completeness the third problem (not limited to secure extension):
The simple core_addr_lessthan() used for set_gdbarch_inner_than()
is not sufficient with multiple stack. Depending on placement
of the secure and non-secure stacks (or the main and process stacks)
unwinding is terminated with the false error:
 Backtrace stopped: previous frame inner to this frame (corrupt stack?)

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[torbjorn.svensson at st dot com]

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

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

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

--- Comment #3 from Torbjörn SVENSSON <torbjorn.svensson at st dot com> ---
(In reply to tomas.vanek from comment #0)
> Discovered on STM32L552 with Cortex-M33, should be same for all ARMv8-M
> devices with the security extension.
> 
> A secure code runs in thread mode and uses MSP_S because CONTROL_S bit SPSEL
> = 0
> A non-secure function is called from secure code.
> Command 'backtrace' does not show the correct stack frames:
> 
>  (gdb) bt
>  #0  NonSecureFc ()
>  #1  <signal handler called>
>  #2  0x00000000 in ?? ()
>  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> The problem is in arm-tdep.c:3394, function arm_m_exception_cache (),
> block if (fnc_return):
> 
>   ULONGEST xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM);
>   if ((xpsr & 0xff) != 0)
>     /* Handler mode: This is the mode that exceptions are handled in.  */
>     arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum);
>   else
>     /* Thread mode: This is the normal mode that programs run in.  */
>     arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum);
> 
> This code does not comply
> Arm®v8-M Architecture Reference Manual
> B3.8 Stack pointer
> IDMLS "In Thread mode, CONTROL.SPSEL determines whether the PE uses the main
> or process stack"
> 
> and uses process stack regardless of SPSEL.
> 
> The test should check SPSEL bit of CONTROL_S register.
> Unfortunately the current arm-tdep is not aware of this register, we need to
> look up the register number in arm_gdbarch_init().
> 
> Moreover stack unwinders probably do not keep track of CONTROL_S updates.
> Even if we neglect usually very seldom updates of SPSEL and use the value
> from the innermost frame we have better chance to see correct stack frames.


I have not been able to reproduce this issue after applying below patch series.
Maybe they are still there, maybe they are indirectly fixed.
https://sourceware.org/pipermail/gdb-patches/2022-November/193391.html

@Tomas, can you take a look at them and see if it resolves the issues you've
seen?

Regarding the use of CONTROL.SPSEL instead of the xspr value; you can't use
CONTROL as it's not stacked on EXC_RETURN/FNC_RETURN and to my knowledge, it's
not part of the DWARF2 info either.

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

--- Comment #4 from tomas.vanek at fbl dot cz ---
(In reply to Torbjörn SVENSSON from comment #3)
> 
> I have not been able to reproduce this issue after applying below patch
> series. Maybe they are still there, maybe they are indirectly fixed.
> https://sourceware.org/pipermail/gdb-patches/2022-November/193391.html
> 
> @Tomas, can you take a look at them and see if it resolves the issues you've
> seen?

Tested with the patch series and all 3 problems persist.

> Regarding the use of CONTROL.SPSEL instead of the xspr value; you can't use
> CONTROL as it's not stacked on EXC_RETURN/FNC_RETURN and to my knowledge,
> it's not part of the DWARF2 info either.

Oh yes, I suspected it.
The only viable solution for FNC_RETURN seems me to use CONTROL_S from the
actual CPU register. Typically the SPSEL bit is set at the app initialisation
and then kept unchanged, so it will mostly work - at least unwinding will be
less broken than without any use of SPSEL.

Similarly we can use CONTROL_NS for return from secure to non-secure.
There is tricky to detect the security state change.

I made some code to address both interstate directions. I will modify it for
the latest arm-tdep.c with your patch series and send to patches ml.

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

--- Comment #5 from tomas.vanek at fbl dot cz ---
My modifications to ST GTZC_TZSC_MPCBB_TrustZone example to exercise FNC_RETURN
and return from secure to non-secure (LED3 indicates secure mode):

Secure/main.c:
  void SECURE_Mode(funcptr back_ns);
  void NSCalledFromSec(void);
  void NSCalledFromNS(void);
  void SecCalledFromNS(void);

  static void SecCalledFromSec(funcptr back_ns)
  {
        if (back_ns) {
                back_ns();
        }
  }

  CMSE_NS_ENTRY void SECURE_Mode(funcptr back_ns)
  {
        HAL_GPIO_WritePin(LED3_GPIO_Port, LED3_Pin, 1);

        SecCalledFromSec(back_ns);
  }

  CMSE_NS_ENTRY void SecCalledFromNS(void)
  {
        HAL_GPIO_WritePin(LED3_GPIO_Port, LED3_Pin, 1);
  }

NonSecure/main.c:

  void NSCalledFromNS(void)
  {
        SecCalledFromNS();
        HAL_GPIO_WritePin(LED3_GPIO_Port, LED3_Pin, 0);
  }

  void NSCalledFromSec(void)
  {
        HAL_GPIO_WritePin(LED3_GPIO_Port, LED3_Pin, 0);
        NSCalledFromNS();
  }

  int main(void)
  {
...
add to while loop:
        SECURE_Mode(&NSCalledFromSec);
        HAL_GPIO_WritePin(LED3_GPIO_Port, LED3_Pin, 0);

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

--- Comment #6 from tomas.vanek at fbl dot cz ---
Desired 'bt' output:

(gdb) bt -frame-info location-and-address
#0  0x0c0004ae in SecCalledFromNS ()
    at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:124
#1  0x0804032e in NSCalledFromNS ()
    at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:173
#2  0x0804034a in NSCalledFromSec ()
    at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:179
#3  <signal handler called>
#4  0x0c00028c in __gnu_cmse_nonsecure_call ()
#5  0x0c0003ba in SecCalledFromSec (back_ns=0x8040341 <NSCalledFromSec>)
    at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:100
#6  0x0c0003e2 in SECURE_Mode (back_ns=0x8040341 <NSCalledFromSec>)
    at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:108
#7  0x0804047c in main ()
    at
C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:307
#8  0x0804026e in Reset_Handler ()
    at ../Application/Startup/startup_stm32l552zetxq.s:99

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

[Bug tdep/29716] Arm v8 M-profile FNC_RETURN unwinder uses wrong stack

[tomas.vanek at fbl dot cz]

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

--- Comment #7 from tomas.vanek at fbl dot cz ---
Torbjorn,

I sent https://sourceware.org/pipermail/gdb-patches/2022-November/193431.html
and 4 others marked as RFC. Please take a look.

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