RE: [PATCH v2 1/9] gdb: Generalize handling of the shadow stack pointer.

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

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Donnerstag, 19. Februar 2026 18:55
> To: Schimpe, Christina <christina.schimpe@intel.com>
> Cc: gdb-patches@sourceware.org; thiago.bauermann@linaro.org
> Subject: Re: [PATCH v2 1/9] gdb: Generalize handling of the shadow stack
> pointer.
> 
> >>>>> Christina Schimpe <christina.schimpe@intel.com> writes:
> 
> > Until now, handling of the shadow stack pointer has been done in the
> > target dependent implementations of the gdbarch hook
> > 'gdbarch_shadow_stack_push'.  Also amd64 and aarch64 linux specific
> > unwinders for the shadow stack pointer are implemented.
> > In a following patch a command line option "-shadow" will be added to
> > the backtrace command to print the shadow stack backtrace.  This
> > requires more target-independent logic to handle the shadow stack
> > pointer.  To avoid that we duplicate the logic, add new source and
> > header files "shadow-stack" for the implementation of
> > shadow_stack_push and shadow stack pointer unwinding in a target-
> independent way.
> 
> This looks reasonable to me, but I think someone else ought to examine the
> arch-specific bits.

Ok, I'll wait for more feedback on this. Thiago has run some tests for GCS already, 
but only with v1 AFAIK.

> Like I mentioned in another note, be sure to run check-gdbarch.py.

I assume you mean gdb/gdbarch.py.

> I had a nit or two, plus a question, but nothing really serious.
> 
> > +static void
> > +amd64_init_reg (gdbarch *gdbarch, int regnum, dwarf2_frame_state_reg
> *reg,
> > +		const frame_info_ptr &this_frame)
> > +{
> > +  if (regnum == gdbarch_pc_regnum (gdbarch))
> > +    reg->how = DWARF2_FRAME_REG_RA;
> > +  else if (regnum == gdbarch_sp_regnum (gdbarch))
> > +    reg->how = DWARF2_FRAME_REG_CFA;
> > +  else if (regnum == AMD64_PL3_SSP_REGNUM)
> > +    {
> > +      reg->how = DWARF2_FRAME_REG_FN;
> > +      reg->loc.fn = dwarf2_prev_ssp;
> > +    }
> > +}
> > +
> >  void
> >  amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> >  		const target_desc *default_tdesc)
> > @@ -3650,6 +3667,9 @@ amd64_init_abi (struct gdbarch_info info, struct
> gdbarch *gdbarch,
> >    set_gdbarch_in_indirect_branch_thunk (gdbarch,
> >  					amd64_in_indirect_branch_thunk);
> 
> > +  set_gdbarch_ssp_regnum (gdbarch, tdep->ssp_regnum);
> > + dwarf2_frame_set_init_reg (gdbarch, amd64_init_reg);
> > +
> 
> I was curious why this code refers to tdep->ssp_regnum but then the regnum
> seems to be hard-coded in amd64_init_reg.

In case CET shadow stack is not available due to missing kernel or HW support 
"tdep->ssp_regnum  == -1".
It should be cleaner to check "if tdep->ssp_regnum != -1" and only then configure it
using tdep->ssp_regnum because the default is -1 anyway.

For amd64_init_reg, I could be using "gdbarch_ssp_regnum (gdbarch)" now, too.
It makes the code more consistent.

Thanks for catching that.

> > +enum class ssp_update_direction
> > +{
> > +  /* Update ssp towards the oldest (outermost) element of the shadow
> > +     stack.  */
> > +  outer = 0,
> > +
> > +  /* Update ssp towards the most recent (innermost) element of the
> > +     shadow stack.  */
> > +  inner
> 
> Mildly prefer a trailing "," on enums.  That way if a new value is needed the
> patch doesn't have to touch this line.

I agree, will fix.

> > +/* See shadow-stack.h.  */
> > +
> > +void shadow_stack_push (regcache *regcache, const CORE_ADDR
> new_addr)
> 
> Newline after 'void'

Will fix.

> Reviewed-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Thank you for the review.

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