RE: [PATCH v4 02/13] aarch64: Implement gdbarch function top_addr_empty_shadow_stack.

["Joos, Christina" <christina.joos@intel.com>]

Thank you for the feedback.

> -----Original Message-----
> From: Luis <luis.machado.foss@gmail.com>
> Sent: Donnerstag, 13. August 2026 00:13
> To: Joos, Christina <christina.joos@intel.com>; gdb-patches@sourceware.org
> Cc: tom@tromey.com; thiago.bauermann@linaro.org
> Subject: Re: [PATCH v4 02/13] aarch64: Implement gdbarch function
> top_addr_empty_shadow_stack.
> 
> On 08/07/2026 15:36, Christina Schimpe wrote:
> > With the implementation of that gdbarch function we can remove the
> > arch specific function aarch64_linux_dwarf2_prev_gcspr and use the
> > generalized function dwarf2_prev_ssp instead.
> >
> > Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
> > ---
> >   gdb/aarch64-linux-tdep.c | 51 ++--------------------------------------
> >   gdb/aarch64-tdep.c       | 28 ++++++++++++++++++++--
> >   2 files changed, 28 insertions(+), 51 deletions(-)
> >
> > diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index
> > f11eccc1bc1..b5959057972 100644
> > --- a/gdb/aarch64-linux-tdep.c
> > +++ b/gdb/aarch64-linux-tdep.c
> > @@ -66,6 +66,7 @@
> >   #include "elf/common.h"
> >   #include "elf/aarch64.h"
> >   #include "arch/aarch64-insn.h"
> > +#include "shadow-stack.h"
> >
> >   /* For std::pow */
> >   #include <cmath>
> > @@ -2617,54 +2618,6 @@ aarch64_linux_get_shadow_stack_pointer
> (gdbarch *gdbarch, regcache *regcache,
> >     return gcspr;
> >   }
> >
> > -/* Implement Guarded Control Stack Pointer Register unwinding.  For each
> > -   previous GCS pointer check if its address is still in the GCS memory
> > -   range.  If it's outside the range set the returned value to unavailable,
> > -   otherwise return a value containing the new GCS pointer.  */
> > -
> > -static value *
> > -aarch64_linux_dwarf2_prev_gcspr (const frame_info_ptr &this_frame,
> > -				 void **this_cache, int regnum)
> > -{
> > -  value *v = frame_unwind_got_register (this_frame, regnum, regnum);
> > -  gdb_assert (v != nullptr);
> > -
> > -  gdbarch *gdbarch = get_frame_arch (this_frame);
> > -
> > -  if (v->entirely_available () && !v->optimized_out ())
> > -    {
> > -      int size = register_size (gdbarch, regnum);
> > -      bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > -      CORE_ADDR gcspr = extract_unsigned_integer (v->contents_all ().data (),
> > -						  size, byte_order);
> > -
> > -      /* Starting with v6.13, the Linux kernel supports Guarded Control
> > -	 Stack.  Using /proc/PID/smaps we can only check if the current
> > -	 GCSPR points to GCS memory.  Only if this is the case a valid
> > -	 previous GCS pointer can be calculated.  */
> > -      std::pair<CORE_ADDR, CORE_ADDR> range;
> > -      if (linux_address_in_shadow_stack_mem_range (gcspr, &range))
> > -	{
> > -	  /* The GCS grows downwards.  To compute the previous GCS pointer,
> > -	     we need to increment the GCSPR.  */
> > -	  CORE_ADDR new_gcspr = gcspr + 8;
> > -
> > -	  /* If NEW_GCSPR still points within the current GCS memory range
> > -	     we consider it to be valid.  */
> > -	  if (new_gcspr < range.second)
> > -	    return frame_unwind_got_address (this_frame, regnum,
> new_gcspr);
> > -	}
> > -    }
> > -
> > -  /* Return a value which is marked as unavailable in case we could not
> > -     calculate a valid previous GCS pointer.  */
> > -  value *retval
> > -    = value::allocate_register (get_next_frame_sentinel_okay (this_frame),
> > -				regnum, register_type (gdbarch, regnum));
> > -  retval->mark_bytes_unavailable (0, retval->type ()->length ());
> > -  return retval;
> > -}
> > -
> >   /* AArch64 Linux implementation of the report_signal_info gdbarch
> >      hook.  Displays information about possible memory tag violations.
> > */
> >
> > @@ -3243,7 +3196,7 @@ aarch64_linux_init_abi (struct gdbarch_info info,
> struct gdbarch *gdbarch)
> >       {
> >         set_gdbarch_get_shadow_stack_pointer (gdbarch,
> >
> 	aarch64_linux_get_shadow_stack_pointer);
> > -      tdep->fn_prev_gcspr = aarch64_linux_dwarf2_prev_gcspr;
> > +      tdep->fn_prev_gcspr = dwarf2_prev_ssp;
> >       }
> >   }
> >
> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> > 848cee3043c..d789b8569d2 100644
> > --- a/gdb/aarch64-tdep.c
> > +++ b/gdb/aarch64-tdep.c
> > @@ -1908,6 +1908,26 @@ aarch64_pop_gcs_entry (regcache *regs)
> >     regcache_cooked_write_unsigned (regs, tdep->gcs_reg_base, gcs_addr +
> 8);
> >   }
> >
> > +/* Implement the "top_addr_empty_shadow_stack" gdbarch method.  */
> > +
> > +static bool
> > +aarch64_top_addr_empty_shadow_stack
> > +  (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'.  */
> 
> Nit: Might read better if...
> 
> "The GCS grows down, and the oldest entry is the value '0', not an address."

Will fix.

> > +  return addr >= range.second - 8;
> > +}
> > +
> >   /* Implement the "push_dummy_call" gdbarch method.  */
> >
> >   static CORE_ADDR
> > @@ -4783,9 +4803,13 @@ 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);
> >
> > -  /* 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);
> 
> Should we have a small comment here stating why we're setting up this hook?

I can for sure add a comment that this is required for ARM to allow unwinding of
the shadow stack pointer in dwarf2_prev_ssp.

> > +      set_gdbarch_top_addr_empty_shadow_stack
> > +	(gdbarch, aarch64_top_addr_empty_shadow_stack);
> > +    }
> >
> >     /* ABI */
> >     set_gdbarch_short_bit (gdbarch, 16);
> 
> Otherwise looks OK to me.

I will wait for more feedback for this series (Thiago indicated he will review it soon)
and then post a new series with your and his feedback included. 

Regards,
Christina
________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.