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

["Schimpe, Christina" <christina.schimpe@intel.com>]

> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Saturday, February 8, 2025 5:04 AM
> To: Schimpe, Christina <christina.schimpe@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 11/12] gdb, gdbarch: Introduce gdbarch method to get the
> shadow stack pointer.
> 
> 
> "Schimpe, Christina" <christina.schimpe@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> >> Sent: Thursday, February 6, 2025 4:35 AM
> >> To: Schimpe, Christina <christina.schimpe@intel.com>
> >> Cc: gdb-patches@sourceware.org
> >> Subject: Re: [PATCH 11/12] gdb, gdbarch: Introduce gdbarch method to
> >> get the shadow stack pointer.
> >>
> >>
> >> "Schimpe, Christina" <christina.schimpe@intel.com> writes:
> >>
> >> > diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> >> > index 52f265e8e0e..df70cb082a4 100644
> >> > --- a/gdb/gdbarch_components.py
> >> > +++ b/gdb/gdbarch_components.py
> >> > @@ -2822,6 +2822,8 @@ Some targets support special
> >> > hardware-assisted control-flow protection  technologies.  For
> >> > example, Intel's Control-flow Enforcement Technology (CET)
> >> > 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.
> >> > @@ -2831,3 +2833,11 @@ pointer.
> >> >      params=[("CORE_ADDR", "new_addr")],
> >> >      predicate=True,
> >> >  )
> >> > +
> >> > +Method(
> >> > +    type="std::optional<CORE_ADDR>",
> >> > +    name="get_shadow_stack_pointer",
> >> > +    params=[],
> >> > +    predefault="default_get_shadow_stack_pointer",
> >> > +    invalid=False,
> >> > +)
> >>
> >> Ideally, there should be a comment on this method entry.
> >>
> >> This method is only used in amd64-tdep.c and i386-tdep.c. IMHO it
> >> would be better to put it in i386_gdbarch_tdep instead.
> >
> > Hi Thiago,
> >
> > Thank you for the review.
> >
> > As also discussed here:
> > https://sourceware.org/pipermail/gdb-patches/2025-February/215266.html
> > , I wonder if it's better to keep the code generic.
> 
> Indeed, that's a good point. In the case of this method, I have an additional
> concern though: its callers assume that if gdbarch_get_shadow_stack_pointer
> returns a value, then it means that shadow stacks are enabled in the inferior. But
> this is an x86 particularity. On AArch64, if the processor supports shadow stacks
> then the shadow stack register is always available, even if shadow stacks are
> turned off.

Ah ok, I did not know.

> So I think that there should be an additional method to indicate whether shadow
> stacks are enabled in the inferior.

Yes, that makes sense. But I want to avoid that we call ptrace twice on x86,  once in the method
to check the enablement state and once to get the shadow stack pointer.
Would something like that be acceptable as well?

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=[("bool &", "shadow_stack_enabled")],
predefault="default_get_shadow_stack_pointer",
invalid=False,
)

> > And I plan to use the gdbarch method gdbarch_get_shadow_stack_pointer
> > in a future series for "bt shadow".
> > I shared the commits in this comment:
> > https://sourceware.org/pipermail/gdb-patches/2025-February/215178.html
> > ,
> 
> Indeed. I see the same assumption in that code as well. Which makes a lot of
> sense since it only needs to support x86. But that will need to be adapted when
> posted upstream.

Yes, I agree.

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