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

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

> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Samstag, 12. September 2026 08:03
> To: Joos, Christina <christina.joos@intel.com>
> Cc: gdb-patches@sourceware.org; tom@tromey.com
> Subject: Re: [PATCH v4 02/13] aarch64: Implement gdbarch function
> top_addr_empty_shadow_stack.
> 
> Christina Schimpe <christina.schimpe@intel.com> writes:
> 
> > @@ -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);
> 
> As I just found out and I just mentioned in my email on patch 8, ideally this
> gdbarch hook should be set unconditionally, not only when
> tdep->has_gcs_linux ().
> 
> But this would be a change unrelated to your patch. I can make it later when I
> submit patches enabling "bt -shadow" for AArch64.
> 
> > -      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'.  */
> > +  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);
> > +      set_gdbarch_top_addr_empty_shadow_stack
> > +	(gdbarch, aarch64_top_addr_empty_shadow_stack);
> > +    }
> 
> On the other hand, since this is new code added by this patch, could you
> please change this to the following?
> 
>   if (tdep->has_gcs ())
>     /* 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);

Yes, of course.
One nit: I think the GDB Coding Standards still require some braces here.

So, my overall changes for this patch will look as follows (Luis requested some
comment changes, too).

~~~
$ git diff
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index d789b8569d2..94893dd4bcf 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1923,8 +1923,8 @@ aarch64_top_addr_empty_shadow_stack
      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'.  */
+  /* The GCS grows down, and the oldest entry is the value '0', not an
+     address.  */
   return addr >= range.second - 8;
 }

@@ -4807,10 +4807,13 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     {
       /* 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);
     }

+  /* Implementing this hook is required to allow the unwinding of
+     the shadow stack pointer in shadow-stack.c:dwarf2_prev_ssp.  */
+  set_gdbarch_top_addr_empty_shadow_stack
+    (gdbarch, aarch64_top_addr_empty_shadow_stack);
+
~~~

I hope this is ok from your side, if not please let me know. 😊

Kind 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.