RE: [PATCH 3/3] gdb/arm: Track msp and psp

[Torbjorn SVENSSON <torbjorn.svensson@st.com>]

Hello,


> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: den 19 maj 2022 10:08
> To: Torbjorn SVENSSON <torbjorn.svensson@st.com>; Yvan ROUX - foss
> <yvan.roux@foss.st.com>; gdb-patches@sourceware.org
> Subject: Re: [PATCH 3/3] gdb/arm: Track msp and psp
> 
> On 5/18/22 20:18, Torbjorn SVENSSON wrote:
> >
> >
> > ST Restricted
> >
> >
> >> -----Original Message-----
> >> From: Luis Machado <luis.machado@arm.com>
> >> Sent: den 17 maj 2022 17:53
> >> To: Yvan ROUX - foss <yvan.roux@foss.st.com>; gdb-
> >> patches@sourceware.org
> >> Cc: Torbjorn SVENSSON <torbjorn.svensson@st.com>
> >> Subject: Re: [PATCH 3/3] gdb/arm: Track msp and psp
> >>
> >> On 5/16/22 15:00, Yvan Roux via Gdb-patches wrote:
> >>> For Arm Cortex-M33 with security extensions, there are 4 different
> >>> stacks pointers (msp_s, msp_ns, psp_s, psp_ns).  To be compatible
> >>
> >> stacks -> stack
> >>
> >>> with earlier Cortex-M derivates, the msp and psp register are
> >>
> >> register -> registers
> >>
> >>> aliases for one of the 4 real stack pointer registers.
> >>>
> >>> These are the combinations that exist:
> >>> sp -> msp -> msp_s
> >>> sp -> msp -> msp_ns
> >>> sp -> psp -> psp_s
> >>> sp -> psp -> psp_ns
> >>>
> >>> This means that when the GDB client is to show the value of "msp",
> >>> the value should always be equal to either "msp_s" or "msp_ns".
> >>> Same goes for "psp".
> >>>
> >>
> >> As a general comment, we should include, in the commit message, the
> bug
> >> ticket link related to this problem:
> >>
> >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29121
> >>
> >>> Signed-off-by: Torbj�rn SVENSSON <torbjorn.svensson@st.com>
> >>> Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> >>> ---
> >>>    gdb/arm-tdep.c | 18 ++++++++++++++++++
> >>>    1 file changed, 18 insertions(+)
> >>>
> >>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> >>> index 4aa277f5bc8..b9c35bcdae6 100644
> >>> --- a/gdb/arm-tdep.c
> >>> +++ b/gdb/arm-tdep.c
> >>> @@ -288,6 +288,8 @@ struct arm_prologue_cache
> >>>
> >>>      /* Active stack pointer.  */
> >>>      int active_sp_regnum;
> >>> +  int active_msp_regnum;
> >>> +  int active_psp_regnum;
> >>
> >> Is it possible to have 3 active SP registers at the same time? Are msp and
> psp
> >> really needed here or are we just not doing a good
> >> job of figuring out what SP maps to? (msp_s, msp_ns, psp_s, psp_ns)
> >
> > Only one is "active", but you can view them all at the same time.
> > The problem without tracking msp and psp (without suffixes) is that a
> simple "info registers" will not have anything to show for these 2 registers:
> >
> 
> That's fine, but the naming is incorrect and possibly misleading. My
> understanding is that there is only SP (the active stack pointer). Yes, we can
> track the values of
> the other registers, but the code should make that clear.

Depends on how you look at it.

At any given moment, then sp will have the same value as either msp or psp.
At the same time, msp must have the same value as either msp_s or msp_ns. Same goes for psp.

So, the core only "works" on sp, but the user needs to be able to see the state of all the different stack pointers at any time.

Due to the constraint in hardware that msp is either msp_s or msp_ns and that sp is either msp or psp, the value of them can never diverge in the hardware, that's why we need to keep track of what msp and psp "points" to. Do you have a better terminology that "active_msp_regnum" for the "pointer"?


> 
> > The following was executed in secure context of a Cortex-M33.
> > info r
> > r0             0x30000000          805306368
> > r1             0x74                116
> > r2             0x300000e8          805306600
> > r3             0xc00021d           201327133
> > r4             0x0                 0
> > r5             0x0                 0
> > r6             0x0                 0
> > r7             0x30017fe0          805404640
> > r8             0xffffffff          -1
> > r9             0xffffffff          -1
> > r10            0xffffffff          -1
> > r11            0xffffffff          -1
> > r12            0xffffffff          -1
> > sp             0x30017fe0          0x30017fe0
> > lr             0xc00ef57           201387863
> > pc             0xc00dc6e           0xc00dc6e <main+6>
> > xpsr           0x61000000          1627389952
> > fpscr          0x0                 0
> > primask        0x0                 0
> > basepri        0x0                 0
> > faultmask      0x0                 0
> > control        0x0                 0
> > msp            0x30017fe0          0x30017fe0
> > psp            0x0                 0x0
> > msp_ns         0x0                 0x0
> > psp_ns         0xfffffffc          0xfffffffc
> > msp_s          0x30017fe0          0x30017fe0
> > psp_s          0x0                 0x0
> > msplim_s       0x0                 0x0
> > psplim_s       0x0                 0x0
> > msplim_ns      0x0                 0x0
> > psplim_ns      0x0                 0x0
> > primask_s      0x0                 0
> > basepri_s      0x0                 0
> > faultmask_s    0x0                 0
> > control_s      0x0                 0
> > primask_ns     0x0                 0
> > basepri_ns     0x0                 0
> > faultmask_ns   0x0                 0
> > control_ns     0x0                 0
> >
> >
> > As the registers are listed, they need to be tracked, or do you see any
> other way to give the right value for the register?
> 
> The most important thing is for GDB to produce a correct stack trace. If it is
> not doing that properly, then it is failing to calculate the correct SP.

The issue this patch is attempting to correct does not have any implication on the unwinding itself, but as the previous patches by Christophe Lyon added handling of the 4 different stack pointers, the patch did not include the mandatory tracking of the msp and psp "pointers" and as a result, without this patch, an assert will be triggered when doing "info r" in some situations.

> I don't particularly understand what tracking msp/psp has to do with
> calculating the correct SP here. Maybe I'm missing some context.

Well, nothing :)

Kind regards,
Torbjörn