RE: [PATCH v4 08/11] gdb, gdbarch: Enable inferior calls for shadow stack support.

Schimpe, Christina
2025-06-23T17:50:29Z

> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Thursday, June 19, 2025 11:25 AM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: thiago.bauermann@linaro.org; eliz@gnu.org
> Subject: Re: [PATCH v4 08/11] gdb, gdbarch: Enable inferior calls for shadow
> stack support.
>
> On 6/17/25 13:11, Christina Schimpe wrote:
> > Inferior calls in GDB reset the current PC to the beginning of the
> > function that is called. As no call instruction is executed the new
> > return address needs to be pushed to the shadow stack and the shadow
> > stack pointer needs to be updated.
> >
> > This commit adds a new gdbarch method to push an address on the shadow
> > stack. The method is used to adapt the function
> 'call_function_by_hand_dummy'
> > for inferior call shadow stack support.
> >
> > Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > ---
> > gdb/gdbarch-gen.c | 32 ++++++++++++++++++++++++++++++++
> > gdb/gdbarch-gen.h | 14 ++++++++++++++
> > gdb/gdbarch_components.py | 16 ++++++++++++++++
> > gdb/infcall.c | 14 ++++++++++----
> > 4 files changed, 72 insertions(+), 4 deletions(-)
> >
> > diff --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
> > 32d16598940..3ca19c427a3 100644
> > --- a/gdb/gdbarch-gen.c
> > +++ b/gdb/gdbarch-gen.c
> > @@ -262,6 +262,7 @@ struct gdbarch
> > gdbarch_read_core_file_mappings_ftype *read_core_file_mappings =
> default_read_core_file_mappings;
> > 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;
> > };
> >
> > /* Create a new ``struct gdbarch'' based on information provided by
> > @@ -535,6 +536,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
> > /* Skip verify of read_core_file_mappings, invalid_p == 0. */
> > /* 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. */
> > if (!log.empty ())
> > internal_error (_("verify_gdbarch: the following are invalid ...%s"),
> > log.c_str ());
> > @@ -1406,6 +1408,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct
> ui_file *file)
> > gdb_printf (file,
> > "gdbarch_dump: core_parse_exec_context = <%s>\n",
> > host_address_to_string (gdbarch->core_parse_exec_context));
> > + gdb_printf (file,
> > + "gdbarch_dump: gdbarch_shadow_stack_push_p() = %d\n",
> > + gdbarch_shadow_stack_push_p (gdbarch));
> > + gdb_printf (file,
> > + "gdbarch_dump: shadow_stack_push = <%s>\n",
> > + host_address_to_string (gdbarch->shadow_stack_push));
> > if (gdbarch->dump_tdep != NULL)
> > gdbarch->dump_tdep (gdbarch, file); } @@ -5551,3 +5559,27 @@
> > set_gdbarch_core_parse_exec_context (struct gdbarch *gdbarch, {
> > gdbarch->core_parse_exec_context = core_parse_exec_context; }
> > +
> > +bool
> > +gdbarch_shadow_stack_push_p (struct gdbarch *gdbarch) {
> > + gdb_assert (gdbarch != NULL);
> > + return gdbarch->shadow_stack_push != NULL; }
> > +
> > +void
> > +gdbarch_shadow_stack_push (struct gdbarch *gdbarch, CORE_ADDR
> > +new_addr, regcache *regcache) {
> > + gdb_assert (gdbarch != NULL);
> > + gdb_assert (gdbarch->shadow_stack_push != NULL);
> > + if (gdbarch_debug >= 2)
> > + gdb_printf (gdb_stdlog, "gdbarch_shadow_stack_push called\n");
> > + gdbarch->shadow_stack_push (gdbarch, new_addr, regcache); }
> > +
> > +void
> > +set_gdbarch_shadow_stack_push (struct gdbarch *gdbarch,
> > + gdbarch_shadow_stack_push_ftype
> shadow_stack_push) {
> > + gdbarch->shadow_stack_push = shadow_stack_push; }
> > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> > 313a8f198fd..c276cd66d71 100644
> > --- a/gdb/gdbarch-gen.h
> > +++ b/gdb/gdbarch-gen.h
> > @@ -1801,3 +1801,17 @@ extern void
> > set_gdbarch_use_target_description_from_corefile_notes (struct gdbar
> > typedef core_file_exec_context (gdbarch_core_parse_exec_context_ftype)
> > (struct gdbarch *gdbarch, bfd *cbfd); extern core_file_exec_context
> > gdbarch_core_parse_exec_context (struct gdbarch *gdbarch, bfd *cbfd);
> > extern void set_gdbarch_core_parse_exec_context (struct gdbarch
> > *gdbarch, gdbarch_core_parse_exec_context_ftype
> > *core_parse_exec_context);
> > +
> > +/* 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.
> > +> + Push the address NEW_ADDR on the shadow stack and update the
> > +> + shadow stack
> > + pointer. */
> > +
> > +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);
> > diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> > index ec09d955088..ab685b14ec7 100644
> > --- a/gdb/gdbarch_components.py
> > +++ b/gdb/gdbarch_components.py
> > @@ -2848,3 +2848,19 @@ which all assume current_inferior() is the one to
> read from.
> > predefault="default_core_parse_exec_context",
> > invalid=False,
> > )
> > +
> > +Method(
> > + comment="""
> > +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.
>
> Maybe update the last part like so?
>
> "To enable shadow stack support for inferior calls the shadow_stack_push
> gdbarch hook has to be provided."
>
> > +
> > +Push the address NEW_ADDR on the shadow stack and update the shadow
> > +stack pointer.
> > +""",
>
> Maybe...
>
> "Push NEW_ADDR to the shadow stack..."?
>
> > + type="void",
> > + name="shadow_stack_push",
> > + params=[("CORE_ADDR", "new_addr"), ("regcache *", "regcache")],
> > + predicate=True,
> > +)
> > diff --git a/gdb/infcall.c b/gdb/infcall.c index
> > 098072dfd2a..a1c80c81238 100644
> > --- a/gdb/infcall.c
> > +++ b/gdb/infcall.c
> > @@ -1448,10 +1448,16 @@ call_function_by_hand_dummy (struct value
> *function,
> > /* Create the dummy stack frame. Pass in the call dummy address as,
> > presumably, the ABI code knows where, in the call dummy, the
> > return address should be pointed. */
> > - sp = gdbarch_push_dummy_call (gdbarch, function,
> > - get_thread_regcache (inferior_thread ()),
> > - bp_addr, args.size (), args.data (),
> > - sp, return_method, struct_addr);
> > + regcache *regcache = get_thread_regcache (inferior_thread ());
> > + sp = gdbarch_push_dummy_call (gdbarch, function, regcache, bp_addr,
> > + args.size (), args.data (), sp,
> > + return_method, struct_addr);
> > +
> > + /* Push the return address of the inferior (bp_addr) on the shadow
> > + stack
>
> s/on the shadow stack/to the shadow stack
>
> > + and update the shadow stack pointer. As we don't execute a call
> > + instruction to start the inferior we need to handle this
> > + manually. */
>
> s/start the inferior/call the function
>
> > + if (gdbarch_shadow_stack_push_p (gdbarch))
> > + gdbarch_shadow_stack_push (gdbarch, bp_addr, regcache);
> >
> > /* Set up a frame ID for the dummy frame so we can pass it to
> > set_momentary_breakpoint. We need to give the breakpoint a
> > frame
>
> Otherwise looks OK to me.
>
> Reviewed-By: Luis Machado <luis.machado@arm.com>

Thanks for the review. I agree with all your comments and will fix them in v5.

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