RE: [PATCH 0/9] Add new command to print the shadow stack backtrace

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

Hi Thiago,

apologies for the delayed feedback. Please find my comments below.

> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Freitag, 31. Oktober 2025 01:47
> To: Schimpe, Christina <christina.schimpe@intel.com>
> Cc: 'gdb-patches@sourceware.org' <gdb-patches@sourceware.org>
> Subject: Re: [PATCH 0/9] Add new command to print the shadow stack
> backtrace
> 
> Hello Christina,
> 
> "Schimpe, Christina" <christina.schimpe@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Schimpe, Christina
> >> Sent: Monday, October 13, 2025 8:35 AM
> >> To: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> >> Cc: gdb-patches@sourceware.org
> >> Subject: RE: [PATCH 0/9] Add new command to print the shadow stack
> >> backtrace
> >>
> >> > -----Original Message-----
> >> > From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> >> > Sent: Monday, October 13, 2025 3:19 AM
> >> > To: Schimpe, Christina <christina.schimpe@intel.com>
> >> > Cc: gdb-patches@sourceware.org
> >> > Subject: Re: [PATCH 0/9] Add new command to print the shadow stack
> >> > backtrace
> >> >
> >> > Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
> >> >
> >> > > I tested your patches and there's a bug unfortunately:
>         ⋮
> >> > The problem turned out to be simple. I just had to set the regnum
> >> > for the GCSPR in the gdbarch:
> >> >
> >> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> >> > 95af82c26327..9e866fc319d4 100644
> >> > --- a/gdb/aarch64-tdep.c
> >> > +++ b/gdb/aarch64-tdep.c
> >> > @@ -4780,6 +4780,10 @@ aarch64_gdbarch_init (struct gdbarch_info
> >> info,
> >> > struct gdbarch_list *arches)
> >> >    /* Register a hook for converting a memory tag to a string.  */
> >> >    set_gdbarch_memtag_to_string (gdbarch,
> >> > aarch64_memtag_to_string);
> >> >
> >> > +  if (tdep->has_gcs ())
> >> > +    /* AArch64's shadow stack pointer is the GCSPR.  */
> >> > +    set_gdbarch_ssp_regnum (gdbarch, tdep->gcs_reg_base);
> >> > +
> >> >    /* ABI */
> >> >    set_gdbarch_short_bit (gdbarch, 16);
> >> >    set_gdbarch_int_bit (gdbarch, 32);
> >>
> >> Ah, I forgot about that. Cool that it works now. 😊
> >
> > I thought about this again. In addition to setting the regnum for the
> > shadow stack pointer, I would have expected that you also have to
> > implement the gdbarch hook top_addr_empty_shadow_stack to make GCS
> > work for shadow stack pointer unwinding and the shadow stack backtrace, or
> am I missing something?
> 
> Ah, I have a comment about this on my review of patch 1 that is sitting in my
> drafts folder. I was waiting to send them all together, but it's been taking me
> longer than I expected. I will just send what I have so far.
>
> But essentially, if there's no gdbarch_top_addr_empty_shadow_stack hook this
> patch series changes the check of whether ssp is empty from a <= comparison
> with the end of the range to a < comparison. Because of this,
> AArch64 doesn't need to implement the hook to make the existing aarch64-
> gcs*.exp tests work. It still needs it to make the "bt shadow"
> command work though.

I'll comment in the corresponding patch. Thanks a lot for catching that!
As you already commented,  the hook is required for amd64.

> > Both, the aarch64 implememtation for top_addr_empty_shadow_stack and
> > ssp regnum configuration, should then be part the first patch "Generalize
> handling ...".
> > Does that make sense to you?
> 
> Yes, patch 1 should include the changes needed to make the existing aarch64-
> gcs*.exp tests passing. As of v1, that's just the ssp regnum configuration.
> Depending on what you decide to do for v2, that could include the
> top_addr_empty_shadow_stack hook as well.
> 
> > Also I should probably add proper error messages when the
> > configuration of ssp_regnum is missing,
> 
> Is that possible? How would GDB distinguish between missing ssp regnum
> configuration versus the target not supporting shadow stacks?

I would add it to the error message that already exists for the absence of the hook
gdbarch_address_in_shadow_stack_memory_range:

--- a/gdb/shadow-stack.c
+++ b/gdb/shadow-stack.c
@@ -537,7 +537,8 @@ backtrace_shadow_command (const shadow_stack_print_options &print_options,
     error (_("No shadow stack."));
 
   gdbarch *gdbarch = get_current_arch ();
-  if (!gdbarch_address_in_shadow_stack_memory_range_p (gdbarch))
+  if (!gdbarch_address_in_shadow_stack_memory_range_p (gdbarch)
+      || gdbarch_ssp_regnum (gdbarch) == -1)
     error (_("Printing of the shadow stack backtrace is not supported for"
             " the current target."))

So the error message could mean both, either missing ssp regnum or the target
not supporting shadow stack. Later in that function we check if the shadow stack
is enabled for the current target.

Does that sound ok?

> > and improve the documentation for required gdbarch hooks to properly
> > implement "bt shadow" in gdbarch_components.py.
> 
> --
> Thiago

Christina
Intel Deutschland GmbH
Registered Address: Dornacher Straße 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht München HRB 186928