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

[Luis Machado <luis.machado@arm.com>]

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>