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

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

> -----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