RE: [PATCH v4 10/11] gdb, gdbarch: Introduce gdbarch method to get the shadow stack pointer.

Schimpe, Christina
2025-06-23T18:01:09Z

Hi Luis,

> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Thursday, June 19, 2025 11:26 AM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: thiago.bauermann@linaro.org; eliz@gnu.org
> Subject: Re: [PATCH v4 10/11] gdb, gdbarch: Introduce gdbarch method to get
> the shadow stack pointer.
>
> On 6/17/25 13:11, Christina Schimpe wrote:
> > This patch is required by the following commit.
>
> What particular entry in the series uses this? The displaced stepping one?
>
> If so, we should make it more explicit.
>
> >
> > Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > ---
> > gdb/arch-utils.c | 10 ++++++++++
> > gdb/arch-utils.h | 5 +++++
> > gdb/gdbarch-gen.c | 22 ++++++++++++++++++++++
> > gdb/gdbarch-gen.h | 10 ++++++++++
> > gdb/gdbarch_components.py | 15 +++++++++++++++
> > 5 files changed, 62 insertions(+)
> >
> > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index
> > be0494fba1e..c86df72a6d8 100644
> > --- a/gdb/arch-utils.c
> > +++ b/gdb/arch-utils.c
> > @@ -1218,6 +1218,16 @@ default_gdbarch_return_value
> > readbuf, writebuf);
> > }
> >
> > +/* See arch-utils.h. */
> > +
> > +std::optional<CORE_ADDR>
> > +default_get_shadow_stack_pointer (gdbarch *gdbarch, regcache *regcache,
> > + bool &shadow_stack_enabled)
> > +{
> > + shadow_stack_enabled = false;
> > + return {};
> > +}
> > +
> > obstack *gdbarch_obstack (gdbarch *arch) {
> > return &arch->obstack;
> > diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index
> > 1509cb7441e..14a84b74733 100644
> > --- a/gdb/arch-utils.h
> > +++ b/gdb/arch-utils.h
> > @@ -414,4 +414,9 @@ extern enum return_value_convention
> default_gdbarch_return_value
> > struct regcache *regcache, struct value **read_value,
> > const gdb_byte *writebuf);
> >
> > +/* Default implementation of gdbarch default_get_shadow_stack_pointer
> > + method. */
> > +extern std::optional<CORE_ADDR> default_get_shadow_stack_pointer
> > + (gdbarch *gdbarch, regcache *regcache, bool &shadow_stack_enabled);
> > +
> > #endif /* GDB_ARCH_UTILS_H */
> > diff --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
> > 3ca19c427a3..4490d53a940 100644
> > --- a/gdb/gdbarch-gen.c
> > +++ b/gdb/gdbarch-gen.c
> > @@ -263,6 +263,7 @@ struct gdbarch
> > gdbarch_use_target_description_from_corefile_notes_ftype
> *use_target_description_from_corefile_notes =
> default_use_target_description_from_corefile_notes;
> > gdbarch_core_parse_exec_context_ftype *core_parse_exec_context =
> default_core_parse_exec_context;
> > gdbarch_shadow_stack_push_ftype *shadow_stack_push = nullptr;
> > + gdbarch_get_shadow_stack_pointer_ftype *get_shadow_stack_pointer =
> > + default_get_shadow_stack_pointer;
> > };
> >
> > /* Create a new ``struct gdbarch'' based on information provided by
> > @@ -537,6 +538,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
> > /* Skip verify of use_target_description_from_corefile_notes, invalid_p == 0.
> */
> > /* Skip verify of core_parse_exec_context, invalid_p == 0. */
> > /* Skip verify of shadow_stack_push, has predicate. */
> > + /* Skip verify of get_shadow_stack_pointer, invalid_p == 0. */
> > if (!log.empty ())
> > internal_error (_("verify_gdbarch: the following are invalid ...%s"),
> > log.c_str ());
> > @@ -1414,6 +1416,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct
> ui_file *file)
> > gdb_printf (file,
> > "gdbarch_dump: shadow_stack_push = <%s>\n",
> > host_address_to_string (gdbarch->shadow_stack_push));
> > + gdb_printf (file,
> > + "gdbarch_dump: get_shadow_stack_pointer = <%s>\n",
> > + host_address_to_string (gdbarch->get_shadow_stack_pointer));
> > if (gdbarch->dump_tdep != NULL)
> > gdbarch->dump_tdep (gdbarch, file); } @@ -5583,3 +5588,20 @@
> > set_gdbarch_shadow_stack_push (struct gdbarch *gdbarch, {
> > gdbarch->shadow_stack_push = shadow_stack_push; }
> > +
> > +std::optional<CORE_ADDR>
> > +gdbarch_get_shadow_stack_pointer (struct gdbarch *gdbarch, regcache
> > +*regcache, bool &shadow_stack_enabled) {
> > + gdb_assert (gdbarch != NULL);
> > + gdb_assert (gdbarch->get_shadow_stack_pointer != NULL);
> > + if (gdbarch_debug >= 2)
> > + gdb_printf (gdb_stdlog, "gdbarch_get_shadow_stack_pointer
> > +called\n");
> > + return gdbarch->get_shadow_stack_pointer (gdbarch, regcache,
> > +shadow_stack_enabled); }
> > +
> > +void
> > +set_gdbarch_get_shadow_stack_pointer (struct gdbarch *gdbarch,
> > + gdbarch_get_shadow_stack_pointer_ftype
> > +get_shadow_stack_pointer) {
> > + gdbarch->get_shadow_stack_pointer = get_shadow_stack_pointer; }
> > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> > c276cd66d71..b8d1df94f59 100644
> > --- a/gdb/gdbarch-gen.h
> > +++ b/gdb/gdbarch-gen.h
> > @@ -1806,6 +1806,8 @@ extern void set_gdbarch_core_parse_exec_context
> (struct gdbarch *gdbarch, gdbarc
> > technologies. For example, the Intel Control-Flow Enforcement Technology
> > (Intel CET) on x86 provides a shadow stack and indirect branch tracking.
> > To enable inferior calls the function shadow_stack_push has to be provided.
> > + The method get_shadow_stack_pointer has to be provided to enable
> displaced
> > + stepping.
> >
> > Push the address NEW_ADDR on the shadow stack and update the shadow
> stack
> > pointer. */
> > @@ -1815,3 +1817,11 @@ extern bool gdbarch_shadow_stack_push_p
> (struct
> > gdbarch *gdbarch); typedef void (gdbarch_shadow_stack_push_ftype)
> > (struct gdbarch *gdbarch, CORE_ADDR new_addr, regcache *regcache);
> > extern void gdbarch_shadow_stack_push (struct gdbarch *gdbarch,
> > CORE_ADDR new_addr, regcache *regcache); extern void
> > set_gdbarch_shadow_stack_push (struct gdbarch *gdbarch,
> > gdbarch_shadow_stack_push_ftype *shadow_stack_push);
> > +
> > +/* If possible, return the shadow stack pointer. On some architectures, the
> > + shadow stack pointer is available even if the feature is disabled. To
> > + return the shadow stack enablement state configure
> > +SHADOW_STACK_ENABLED. */
> > +
> > +typedef std::optional<CORE_ADDR>
> > +(gdbarch_get_shadow_stack_pointer_ftype) (struct gdbarch *gdbarch,
> > +regcache *regcache, bool &shadow_stack_enabled); extern
> > +std::optional<CORE_ADDR> gdbarch_get_shadow_stack_pointer (struct
> > +gdbarch *gdbarch, regcache *regcache, bool &shadow_stack_enabled);
> > +extern void set_gdbarch_get_shadow_stack_pointer (struct gdbarch
> > +*gdbarch, gdbarch_get_shadow_stack_pointer_ftype
> > +*get_shadow_stack_pointer);
> > diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> > index ab685b14ec7..ccbdcaf07c9 100644
> > --- a/gdb/gdbarch_components.py
> > +++ b/gdb/gdbarch_components.py
> > @@ -2855,6 +2855,8 @@ Some targets support special hardware-assisted
> > control-flow protection technologies. For example, the Intel
> > Control-Flow Enforcement Technology (Intel CET) on x86 provides a shadow
> stack and indirect branch tracking.
> > To enable inferior calls the function shadow_stack_push has to be provided.
> > +The method get_shadow_stack_pointer has to be provided to enable
> > +displaced
>
> Maybe...
>
> "The get_shadow_stack_pointer gdbarch hook has to be..."
>
> > +stepping.
> >
> > Push the address NEW_ADDR on the shadow stack and update the shadow
> > stack pointer.
> > @@ -2864,3 +2866,16 @@ pointer.
> > params=[("CORE_ADDR", "new_addr"), ("regcache *", "regcache")],
> > predicate=True,
> > )
> > +
> > +Method(
> > + comment="""
> > +If possible, return the shadow stack pointer. On some architectures,
> > +the shadow stack pointer is available even if the feature is
> > +disabled. To return the shadow stack enablement state configure
> SHADOW_STACK_ENABLED.
> > +""",
> > + type="std::optional<CORE_ADDR>",
> > + name="get_shadow_stack_pointer",
> > + params=[("regcache *", "regcache"), ("bool &", "shadow_stack_enabled")],
> > + predefault="default_get_shadow_stack_pointer",
> > + invalid=False,
> > +)
>
> Otherwise looks OK.
>
> Reviewed-By: Luis Machado <luis.machado@arm.com>

Thanks for the review. For v5, I will apply all your comments and also fix the comment
as suggested to Tom here, if there are no concerns:
https://sourceware.org/pipermail/gdb-patches/2025-June/218864.html

Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928