Re: [PATCH v4 02/13] aarch64: Implement gdbarch function top_addr_empty_shadow_stack.
Luis <[email protected]>
| Newsgroups | gmane.comp.gdb.patches |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> > --- > 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." > + 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? > + 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.