* [PATCH v5 0/5] arm: Add support for multiple stacks on Cortex-M
@ 2022-04-25 9:59 Christophe Lyon
2022-04-25 9:59 ` [PATCH v5 1/5] gdb/arm: Fix prologue analysis to support vpush Christophe Lyon
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-25 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: torbjorn.svensson, yvan.roux
This patch series introduces support for the multiple stack pointers
on v8-M architecture (MSP_S, MSP_NS, PSP_S, PSP_NS).
This enables to unwind through Secure vs Non-secure context switches.
Along the way, the first patch adds support to detect vpush
instructions in function prologues, which in particular is used in the
CMSE trampolines.
Changes v4 -> v5:
- rebased on top of recent trunk, which includes:
* fprintf_filtered calls renamed into gdb_printf
* cache is no longer a POD, so we cannot use memset anymore
* merge conflicts with the recent PACBTI patches
- renamed unwind-ns-to-s into unwind-secure-frames
- fixed a bug where we could call arm_cache_get_sp_register (cache,
tdep, prev_regnum) with prev_regnum not being one of the stack
pointers, leading to a GDB crash. (Found by running make check on a
arm-linux host)
Christophe Lyon (5):
gdb/arm: Fix prologue analysis to support vpush
gdb/arm: Define MSP and PSP registers for M-Profile
gdb/arm: Introduce arm_cache_init
gdb/arm: Add support for multiple stack pointers on Cortex-M
gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add
unwind-secure-frames command
gdb/NEWS | 5 +
gdb/arch/arm.c | 6 +
gdb/arch/arm.h | 1 +
gdb/arm-tdep.c | 685 +++++++++++++++++++++++++-----
gdb/arm-tdep.h | 10 +
gdb/doc/gdb.texinfo | 10 +
gdb/features/Makefile | 1 +
gdb/features/arm/arm-m-system.c | 15 +
gdb/features/arm/arm-m-system.xml | 12 +
gdb/features/arm/arm-secext.c | 17 +
gdb/features/arm/arm-secext.xml | 15 +
11 files changed, 678 insertions(+), 99 deletions(-)
create mode 100644 gdb/features/arm/arm-m-system.c
create mode 100644 gdb/features/arm/arm-m-system.xml
create mode 100644 gdb/features/arm/arm-secext.c
create mode 100644 gdb/features/arm/arm-secext.xml
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/5] gdb/arm: Fix prologue analysis to support vpush
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 ` Christophe Lyon
2022-04-25 9:59 ` [PATCH v5 2/5] gdb/arm: Define MSP and PSP registers for M-Profile Christophe Lyon
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-25 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: torbjorn.svensson, yvan.roux
While working on adding support for Non-secure/Secure modes unwinding,
I noticed that the prologue analysis lacked support for vpush, which
is used for instance in the CMSE stub routine.
This patch updates thumb_analyze_prologue accordingly, adding support
for vpush of D-registers.
Signed-off-by: Christophe Lyon <christophe.lyon@foss.st.com>
Signed-off-by: Christophe Lyon <christophe.lyon@arm.com>
---
gdb/arm-tdep.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index cc7773914d7..66e26e6e212 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -902,6 +902,35 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
regs[bits (insn, 0, 3)] = addr;
}
+ /* vstmdb Rn{!}, { D-registers } (aka vpush). */
+ else if ((insn & 0xff20) == 0xed20
+ && (inst2 & 0x0f00) == 0x0b00
+ && pv_is_register (regs[bits (insn, 0, 3)], ARM_SP_REGNUM))
+ {
+ /* Address SP points to. */
+ pv_t addr = regs[bits (insn, 0, 3)];
+
+ /* Number of registers saved. */
+ unsigned int number = bits (inst2, 0, 7) >> 1;
+
+ /* First register to save. */
+ int vd = bits (inst2, 12, 15) | (bits (insn, 6, 6) << 4);
+
+ if (stack.store_would_trash (addr))
+ break;
+
+ /* Calculate offsets of saved registers. */
+ for (; number > 0; number--)
+ {
+ addr = pv_add_constant (addr, -8);
+ stack.store (addr, 8, pv_register (ARM_D0_REGNUM
+ + vd + number, 0));
+ }
+
+ /* Writeback SP to account for the saved registers. */
+ regs[bits (insn, 0, 3)] = addr;
+ }
+
else if ((insn & 0xff50) == 0xe940 /* strd Rt, Rt2,
[Rn, #+/-imm]{!} */
&& pv_is_register (regs[bits (insn, 0, 3)], ARM_SP_REGNUM))
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 2/5] gdb/arm: Define MSP and PSP registers for M-Profile
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 ` Christophe Lyon
2022-04-25 9:59 ` [PATCH v5 3/5] gdb/arm: Introduce arm_cache_init Christophe Lyon
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-25 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: torbjorn.svensson, yvan.roux
This patch removes the hardcoded access to PSP in
arm_m_exception_cache() and relies on the definition with the XML
descriptions.
Signed-off-by: Christophe Lyon <christophe.lyon@foss.st.com>
Signed-off-by: Christophe Lyon <christophe.lyon@arm.com>
---
gdb/arch/arm.c | 6 +++
gdb/arch/arm.h | 1 +
gdb/arm-tdep.c | 80 ++++++++++++++++++++-----------
gdb/arm-tdep.h | 3 ++
gdb/features/Makefile | 1 +
gdb/features/arm/arm-m-system.c | 15 ++++++
gdb/features/arm/arm-m-system.xml | 12 +++++
7 files changed, 89 insertions(+), 29 deletions(-)
create mode 100644 gdb/features/arm/arm-m-system.c
create mode 100644 gdb/features/arm/arm-m-system.xml
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index 126e46a950a..bc6e5ce3f09 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -28,6 +28,7 @@
#include "../features/arm/arm-m-profile.c"
#include "../features/arm/arm-m-profile-with-fpa.c"
#include "../features/arm/arm-m-profile-mve.c"
+#include "../features/arm/arm-m-system.c"
/* See arm.h. */
@@ -446,6 +447,11 @@ arm_create_mprofile_target_description (arm_m_profile_type m_type)
regnum = create_feature_arm_arm_m_profile_mve (tdesc, regnum);
break;
+ case ARM_M_TYPE_SYSTEM:
+ regnum = create_feature_arm_arm_m_profile (tdesc, regnum);
+ regnum = create_feature_arm_arm_m_system (tdesc, regnum);
+ break;
+
default:
error (_("Invalid Arm M type: %d"), m_type);
}
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index 755391fe6fe..0728bea1501 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -105,6 +105,7 @@ enum arm_m_profile_type {
ARM_M_TYPE_VFP_D16,
ARM_M_TYPE_WITH_FPA,
ARM_M_TYPE_MVE,
+ ARM_M_TYPE_SYSTEM,
ARM_M_TYPE_INVALID
};
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 66e26e6e212..6b4b00993fa 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3076,9 +3076,9 @@ 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);
struct arm_prologue_cache *cache;
CORE_ADDR lr;
- CORE_ADDR sp;
CORE_ADDR unwound_sp;
LONGEST xpsr;
uint32_t exc_return;
@@ -3094,7 +3094,6 @@ 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);
/* Check EXC_RETURN indicator bits. */
exc_return = (((lr >> 28) & 0xf) == 0xf);
@@ -3103,37 +3102,13 @@ arm_m_exception_cache (struct frame_info *this_frame)
process_stack_used = ((lr & (1 << 2)) != 0);
if (exc_return && process_stack_used)
{
- /* Thread (process) stack used.
- Potentially this could be other register defined by target, but PSP
- can be considered a standard name for the "Process Stack Pointer".
- To be fully aware of system registers like MSP and PSP, these could
- be added to a separate XML arm-m-system-profile that is valid for
- ARMv6-M and ARMv7-M architectures. Also to be able to debug eg a
- corefile off-line, then these registers must be defined by GDB,
- and also be included in the corefile regsets. */
-
- int psp_regnum = user_reg_map_name_to_regnum (gdbarch, "psp", -1);
- if (psp_regnum == -1)
- {
- /* Thread (process) stack could not be fetched,
- give warning and exit. */
-
- warning (_("no PSP thread stack unwinding supported."));
-
- /* Terminate any further stack unwinding by refer to self. */
- cache->prev_sp = sp;
- return cache;
- }
- else
- {
- /* Thread (process) stack used, use PSP as SP. */
- unwound_sp = get_frame_register_unsigned (this_frame, psp_regnum);
- }
+ /* Thread (process) stack used, use PSP as SP. */
+ unwound_sp = get_frame_register_unsigned (this_frame, tdep->m_profile_psp_regnum);
}
else
{
/* Main stack used, use MSP as SP. */
- unwound_sp = sp;
+ unwound_sp = get_frame_register_unsigned (this_frame, tdep->m_profile_msp_regnum);
}
/* The hardware saves eight 32-bit words, comprising xPSR,
@@ -9188,6 +9163,10 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE
+ ARM_VFP2_REGS_SIZE
+ ARM_INT_REGISTER_SIZE, tdesc);
+
+ /* M-profile system (stack pointers). */
+ tdesc = arm_read_mprofile_description (ARM_M_TYPE_SYSTEM);
+ register_remote_g_packet_guess (gdbarch, 2 * ARM_INT_REGISTER_SIZE, tdesc);
}
/* Otherwise we don't have a useful guess. */
@@ -9260,6 +9239,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
bool have_pacbti = false;
int mve_vpr_regnum = -1;
int register_count = ARM_NUM_REGS;
+ bool have_m_profile_msp = false;
+ int m_profile_msp_regnum = -1;
+ int m_profile_psp_regnum = -1;
/* If we have an object to base this architecture on, try to determine
its ABI. */
@@ -9490,6 +9472,35 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (!valid_p)
return NULL;
+ if (is_m)
+ {
+ feature = tdesc_find_feature (tdesc,
+ "org.gnu.gdb.arm.m-system");
+ if (feature != nullptr)
+ {
+ /* MSP */
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ register_count, "msp");
+ if (!valid_p)
+ {
+ warning (_("M-profile m-system feature is missing required register msp."));
+ return nullptr;
+ }
+ have_m_profile_msp = true;
+ m_profile_msp_regnum = register_count++;
+
+ /* PSP */
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ register_count, "psp");
+ if (!valid_p)
+ {
+ warning (_("M-profile m-system feature is missing required register psp."));
+ return nullptr;
+ }
+ m_profile_psp_regnum = register_count++;
+ }
+ }
+
feature = tdesc_find_feature (tdesc,
"org.gnu.gdb.arm.fpa");
if (feature != NULL)
@@ -9713,6 +9724,13 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* Adjust the PACBTI feature settings. */
tdep->have_pacbti = have_pacbti;
+ /* Adjust the M-profile stack pointers settings. */
+ if (have_m_profile_msp)
+ {
+ tdep->m_profile_msp_regnum = m_profile_msp_regnum;
+ tdep->m_profile_psp_regnum = m_profile_psp_regnum;
+ }
+
arm_register_g_packet_guesses (gdbarch);
/* Breakpoints. */
@@ -9998,6 +10016,10 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
tdep->mve_pseudo_base);
gdb_printf (file, _("arm_dump_tdep: mve_pseudo_count = %i\n"),
tdep->mve_pseudo_count);
+ gdb_printf (file, _("arm_dump_tdep: m_profile_msp_regnum = %i\n"),
+ tdep->m_profile_msp_regnum);
+ gdb_printf (file, _("arm_dump_tdep: m_profile_psp_regnum = %i\n"),
+ tdep->m_profile_psp_regnum);
gdb_printf (file, _("arm_dump_tdep: Lowest pc = 0x%lx\n"),
(unsigned long) tdep->lowest_pc);
gdb_printf (file, _("arm_dump_tdep: have_pacbti = %s\n"),
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index ae32fffcabf..40170673fde 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -125,6 +125,9 @@ struct arm_gdbarch_tdep : gdbarch_tdep
register. */
int pacbti_pseudo_count = 0; /* Total number of PACBTI pseudo registers. */
+ int m_profile_msp_regnum = 0; /* M-profile MSP register number. */
+ int m_profile_psp_regnum = 0; /* M-profile PSP register number. */
+
bool is_m = false; /* Does the target follow the "M" profile. */
CORE_ADDR lowest_pc = 0; /* Lowest address at which instructions
will appear. */
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index a2bb2a5922f..737d9cbd3db 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -206,6 +206,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
arm/arm-fpa.xml \
arm/arm-m-profile.xml \
arm/arm-m-profile-mve.xml \
+ arm/arm-m-system.xml \
arm/arm-m-profile-with-fpa.xml \
arm/arm-vfpv2.xml \
arm/arm-vfpv3.xml \
diff --git a/gdb/features/arm/arm-m-system.c b/gdb/features/arm/arm-m-system.c
new file mode 100644
index 00000000000..3fb20a57198
--- /dev/null
+++ b/gdb/features/arm/arm-m-system.c
@@ -0,0 +1,15 @@
+/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
+ Original: arm-m-system.xml */
+
+#include "gdbsupport/tdesc.h"
+
+static int
+create_feature_arm_arm_m_system (struct target_desc *result, long regnum)
+{
+ struct tdesc_feature *feature;
+
+ feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-system");
+ tdesc_create_reg (feature, "msp", regnum++, 1, NULL, 32, "data_ptr");
+ tdesc_create_reg (feature, "psp", regnum++, 1, NULL, 32, "data_ptr");
+ return regnum;
+}
diff --git a/gdb/features/arm/arm-m-system.xml b/gdb/features/arm/arm-m-system.xml
new file mode 100644
index 00000000000..eb167adb21a
--- /dev/null
+++ b/gdb/features/arm/arm-m-system.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2022 Free Software Foundation, Inc.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.arm.m-system">
+ <reg name="msp" bitsize="32" type="data_ptr"/>
+ <reg name="psp" bitsize="32" type="data_ptr"/>
+</feature>
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 3/5] gdb/arm: Introduce arm_cache_init
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 ` 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
4 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-25 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: torbjorn.svensson, yvan.roux
This patch is a preparation for the rest of the series and adds two
arm_cache_init helper functions. It updates every place that updates
cache->saved_regs to call the helper instead.
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/arm-tdep.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 6b4b00993fa..f393a3e4ad6 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -292,8 +292,29 @@ struct arm_prologue_cache
/* Saved register offsets. */
trad_frame_saved_reg *saved_regs;
+
+ arm_prologue_cache() = default;
};
+/* Initialize CACHE fields for which zero is not adequate (CACHE is
+ expected to have been ZALLOC'ed before calling this function). */
+
+static void
+arm_cache_init (struct arm_prologue_cache *cache, struct gdbarch *gdbarch)
+{
+ cache->saved_regs = trad_frame_alloc_saved_regs (gdbarch);
+}
+
+/* Similar to the previous function, but extracts GDBARCH from FRAME. */
+
+static void
+arm_cache_init (struct arm_prologue_cache *cache, struct frame_info *frame)
+{
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+
+ arm_cache_init (cache, gdbarch);
+}
+
namespace {
/* Abstract class to read ARM instructions from memory. */
@@ -1981,7 +2002,7 @@ arm_make_prologue_cache (struct frame_info *this_frame)
CORE_ADDR unwound_fp;
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
- cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+ arm_cache_init (cache, this_frame);
arm_scan_prologue (this_frame, cache);
@@ -2454,7 +2475,7 @@ arm_exidx_fill_cache (struct frame_info *this_frame, gdb_byte *entry)
struct arm_prologue_cache *cache;
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
- cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+ arm_cache_init (cache, this_frame);
for (;;)
{
@@ -2848,7 +2869,7 @@ arm_make_epilogue_frame_cache (struct frame_info *this_frame)
int reg;
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
- cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+ arm_cache_init (cache, this_frame);
/* Still rely on the offset calculated from prologue. */
arm_scan_prologue (this_frame, cache);
@@ -3009,7 +3030,7 @@ arm_make_stub_cache (struct frame_info *this_frame)
struct arm_prologue_cache *cache;
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
- cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+ arm_cache_init (cache, this_frame);
cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
@@ -3087,7 +3108,7 @@ arm_m_exception_cache (struct frame_info *this_frame)
uint32_t secure_stack_used;
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
- cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+ arm_cache_init (cache, this_frame);
/* ARMv7-M Architecture Reference "B1.5.6 Exception entry behavior"
describes which bits in LR that define which stack was used prior
@@ -13851,7 +13872,7 @@ arm_analyze_prologue_test ()
test_arm_instruction_reader mem_reader (insns);
arm_prologue_cache cache;
- cache.saved_regs = trad_frame_alloc_saved_regs (gdbarch);
+ arm_cache_init (&cache, gdbarch);
arm_analyze_prologue (gdbarch, 0, sizeof (insns) - 1, &cache, mem_reader);
}
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 4/5] gdb/arm: Add support for multiple stack pointers on Cortex-M
2022-04-25 9:59 [PATCH v5 0/5] arm: Add support for multiple stacks on Cortex-M Christophe Lyon
` (2 preceding siblings ...)
2022-04-25 9:59 ` [PATCH v5 3/5] gdb/arm: Introduce arm_cache_init Christophe Lyon
@ 2022-04-25 9:59 ` 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
4 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-25 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: torbjorn.svensson, yvan.roux
Armv8-M architecture with Security extension features four stack pointers
to handle Secure and Non-secure modes.
This patch adds support to switch between them as needed during
unwinding, and replaces all updates of cache->prev_sp with calls to
arm_cache_set_prev_sp.
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/arm-tdep.c | 261 +++++++++++++++++++++++++++++---
gdb/arm-tdep.h | 11 +-
gdb/features/arm/arm-secext.c | 17 +++
gdb/features/arm/arm-secext.xml | 15 ++
4 files changed, 284 insertions(+), 20 deletions(-)
create mode 100644 gdb/features/arm/arm-secext.c
create mode 100644 gdb/features/arm/arm-secext.xml
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index f393a3e4ad6..7a6c1e49125 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -276,7 +276,18 @@ struct arm_prologue_cache
/* The stack pointer at the time this frame was created; i.e. the
caller's stack pointer when this function was called. It is used
to identify this frame. */
- CORE_ADDR prev_sp;
+ CORE_ADDR sp;
+
+ /* Additional stack pointers used by M-profile with Security extension. */
+ /* Use msp_s / psp_s to hold the values of msp / psp when there is
+ no Security extension. */
+ CORE_ADDR msp_s;
+ CORE_ADDR msp_ns;
+ CORE_ADDR psp_s;
+ CORE_ADDR psp_ns;
+
+ /* Active stack pointer. */
+ int active_sp_regnum;
/* The frame base for this frame is just prev_sp - frame size.
FRAMESIZE is the distance from the frame pointer to the
@@ -296,12 +307,28 @@ struct arm_prologue_cache
arm_prologue_cache() = default;
};
+/* Initialize stack pointers, and flag the active one. */
+
+static inline void
+arm_cache_init_sp (int regnum, CORE_ADDR* member,
+ struct arm_prologue_cache *cache,
+ struct frame_info *frame)
+{
+ CORE_ADDR val = get_frame_register_unsigned (frame, regnum);
+ if (val == cache->sp)
+ cache->active_sp_regnum = regnum;
+
+ *member = val;
+}
+
/* Initialize CACHE fields for which zero is not adequate (CACHE is
expected to have been ZALLOC'ed before calling this function). */
static void
arm_cache_init (struct arm_prologue_cache *cache, struct gdbarch *gdbarch)
{
+ cache->active_sp_regnum = ARM_SP_REGNUM;
+
cache->saved_regs = trad_frame_alloc_saved_regs (gdbarch);
}
@@ -311,8 +338,109 @@ static void
arm_cache_init (struct arm_prologue_cache *cache, struct frame_info *frame)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
+ arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch);
arm_cache_init (cache, gdbarch);
+
+ if (tdep->have_sec_ext)
+ {
+ arm_cache_init_sp (tdep->m_profile_msp_s_regnum, &cache->msp_s, cache, frame);
+ arm_cache_init_sp (tdep->m_profile_psp_s_regnum, &cache->psp_s, cache, frame);
+ arm_cache_init_sp (tdep->m_profile_msp_ns_regnum, &cache->msp_ns, cache, frame);
+ arm_cache_init_sp (tdep->m_profile_psp_ns_regnum, &cache->psp_ns, cache, frame);
+
+ /* Use MSP_S as default stack pointer. */
+ if (cache->active_sp_regnum == ARM_SP_REGNUM)
+ cache->active_sp_regnum = tdep->m_profile_msp_s_regnum;
+ }
+ else if (tdep->is_m)
+ {
+ arm_cache_init_sp (tdep->m_profile_msp_regnum, &cache->msp_s, cache, frame);
+ arm_cache_init_sp (tdep->m_profile_psp_regnum, &cache->psp_s, cache, frame);
+ }
+ else
+ arm_cache_init_sp (ARM_SP_REGNUM, &cache->msp_s, cache, frame);
+}
+
+/* Return the requested stack pointer value (in REGNUM), taking into
+ account whether we have a Security extension or an M-profile
+ CPU. */
+
+static CORE_ADDR
+arm_cache_get_sp_register (struct arm_prologue_cache *cache,
+ arm_gdbarch_tdep *tdep, int regnum)
+{
+ if (regnum == ARM_SP_REGNUM)
+ return cache->sp;
+
+ if (tdep->have_sec_ext)
+ {
+ if (regnum == tdep->m_profile_msp_s_regnum)
+ return cache->msp_s;
+ if (regnum == tdep->m_profile_msp_ns_regnum)
+ return cache->msp_ns;
+ if (regnum == tdep->m_profile_psp_s_regnum)
+ return cache->psp_s;
+ if (regnum == tdep->m_profile_psp_ns_regnum)
+ return cache->psp_ns;
+ }
+ else if (tdep->is_m)
+ {
+ if (regnum == tdep->m_profile_msp_regnum)
+ return cache->msp_s;
+ if (regnum == tdep->m_profile_psp_regnum)
+ return cache->psp_s;
+ }
+
+ gdb_assert_not_reached ("Invalid SP selection");
+}
+
+/* Return the previous stack address, depending on which SP register
+ is active. */
+
+static CORE_ADDR
+arm_cache_get_prev_sp_value (struct arm_prologue_cache *cache, arm_gdbarch_tdep *tdep)
+{
+ CORE_ADDR val = arm_cache_get_sp_register (cache, tdep, cache->active_sp_regnum);
+ return val;
+}
+
+/* Set the active stack pointer to VAL. */
+
+static void
+arm_cache_set_active_sp_value (struct arm_prologue_cache *cache,
+ arm_gdbarch_tdep *tdep, CORE_ADDR val)
+{
+ if (cache->active_sp_regnum == ARM_SP_REGNUM)
+ {
+ cache->sp = val;
+ return;
+ }
+
+ if (tdep->have_sec_ext)
+ {
+ if (cache->active_sp_regnum == tdep->m_profile_msp_s_regnum)
+ cache->msp_s = val;
+ else if (cache->active_sp_regnum == tdep->m_profile_msp_ns_regnum)
+ cache->msp_ns = val;
+ else if (cache->active_sp_regnum == tdep->m_profile_psp_s_regnum)
+ cache->psp_s = val;
+ else if (cache->active_sp_regnum == tdep->m_profile_psp_ns_regnum)
+ cache->psp_ns = val;
+
+ return;
+ }
+ else if (tdep->is_m)
+ {
+ if (cache->active_sp_regnum == tdep->m_profile_msp_regnum)
+ cache->msp_s = val;
+ else if (cache->active_sp_regnum == tdep->m_profile_psp_regnum)
+ cache->psp_s = val;
+
+ return;
+ }
+
+ gdb_assert_not_reached ("Invalid SP selection");
}
namespace {
@@ -2010,14 +2138,17 @@ arm_make_prologue_cache (struct frame_info *this_frame)
if (unwound_fp == 0)
return cache;
- cache->prev_sp = unwound_fp + cache->framesize;
+ arm_gdbarch_tdep *tdep =
+ (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+
+ arm_cache_set_active_sp_value (cache, tdep, unwound_fp + cache->framesize);
/* Calculate actual addresses of saved registers using offsets
determined by arm_scan_prologue. */
for (reg = 0; reg < gdbarch_num_regs (get_frame_arch (this_frame)); reg++)
if (cache->saved_regs[reg].is_addr ())
cache->saved_regs[reg].set_addr (cache->saved_regs[reg].addr ()
- + cache->prev_sp);
+ + arm_cache_get_prev_sp_value (cache, tdep));
return cache;
}
@@ -2043,7 +2174,7 @@ arm_prologue_unwind_stop_reason (struct frame_info *this_frame,
return UNWIND_OUTERMOST;
/* If we've hit a wall, stop. */
- if (cache->prev_sp == 0)
+ if (arm_cache_get_prev_sp_value (cache, tdep) == 0)
return UNWIND_OUTERMOST;
return UNWIND_NO_REASON;
@@ -2065,6 +2196,9 @@ arm_prologue_this_id (struct frame_info *this_frame,
*this_cache = arm_make_prologue_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+
/* Use function start address as part of the frame ID. If we cannot
identify the start address (due to missing symbol information),
fall back to just using the current PC. */
@@ -2073,7 +2207,7 @@ arm_prologue_this_id (struct frame_info *this_frame,
if (!func)
func = pc;
- id = frame_id_build (cache->prev_sp, func);
+ id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), func);
*this_id = id;
}
@@ -2114,7 +2248,8 @@ arm_prologue_prev_register (struct frame_info *this_frame,
identified by the next frame's stack pointer at the time of the call.
The value was already reconstructed into PREV_SP. */
if (prev_regnum == ARM_SP_REGNUM)
- return frame_unwind_got_constant (this_frame, prev_regnum, cache->prev_sp);
+ return frame_unwind_got_constant (this_frame, prev_regnum,
+ arm_cache_get_prev_sp_value (cache, tdep));
/* 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,
@@ -2752,7 +2887,9 @@ arm_exidx_fill_cache (struct frame_info *this_frame, gdb_byte *entry)
= vsp - get_frame_register_unsigned (this_frame, cache->framereg);
/* We already got the previous SP. */
- cache->prev_sp = vsp;
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ arm_cache_set_active_sp_value (cache, tdep, vsp);
return cache;
}
@@ -2875,14 +3012,18 @@ arm_make_epilogue_frame_cache (struct frame_info *this_frame)
arm_scan_prologue (this_frame, cache);
/* Since we are in epilogue, the SP has been restored. */
- cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ arm_cache_set_active_sp_value (cache, tdep,
+ get_frame_register_unsigned (this_frame,
+ ARM_SP_REGNUM));
/* Calculate actual addresses of saved registers using offsets
determined by arm_scan_prologue. */
for (reg = 0; reg < gdbarch_num_regs (get_frame_arch (this_frame)); reg++)
if (cache->saved_regs[reg].is_addr ())
cache->saved_regs[reg].set_addr (cache->saved_regs[reg].addr ()
- + cache->prev_sp);
+ + arm_cache_get_prev_sp_value (cache, tdep));
return cache;
}
@@ -2910,7 +3051,9 @@ arm_epilogue_frame_this_id (struct frame_info *this_frame,
if (func == 0)
func = pc;
- (*this_id) = frame_id_build (cache->prev_sp, pc);
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep), pc);
}
/* Implementation of function hook 'prev_register' in
@@ -3032,7 +3175,11 @@ arm_make_stub_cache (struct frame_info *this_frame)
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
arm_cache_init (cache, this_frame);
- cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ arm_cache_set_active_sp_value (cache, tdep,
+ get_frame_register_unsigned (this_frame,
+ ARM_SP_REGNUM));
return cache;
}
@@ -3050,7 +3197,10 @@ arm_stub_this_id (struct frame_info *this_frame,
*this_cache = arm_make_stub_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
- *this_id = frame_id_build (cache->prev_sp, get_frame_pc (this_frame));
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep),
+ get_frame_pc (this_frame));
}
static int
@@ -3171,12 +3321,12 @@ arm_m_exception_cache (struct frame_info *this_frame)
cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + 0x60);
/* Offset 0x64 is reserved. */
- cache->prev_sp = unwound_sp + 0x68;
+ arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 0x68);
}
else
{
/* Standard stack frame type used. */
- cache->prev_sp = unwound_sp + 0x20;
+ arm_cache_set_active_sp_value (cache, tdep, unwound_sp + 0x20);
}
/* Check EXC_RETURN bit S if Secure or Non-secure stack used. */
@@ -3198,7 +3348,8 @@ arm_m_exception_cache (struct frame_info *this_frame)
previous context's stack pointer. */
if (safe_read_memory_integer (unwound_sp + 28, 4, byte_order, &xpsr)
&& (xpsr & (1 << 9)) != 0)
- cache->prev_sp += 4;
+ arm_cache_set_active_sp_value (cache, tdep,
+ arm_cache_get_prev_sp_value (cache, tdep) + 4);
return cache;
}
@@ -3218,7 +3369,9 @@ arm_m_exception_this_id (struct frame_info *this_frame,
cache = (struct arm_prologue_cache *) *this_cache;
/* Our frame ID for a stub frame is the current SP and LR. */
- *this_id = frame_id_build (cache->prev_sp,
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ *this_id = frame_id_build (arm_cache_get_prev_sp_value (cache, tdep),
get_frame_pc (this_frame));
}
@@ -3237,9 +3390,11 @@ arm_m_exception_prev_register (struct frame_info *this_frame,
cache = (struct arm_prologue_cache *) *this_cache;
/* The value was already reconstructed into PREV_SP. */
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
if (prev_regnum == ARM_SP_REGNUM)
return frame_unwind_got_constant (this_frame, prev_regnum,
- cache->prev_sp);
+ arm_cache_get_prev_sp_value (cache, tdep));
return trad_frame_get_prev_register (this_frame, cache->saved_regs,
prev_regnum);
@@ -3284,7 +3439,9 @@ arm_normal_frame_base (struct frame_info *this_frame, void **this_cache)
*this_cache = arm_make_prologue_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
- return cache->prev_sp - cache->framesize;
+ arm_gdbarch_tdep *tdep
+ = (arm_gdbarch_tdep *) gdbarch_tdep (get_frame_arch (this_frame));
+ return arm_cache_get_prev_sp_value (cache, tdep) - cache->framesize;
}
struct frame_base arm_normal_base = {
@@ -9249,6 +9406,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdesc_arch_data_up tdesc_data;
int i;
bool is_m = false;
+ bool have_sec_ext = false;
int vfp_register_count = 0;
bool have_s_pseudos = false, have_q_pseudos = false;
bool have_wmmx_registers = false;
@@ -9263,6 +9421,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
bool have_m_profile_msp = false;
int m_profile_msp_regnum = -1;
int m_profile_psp_regnum = -1;
+ int m_profile_msp_ns_regnum = -1;
+ int m_profile_psp_ns_regnum = -1;
+ int m_profile_msp_s_regnum = -1;
+ int m_profile_psp_s_regnum = -1;
/* If we have an object to base this architecture on, try to determine
its ABI. */
@@ -9679,6 +9841,56 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
keys. */
have_pacbti = true;
}
+
+ /* Do we have the Security extension? */
+ feature = tdesc_find_feature (tdesc,
+ "org.gnu.gdb.arm.secext");
+ if (feature != nullptr)
+ {
+ /* Secure/Non-secure stack pointers. */
+ /* MSP_NS */
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ register_count, "msp_ns");
+ if (!valid_p)
+ {
+ warning (_("M-profile secext feature is missing required register msp_ns."));
+ return nullptr;
+ }
+ m_profile_msp_ns_regnum = register_count++;
+
+ /* PSP_NS */
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ register_count, "psp_ns");
+ if (!valid_p)
+ {
+ warning (_("M-profile secext feature is missing required register psp_ns."));
+ return nullptr;
+ }
+ m_profile_psp_ns_regnum = register_count++;
+
+ /* MSP_S */
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ register_count, "msp_s");
+ if (!valid_p)
+ {
+ warning (_("M-profile secext feature is missing required register msp_s."));
+ return nullptr;
+ }
+ m_profile_msp_s_regnum = register_count++;
+
+ /* PSP_S */
+ valid_p &= tdesc_numbered_register (feature, tdesc_data.get (),
+ register_count, "psp_s");
+ if (!valid_p)
+ {
+ warning (_("M-profile secext feature is missing required register psp_s."));
+ return nullptr;
+ }
+ m_profile_psp_s_regnum = register_count++;
+
+ have_sec_ext = true;
+ }
+
}
}
@@ -9725,6 +9937,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdep->arm_abi = arm_abi;
tdep->fp_model = fp_model;
tdep->is_m = is_m;
+ tdep->have_sec_ext = have_sec_ext;
tdep->have_fpa_registers = have_fpa_registers;
tdep->have_wmmx_registers = have_wmmx_registers;
gdb_assert (vfp_register_count == 0
@@ -9750,6 +9963,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
{
tdep->m_profile_msp_regnum = m_profile_msp_regnum;
tdep->m_profile_psp_regnum = m_profile_psp_regnum;
+ tdep->m_profile_msp_ns_regnum = m_profile_msp_ns_regnum;
+ tdep->m_profile_psp_ns_regnum = m_profile_psp_ns_regnum;
+ tdep->m_profile_msp_s_regnum = m_profile_msp_s_regnum;
+ tdep->m_profile_psp_s_regnum = m_profile_psp_s_regnum;
}
arm_register_g_packet_guesses (gdbarch);
@@ -10041,6 +10258,14 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
tdep->m_profile_msp_regnum);
gdb_printf (file, _("arm_dump_tdep: m_profile_psp_regnum = %i\n"),
tdep->m_profile_psp_regnum);
+ gdb_printf (file, _("arm_dump_tdep: m_profile_msp_ns_regnum = %i\n"),
+ tdep->m_profile_msp_ns_regnum);
+ gdb_printf (file, _("arm_dump_tdep: m_profile_psp_ns_regnum = %i\n"),
+ tdep->m_profile_psp_ns_regnum);
+ gdb_printf (file, _("arm_dump_tdep: m_profile_msp_s_regnum = %i\n"),
+ tdep->m_profile_msp_s_regnum);
+ gdb_printf (file, _("arm_dump_tdep: m_profile_psp_s_regnum = %i\n"),
+ tdep->m_profile_psp_s_regnum);
gdb_printf (file, _("arm_dump_tdep: Lowest pc = 0x%lx\n"),
(unsigned long) tdep->lowest_pc);
gdb_printf (file, _("arm_dump_tdep: have_pacbti = %s\n"),
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 40170673fde..864406e98d2 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -125,10 +125,17 @@ struct arm_gdbarch_tdep : gdbarch_tdep
register. */
int pacbti_pseudo_count = 0; /* Total number of PACBTI pseudo registers. */
- int m_profile_msp_regnum = 0; /* M-profile MSP register number. */
- int m_profile_psp_regnum = 0; /* M-profile PSP register number. */
+ int m_profile_msp_regnum = ARM_SP_REGNUM; /* M-profile MSP register number. */
+ int m_profile_psp_regnum = ARM_SP_REGNUM; /* M-profile PSP register number. */
+
+ /* Secure and Non-secure stack pointers with security extension. */
+ int m_profile_msp_ns_regnum = ARM_SP_REGNUM; /* M-profile MSP_NS register number. */
+ int m_profile_psp_ns_regnum = ARM_SP_REGNUM; /* M-profile PSP_NS register number. */
+ int m_profile_msp_s_regnum = ARM_SP_REGNUM; /* M-profile MSP_S register number. */
+ int m_profile_psp_s_regnum = ARM_SP_REGNUM; /* M-profile PSP_S register number. */
bool is_m = false; /* Does the target follow the "M" profile. */
+ bool have_sec_ext = false; /* Do we have security extensions? */
CORE_ADDR lowest_pc = 0; /* Lowest address at which instructions
will appear. */
diff --git a/gdb/features/arm/arm-secext.c b/gdb/features/arm/arm-secext.c
new file mode 100644
index 00000000000..39ef4afb05f
--- /dev/null
+++ b/gdb/features/arm/arm-secext.c
@@ -0,0 +1,17 @@
+/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
+ Original: arm-secext.xml */
+
+#include "gdbsupport/tdesc.h"
+
+static int
+create_feature_arm_arm_m_system (struct target_desc *result, long regnum)
+{
+ struct tdesc_feature *feature;
+
+ feature = tdesc_create_feature (result, "org.gnu.gdb.arm.secext");
+ tdesc_create_reg (feature, "msp_ns", regnum++, 1, NULL, 32, "data_ptr");
+ tdesc_create_reg (feature, "psp_ns", regnum++, 1, NULL, 32, "data_ptr");
+ tdesc_create_reg (feature, "msp_s", regnum++, 1, NULL, 32, "data_ptr");
+ tdesc_create_reg (feature, "psp_s", regnum++, 1, NULL, 32, "data_ptr");
+ return regnum;
+}
diff --git a/gdb/features/arm/arm-secext.xml b/gdb/features/arm/arm-secext.xml
new file mode 100644
index 00000000000..52b2c20a982
--- /dev/null
+++ b/gdb/features/arm/arm-secext.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2022 Free Software Foundation, Inc.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.arm.secext">
+ <!-- We have 4 stack pointers with the security extension. -->
+ <reg name="msp_ns" bitsize="32" type="data_ptr"/>
+ <reg name="psp_ns" bitsize="32" type="data_ptr"/>
+ <reg name="msp_s" bitsize="32" type="data_ptr"/>
+ <reg name="psp_s" bitsize="32" type="data_ptr"/>
+</feature>
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command
2022-04-25 9:59 [PATCH v5 0/5] arm: Add support for multiple stacks on Cortex-M Christophe Lyon
` (3 preceding siblings ...)
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 ` Christophe Lyon
2022-04-25 11:31 ` Eli Zaretskii
2022-04-27 13:09 ` Luis Machado
4 siblings, 2 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-25 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: torbjorn.svensson, yvan.roux
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.
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command
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
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2022-04-25 11:31 UTC (permalink / raw)
To: Christophe Lyon; +Cc: gdb-patches, torbjorn.svensson
> Date: Mon, 25 Apr 2022 11:59:42 +0200
> From: Christophe Lyon via Gdb-patches <gdb-patches@sourceware.org>
> Cc: torbjorn.svensson@st.com
>
> gdb/NEWS | 5 +
> gdb/arm-tdep.c | 314 +++++++++++++++++++++++++++++++++++---------
> gdb/doc/gdb.texinfo | 10 ++
> 3 files changed, 267 insertions(+), 62 deletions(-)
OK for the documentation parts, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command
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
1 sibling, 1 reply; 10+ messages in thread
From: Luis Machado @ 2022-04-27 13:09 UTC (permalink / raw)
To: Christophe Lyon, gdb-patches; +Cc: torbjorn.svensson
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.
Thanks!
Luis
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command
2022-04-27 13:09 ` Luis Machado
@ 2022-04-27 13:11 ` Christophe Lyon
2022-04-28 8:55 ` Christophe Lyon
0 siblings, 1 reply; 10+ messages in thread
From: Christophe Lyon @ 2022-04-27 13:11 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: torbjorn.svensson
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!
Christophe
> Thanks!
> Luis
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] gdb/arm: Extend arm_m_addr_is_magic to support FNC_RETURN, add unwind-secure-frames command
2022-04-27 13:11 ` Christophe Lyon
@ 2022-04-28 8:55 ` Christophe Lyon
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2022-04-28 8:55 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-04-28 8:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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).