public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Christophe Lyon <christophe.lyon@arm.com>
To: gdb-patches@sourceware.org
Subject: Re: [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command
Date: Thu, 28 Apr 2022 10:55:34 +0200	[thread overview]
Message-ID: <715a5e5d-431a-2c68-6790-7b26ba72ebda@arm.com> (raw)
In-Reply-To: <0831ab73-182f-ad9d-b14d-01ea4d2b48d5@arm.com>



On 4/27/22 15:11, Christophe Lyon via Gdb-patches wrote:
> 
> 
> On 4/27/22 15:09, Luis Machado wrote:
>> On 4/25/22 10:59, Christophe Lyon via Gdb-patches wrote:
>>> This patch makes use of the support for several stack pointers
>>> introduced by the previous patch to switch between them as needed
>>> during unwinding.
>>>
>>> It introduces a new 'unwind-secure-frames' arm command to enable/disable
>>> mode switching during unwinding. It is enabled by default.
>>>
>>> It has been tested using an STM32L5 board (with cortex-m33) and the
>>> sample applications shipped with the STM32Cube development
>>> environment: GTZC_TZSC_MPCBB_TrustZone in
>>> STM32CubeL5/Projects/NUCLEO-L552ZE-Q/Examples/GTZC.
>>>
>>> The test consisted in setting breakpoints in various places and check
>>> that the backtrace is correct: SecureFault_Callback (Non-secure mode),
>>> __gnu_cmse_nonsecure_call (before and after the vpush instruction),
>>> SecureFault_Handler (Secure mode).
>>>
>>> This implies that we tested only some parts of this patch (only MSP*
>>> were used), but remaining parts seem reasonable.
>>>
>>> Signed-off-by: Torbjörn Svensson <torbjorn.svensson@st.com>
>>> Signed-off-by: Christophe Lyon <christophe.lyon@foss.st.com>
>>> Signed-off-by: Christophe Lyon <christophe.lyon@arm.com>
>>> ---
>>>   gdb/NEWS            |   5 +
>>>   gdb/arm-tdep.c      | 314 +++++++++++++++++++++++++++++++++++---------
>>>   gdb/doc/gdb.texinfo |  10 ++
>>>   3 files changed, 267 insertions(+), 62 deletions(-)
>>>
>>> diff --git a/gdb/NEWS b/gdb/NEWS
>>> index 760cb2b7abc..982f4a1a18c 100644
>>> --- a/gdb/NEWS
>>> +++ b/gdb/NEWS
>>> @@ -5214,6 +5214,11 @@ show arm force-mode
>>>     the current CPSR value for instructions without symbols; previous
>>>     versions of GDB behaved as if "set arm fallback-mode arm".
>>> +set arm unwind-secure-frames
>>> +  Enable unwinding from Non-secure to Secure mode on Cortex-M with
>>> +  Security extension.
>>> +  This can trigger security exceptions when unwinding exception stacks.
>>> +
>>>   set disable-randomization
>>>   show disable-randomization
>>>     Standalone programs run with the virtual address space 
>>> randomization enabled
>>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>>> index 7a6c1e49125..7274752c2b9 100644
>>> --- a/gdb/arm-tdep.c
>>> +++ b/gdb/arm-tdep.c
>>> @@ -443,6 +443,40 @@ arm_cache_set_active_sp_value (struct 
>>> arm_prologue_cache *cache,
>>>     gdb_assert_not_reached ("Invalid SP selection");
>>>   }
>>> +/* Return true if REGNUM is one of the stack pointers.  */
>>> +
>>> +static bool
>>> +arm_cache_is_sp_register (struct arm_prologue_cache *cache,
>>> +              arm_gdbarch_tdep *tdep, int regnum)
>>> +{
>>> +  if ((regnum == ARM_SP_REGNUM)
>>> +      || (regnum == tdep->m_profile_msp_regnum)
>>> +      || (regnum == tdep->m_profile_msp_s_regnum)
>>> +      || (regnum == tdep->m_profile_msp_ns_regnum)
>>> +      || (regnum == tdep->m_profile_psp_regnum)
>>> +      || (regnum == tdep->m_profile_psp_s_regnum)
>>> +      || (regnum == tdep->m_profile_psp_ns_regnum))
>>> +    return true;
>>> +  else
>>> +    return false;
>>> +}
>>> +
>>> +/* Set the active stack pointer to SP_REGNUM.  */
>>> +
>>> +static void
>>> +arm_cache_switch_prev_sp (struct arm_prologue_cache *cache,
>>> +              arm_gdbarch_tdep *tdep, int sp_regnum)
>>> +{
>>> +  gdb_assert (sp_regnum != ARM_SP_REGNUM);
>>> +  gdb_assert (arm_cache_is_sp_register (cache, tdep, sp_regnum));
>>> +
>>> +  if (tdep->have_sec_ext)
>>> +    gdb_assert (sp_regnum != tdep->m_profile_msp_regnum
>>> +        && sp_regnum != tdep->m_profile_psp_regnum);
>>> +
>>> +  cache->active_sp_regnum = sp_regnum;
>>> +}
>>> +
>>>   namespace {
>>>   /* Abstract class to read ARM instructions from memory.  */
>>> @@ -479,6 +513,7 @@ static CORE_ADDR arm_analyze_prologue
>>>   /* See arm-tdep.h.  */
>>>   bool arm_apcs_32 = true;
>>> +bool arm_unwind_secure_frames = true;
>>>   /* Return the bit mask in ARM_PS_REGNUM that indicates Thumb mode.  */
>>> @@ -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;
>>> +    }
>>>       }
>>>   }
>>> @@ -728,7 +778,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 (tdep->is_m && arm_m_addr_is_magic (val))
>>> +  if (tdep->is_m && arm_m_addr_is_magic (gdbarch, val))
>>>       return val;
>>>     if (arm_apcs_32)
>>> @@ -2218,6 +2268,7 @@ arm_prologue_prev_register (struct frame_info 
>>> *this_frame,
>>>   {
>>>     struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>>     struct arm_prologue_cache *cache;
>>> +  CORE_ADDR sp_value;
>>>     if (*this_cache == NULL)
>>>       *this_cache = arm_make_prologue_cache (this_frame);
>>> @@ -2251,6 +2302,14 @@ arm_prologue_prev_register (struct frame_info 
>>> *this_frame,
>>>       return frame_unwind_got_constant (this_frame, prev_regnum,
>>>                         arm_cache_get_prev_sp_value (cache, tdep));
>>> +  /* The value might be one of the alternative SP, if so, use the
>>> +     value already constructed.  */
>>> +  if (arm_cache_is_sp_register (cache, tdep, prev_regnum))
>>> +    {
>>> +      sp_value = arm_cache_get_sp_register (cache, tdep, prev_regnum);
>>> +      return frame_unwind_got_constant (this_frame, prev_regnum, 
>>> sp_value);
>>> +    }
>>> +
>>>     /* The CPSR may have been changed by the call instruction and by the
>>>        called function.  The only bit we can reconstruct is the T bit,
>>>        by checking the low bit of LR as of the call.  This is a reliable
>>> @@ -3246,16 +3305,20 @@ static struct arm_prologue_cache *
>>>   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);
>>>     arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep 
>>> (gdbarch);
>>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>>     struct arm_prologue_cache *cache;
>>>     CORE_ADDR lr;
>>> +  CORE_ADDR sp;
>>>     CORE_ADDR unwound_sp;
>>> +  uint32_t sp_r0_offset = 0;
>>>     LONGEST xpsr;
>>>     uint32_t exc_return;
>>> -  uint32_t process_stack_used;
>>> +  bool fnc_return;
>>>     uint32_t extended_frame_used;
>>> -  uint32_t secure_stack_used;
>>> +  bool secure_stack_used = false;
>>> +  bool default_callee_register_stacking = false;
>>> +  bool exception_domain_is_secure = false;
>>>     cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
>>>     arm_cache_init (cache, this_frame);
>>> @@ -3265,35 +3328,123 @@ arm_m_exception_cache (struct frame_info 
>>> *this_frame)
>>>        to the exception and if FPU is used (causing extended stack 
>>> frame).  */
>>>     lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
>>> +  sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
>>> +
>>> +  fnc_return = ((lr & 0xfffffffe) == 0xfefffffe);
>>> +  if (tdep->have_sec_ext && fnc_return)
>>> +    {
>>> +      int actual_sp;
>>> +
>>> +      arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_msp_ns_regnum);
>>> +      arm_cache_set_active_sp_value (cache, tdep, sp);
>>> +      if (lr & 1)
>>> +    actual_sp = tdep->m_profile_msp_s_regnum;
>>> +      else
>>> +    actual_sp = tdep->m_profile_msp_ns_regnum;
>>> +
>>> +      arm_cache_switch_prev_sp (cache, tdep, actual_sp);
>>> +      sp = get_frame_register_unsigned (this_frame, actual_sp);
>>> -  /* Check EXC_RETURN indicator bits.  */
>>> -  exc_return = (((lr >> 28) & 0xf) == 0xf);
>>> +      cache->saved_regs[ARM_LR_REGNUM].set_addr (sp);
>>> -  /* 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)
>>> +      arm_cache_set_active_sp_value (cache, tdep, sp + 8);
>>> +
>>> +      return cache;
>>> +    }
>>> +
>>> +  /* Check EXC_RETURN indicator bits (24-31).  */
>>> +  exc_return = (((lr >> 24) & 0xff) == 0xff);
>>> +  if (exc_return)
>>>       {
>>> -      /* Thread (process) stack used, use PSP as SP.  */
>>> -      unwound_sp = get_frame_register_unsigned (this_frame, 
>>> tdep->m_profile_psp_regnum);
>>> +      /* Check EXC_RETURN bit SPSEL if Main or Thread (process) 
>>> stack used.  */
>>> +      bool process_stack_used = ((lr & (1 << 2)) != 0);
>>> +
>>> +      if (tdep->have_sec_ext)
>>> +    {
>>> +      secure_stack_used = ((lr & (1 << 6)) != 0);
>>> +      default_callee_register_stacking = ((lr & (1 << 5)) != 0);
>>> +      exception_domain_is_secure = ((lr & (1 << 0)) == 0);
>>> +
>>> +      /* Unwinding from non-secure to secure can trip security
>>> +         measures.  In order to avoid the debugger being
>>> +         intrusive, rely on the user to configure the requested
>>> +         mode.  */
>>> +      if (secure_stack_used && !exception_domain_is_secure
>>> +          && !arm_unwind_secure_frames)
>>> +        {
>>> +          warning (_("Non-secure to secure stack unwinding 
>>> disabled."));
>>> +
>>> +          /* Terminate any further stack unwinding by referring to 
>>> self.  */
>>> +          arm_cache_set_active_sp_value (cache, tdep, sp);
>>> +          return cache;
>>> +        }
>>> +
>>> +      if (process_stack_used)
>>> +        {
>>> +          if (secure_stack_used)
>>> +        /* Secure thread (process) stack used, use PSP_S as SP.  */
>>> +        arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_psp_s_regnum);
>>> +          else
>>> +        /* Non-secure thread (process) stack used, use PSP_NS as 
>>> SP.  */
>>> +        arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_psp_ns_regnum);
>>> +        }
>>> +      else
>>> +        {
>>> +          if (secure_stack_used)
>>> +        /* Secure main stack used, use MSP_S as SP.  */
>>> +        arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_msp_s_regnum);
>>> +          else
>>> +        /* Non-secure main stack used, use MSP_NS as SP.  */
>>> +        arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_msp_ns_regnum);
>>> +        }
>>> +    }
>>> +      else
>>> +    {
>>> +      if (process_stack_used)
>>> +        /* Thread (process) stack used, use PSP as SP.  */
>>> +        arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_psp_regnum);
>>> +      else
>>> +        /* Main stack used, use MSP as SP.  */
>>> +        arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_msp_regnum);
>>> +    }
>>>       }
>>>     else
>>>       {
>>>         /* Main stack used, use MSP as SP.  */
>>> -      unwound_sp = get_frame_register_unsigned (this_frame, 
>>> tdep->m_profile_msp_regnum);
>>> +      arm_cache_switch_prev_sp (cache, tdep, 
>>> tdep->m_profile_msp_regnum);
>>> +    }
>>> +
>>> +  /* Fetch the SP to use for this frame.  */
>>> +  unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);
>>> +
>>> +  /* With the Security extension, the hardware saves R4..R11 too.  */
>>> +  if (exc_return && tdep->have_sec_ext && secure_stack_used
>>> +      && (!default_callee_register_stacking || 
>>> exception_domain_is_secure))
>>> +    {
>>> +      /* Read R4..R11 from the integer callee registers.  */
>>> +      cache->saved_regs[4].set_addr (unwound_sp + 0x08);
>>> +      cache->saved_regs[5].set_addr (unwound_sp + 0x0C);
>>> +      cache->saved_regs[6].set_addr (unwound_sp + 0x10);
>>> +      cache->saved_regs[7].set_addr (unwound_sp + 0x14);
>>> +      cache->saved_regs[8].set_addr (unwound_sp + 0x18);
>>> +      cache->saved_regs[9].set_addr (unwound_sp + 0x1C);
>>> +      cache->saved_regs[10].set_addr (unwound_sp + 0x20);
>>> +      cache->saved_regs[11].set_addr (unwound_sp + 0x24);
>>> +      sp_r0_offset = 0x28;
>>>       }
>>>     /* The hardware saves eight 32-bit words, comprising xPSR,
>>>        ReturnAddress, LR (R14), R12, R3, R2, R1, R0.  See details in
>>>        "B1.5.6 Exception entry behavior" in
>>>        "ARMv7-M Architecture Reference Manual".  */
>>> -  cache->saved_regs[0].set_addr (unwound_sp);
>>> -  cache->saved_regs[1].set_addr (unwound_sp + 4);
>>> -  cache->saved_regs[2].set_addr (unwound_sp + 8);
>>> -  cache->saved_regs[3].set_addr (unwound_sp + 12);
>>> -  cache->saved_regs[ARM_IP_REGNUM].set_addr (unwound_sp + 16);
>>> -  cache->saved_regs[ARM_LR_REGNUM].set_addr (unwound_sp + 20);
>>> -  cache->saved_regs[ARM_PC_REGNUM].set_addr (unwound_sp + 24);
>>> -  cache->saved_regs[ARM_PS_REGNUM].set_addr (unwound_sp + 28);
>>> +  cache->saved_regs[0].set_addr (unwound_sp + sp_r0_offset);
>>> +  cache->saved_regs[1].set_addr (unwound_sp + sp_r0_offset + 4);
>>> +  cache->saved_regs[2].set_addr (unwound_sp + sp_r0_offset + 8);
>>> +  cache->saved_regs[3].set_addr (unwound_sp + sp_r0_offset + 12);
>>> +  cache->saved_regs[ARM_IP_REGNUM].set_addr (unwound_sp + 
>>> sp_r0_offset + 16);
>>> +  cache->saved_regs[ARM_LR_REGNUM].set_addr (unwound_sp + 
>>> sp_r0_offset + 20);
>>> +  cache->saved_regs[ARM_PC_REGNUM].set_addr (unwound_sp + 
>>> sp_r0_offset + 24);
>>> +  cache->saved_regs[ARM_PS_REGNUM].set_addr (unwound_sp + 
>>> sp_r0_offset + 28);
>>>     /* Check EXC_RETURN bit FTYPE if extended stack frame (FPU regs 
>>> stored)
>>>        type used.  */
>>> @@ -3312,41 +3463,43 @@ arm_m_exception_cache (struct frame_info 
>>> *this_frame)
>>>        This register is located at address 0xE000EF34.  */
>>>         /* Extended stack frame type used.  */
>>> -      fpu_regs_stack_offset = unwound_sp + 0x20;
>>> +      fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
>>>         for (i = 0; i < 16; i++)
>>>       {
>>>         cache->saved_regs[ARM_D0_REGNUM + i].set_addr 
>>> (fpu_regs_stack_offset);
>>>         fpu_regs_stack_offset += 4;
>>>       }
>>> -      cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + 0x60);
>>> +      cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + 
>>> sp_r0_offset + 0x60);
>>> +      fpu_regs_stack_offset += 4;
>>> +
>>> +      if (tdep->have_sec_ext && !default_callee_register_stacking)
>>> +    {
>>> +      /* Handle floating-point callee saved registers.  */
>>> +      fpu_regs_stack_offset = 0x90;
>>> +      for (i = 16; i < 32; i++)
>>> +        {
>>> +          cache->saved_regs[ARM_D0_REGNUM + i].set_addr 
>>> (fpu_regs_stack_offset);
>>> +          fpu_regs_stack_offset += 4;
>>> +        }
>>> -      /* Offset 0x64 is reserved.  */
>>> -      arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 0x68);
>>> +      arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 
>>> sp_r0_offset + 0xD0);
>>> +    }
>>> +      else
>>> +    {
>>> +      /* Offset 0x64 is reserved.  */
>>> +      arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 
>>> sp_r0_offset + 0x68);
>>> +    }
>>>       }
>>>     else
>>>       {
>>>         /* Standard stack frame type used.  */
>>> -      arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 0x20);
>>> -    }
>>> -
>>> -  /* Check EXC_RETURN bit S if Secure or Non-secure stack used.  */
>>> -  secure_stack_used = ((lr & (1 << 6)) != 0);
>>> -  if (exc_return && secure_stack_used)
>>> -    {
>>> -      /* ARMv8-M Exception and interrupt handling is not considered 
>>> here.
>>> -     In the ARMv8-M architecture also EXC_RETURN bit S is 
>>> controlling if
>>> -     the Secure or Non-secure stack was used. To separate Secure and
>>> -     Non-secure stacks, processors that are based on the ARMv8-M
>>> -     architecture support 4 stack pointers: MSP_S, PSP_S, MSP_NS, 
>>> PSP_NS.
>>> -     In addition, a stack limit feature is provided using stack limit
>>> -     registers (accessible using MSR and MRS instructions) in 
>>> Privileged
>>> -     level.  */
>>> +      arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 
>>> sp_r0_offset + 0x20);
>>>       }
>>>     /* If bit 9 of the saved xPSR is set, then there is a four-byte
>>>        aligner between the top of the 32-byte stack frame and the
>>>        previous context's stack pointer.  */
>>> -  if (safe_read_memory_integer (unwound_sp + 28, 4, byte_order, &xpsr)
>>> +  if (safe_read_memory_integer (unwound_sp + sp_r0_offset + 28, 4, 
>>> byte_order, &xpsr)
>>>         && (xpsr & (1 << 9)) != 0)
>>>       arm_cache_set_active_sp_value (cache, tdep,
>>>                      arm_cache_get_prev_sp_value (cache, tdep) + 4);
>>> @@ -3384,6 +3537,7 @@ arm_m_exception_prev_register (struct 
>>> frame_info *this_frame,
>>>                      int prev_regnum)
>>>   {
>>>     struct arm_prologue_cache *cache;
>>> +  CORE_ADDR sp_value;
>>>     if (*this_cache == NULL)
>>>       *this_cache = arm_m_exception_cache (this_frame);
>>> @@ -3396,6 +3550,23 @@ arm_m_exception_prev_register (struct 
>>> frame_info *this_frame,
>>>       return frame_unwind_got_constant (this_frame, prev_regnum,
>>>                         arm_cache_get_prev_sp_value (cache, tdep));
>>> +  /* The value might be one of the alternative SP, if so, use the
>>> +     value already constructed.  */
>>> +  if (arm_cache_is_sp_register (cache, tdep, prev_regnum))
>>> +    {
>>> +      sp_value = arm_cache_get_sp_register (cache, tdep, prev_regnum);
>>> +      return frame_unwind_got_constant (this_frame, prev_regnum, 
>>> sp_value);
>>> +    }
>>> +
>>> +  if (prev_regnum == ARM_PC_REGNUM)
>>> +    {
>>> +      CORE_ADDR lr = frame_unwind_register_unsigned (this_frame, 
>>> ARM_LR_REGNUM);
>>> +      struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>> +
>>> +      return frame_unwind_got_constant (this_frame, prev_regnum,
>>> +                    arm_addr_bits_remove (gdbarch, lr));
>>> +    }
>>> +
>>>     return trad_frame_get_prev_register (this_frame, cache->saved_regs,
>>>                          prev_regnum);
>>>   }
>>> @@ -3408,13 +3579,14 @@ arm_m_exception_unwind_sniffer (const struct 
>>> frame_unwind *self,
>>>                   struct frame_info *this_frame,
>>>                   void **this_prologue_cache)
>>>   {
>>> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>>     CORE_ADDR this_pc = get_frame_pc (this_frame);
>>>     /* No need to check is_m; this sniffer is only registered for
>>>        M-profile architectures.  */
>>>     /* Check if exception frame returns to a magic PC value.  */
>>> -  return arm_m_addr_is_magic (this_pc);
>>> +  return arm_m_addr_is_magic (gdbarch, this_pc);
>>>   }
>>>   /* Frame unwinder for M-profile exceptions.  */
>>> @@ -8914,6 +9086,15 @@ arm_show_force_mode (struct ui_file *file, int 
>>> from_tty,
>>>             arm_force_mode_string);
>>>   }
>>> +static void
>>> +arm_show_unwind_secure_frames (struct ui_file *file, int from_tty,
>>> +             struct cmd_list_element *c, const char *value)
>>> +{
>>> +  gdb_printf (file,
>>> +          _("Usage of non-secure to secure exception stack unwinding 
>>> is %s.\n"),
>>> +          arm_unwind_secure_frames ? "on" : "off");
>>> +}
>>> +
>>>   /* If the user changes the register disassembly style used for info
>>>      register and other commands, we have to also switch the style used
>>>      in opcodes for disassembly output.  This function is run in the 
>>> "set
>>> @@ -10397,6 +10578,15 @@ vfp - VFP co-processor."),
>>>               NULL, NULL, arm_show_force_mode,
>>>               &setarmcmdlist, &showarmcmdlist);
>>> +  /* Add a command to stop triggering security exceptions when
>>> +     unwinding exception stacks.  */
>>> +  add_setshow_boolean_cmd ("unwind-secure-frames", no_class, 
>>> &arm_unwind_secure_frames,
>>> +               _("Set usage of non-secure to secure exception stack 
>>> unwinding."),
>>> +               _("Show usage of non-secure to secure exception stack 
>>> unwinding."),
>>> +               _("When on, the debugger can trigger memory access 
>>> traps."),
>>> +               NULL, arm_show_unwind_secure_frames,
>>> +               &setarmcmdlist, &showarmcmdlist);
>>> +
>>>     /* Debugging flag.  */
>>>     add_setshow_boolean_cmd ("arm", class_maintenance, &arm_debug,
>>>                  _("Set ARM debugging."),
>>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>>> index c1e9b09e833..22b07e7b67b 100644
>>> --- a/gdb/doc/gdb.texinfo
>>> +++ b/gdb/doc/gdb.texinfo
>>> @@ -25133,6 +25133,16 @@ of @samp{set arm fallback-mode}.
>>>   @item show arm force-mode
>>>   Show the current forced instruction mode.
>>> +@item set arm unwind-ns-to-s
>>> +This command enables unwinding from Non-secure to Secure mode on
>>> +Cortex-M with Security extension.
>>> +This can trigger security exceptions when unwinding the exception
>>> +stack.
>>> +It is enabled by default.
>>> +
>>> +@item show arm unwind-ns-to-s
>>> +Show whether unwind from Non-secure to Secure mode is enabled.
>>> +
>>>   @item set debug arm
>>>   Toggle whether to display ARM-specific debugging messages from the ARM
>>>   target support subsystem.
>>
>> The documentation should mention "unwind-secure-frames" instead of 
>> unwind-ns-to-s. Other than that, this series is OK.
>>
> 
> Sigh, I touhght about updating the NEWS file, but forgot about 
> gdb.texinfo :-(
> 
> Thanks for catching this!
> 
Just to clarify: I pushed the 5 patches with the above change.

Christophe

> Christophe
> 
>> Thanks!
>> Luis

      reply	other threads:[~2022-04-28  8:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-25  9:59 [PATCH v5 0/5] arm: Add support for multiple stacks on Cortex-M Christophe Lyon
2022-04-25  9:59 ` [PATCH v5 1/5] gdb/arm: Fix prologue analysis to support vpush Christophe Lyon
2022-04-25  9:59 ` [PATCH v5 2/5] gdb/arm: Define MSP and PSP registers for M-Profile Christophe Lyon
2022-04-25  9:59 ` [PATCH v5 3/5] gdb/arm: Introduce arm_cache_init Christophe Lyon
2022-04-25  9:59 ` [PATCH v5 4/5] gdb/arm: Add support for multiple stack pointers on Cortex-M Christophe Lyon
2022-04-25  9:59 ` [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command Christophe Lyon
2022-04-25 11:31   ` Eli Zaretskii
2022-04-27 13:09   ` Luis Machado
2022-04-27 13:11     ` Christophe Lyon
2022-04-28  8:55       ` Christophe Lyon [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=715a5e5d-431a-2c68-6790-7b26ba72ebda@arm.com \
    --to=christophe.lyon@arm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).