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

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

Hi Thiago, 

Thank you for sharing the patch and the review.

I wonder if we should we split this commit into 2 separate ones:
#1 "aarch64: Implement gdbarch function top_addr_empty_shadow_stack."
#2 "gdb: Generalize handling of the shadow stack pointer."

This way you could be the author of patch #2, and I'll add my self as co-author
since I'll also move my changes from aarch64-linux-tdep.c into this patch.

With the commit "gdb: Generalize handling.." only, the GCS unwinding logic is
still functional and will be generalized in the aarch64 specific patch.
 
 I'll also include the patch in my v3 then.

What do you think?

Christina 

> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Freitag, 6. März 2026 04:57
> To: Schimpe, Christina <christina.schimpe@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 1/9] gdb: Generalize handling of the shadow stack
> pointer.
> 
> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
> 
> > Christina Schimpe <christina.schimpe@intel.com> writes:
> >
> >> +static bool
> >> +amd64_linux_top_addr_empty_shadow_stack
> >> +  (gdbarch *gdbarch, const CORE_ADDR addr,
> >> +   const std::pair<CORE_ADDR, CORE_ADDR> range)
> >>  {
> >> -  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 = amd64_linux_dwarf2_prev_ssp;
> >> -    }
> >> +  return addr == range.second;
> >>  }
> >
> > Next week I'll provide the AArch64 implementation of this hook, if you
> > can squash it into your patch.
> 
> It turns out I had it in my notes for patch 6:
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> 836af863d3ad..112d42c6a1ac 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1909,6 +1909,23 @@ aarch64_pop_gcs_entry (regcache *regs)
>    regcache_cooked_write_unsigned (regs, tdep->gcs_reg_base, gcs_addr + 8);
> }
> 
> +static bool
> +aarch64_top_addr_empty_shadow_stack (gdbarch *gdbarch, const
> CORE_ADDR addr,
> +				     const std::pair<CORE_ADDR, CORE_ADDR>
> range) {
> +  gdb_assert (addr >= range.first);
> +
> +  /* For AArch64, addr must be strictly less than the upper address in the
> +     range, but other architectures allow it to be equal to the upper
> +     address when the stack is empty so GDB core works with those addresses
> +     and can send them our way.  */
> +  gdb_assert (addr <= range.second);
> +
> +  /* The GCS grows down, and the oldest entry isn't an address.
> +     Just the value '0'.  */
> +  return addr >= range.second - 8;
> +}
> +
>  /* Implement the "push_dummy_call" gdbarch method.  */
> 
>  static CORE_ADDR
> @@ -4790,7 +4807,12 @@ aarch64_gdbarch_init (struct gdbarch_info info,
> struct gdbarch_list *arches)
> 
>    /* AArch64's shadow stack pointer is the GCSPR.  */
>    if (tdep->has_gcs ())
> -    set_gdbarch_ssp_regnum (gdbarch, tdep->gcs_reg_base);
> +    {
> +      /* AArch64's shadow stack pointer is the GCSPR.  */
> +      set_gdbarch_ssp_regnum (gdbarch, tdep->gcs_reg_base);
> +      set_gdbarch_top_addr_empty_shadow_stack
> +	(gdbarch, aarch64_top_addr_empty_shadow_stack);
> +    }
> 
>    /* ABI */
>    set_gdbarch_short_bit (gdbarch, 16);
> 
> 
> So with the above, for this patch:
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
> --
> Thiago
Intel Deutschland GmbH
Registered Address: Dornacher Strasse 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 Muenchen HRB 186928