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

[Luis Machado <luis.machado@arm.com>]

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.