RE: [PATCH v2 6/9] gdb: Add command option 'bt -shadow' to print the shadow stack backtrace.

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

> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Freitag, 6. März 2026 05:31
> To: Schimpe, Christina <christina.schimpe@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 6/9] gdb: Add command option 'bt -shadow' to print
> the shadow stack backtrace.
> 
> 
> I'll have a closer look at this patch next week, including your comments about
> the count argument to the get_trailing_outermost_shadow_stack_frame_info
> in the thread for v1 of this series.. Unfortunately I didn't have much time to
> dig into it yet.
> 
> Some minor comments for now:
> 
> Christina Schimpe <christina.schimpe@intel.com> writes:
> 
> > diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index
> > 656daa0f0ee..a4eabccf667 100644
> > --- a/gdb/amd64-linux-tdep.c
> > +++ b/gdb/amd64-linux-tdep.c
> > @@ -1963,6 +1963,29 @@ amd64_linux_top_addr_empty_shadow_stack
> >    return addr == range.second;
> >  }
> >
> > +/* Return the number of elements which are currently on the shadow
> stack
> > +   based on the shadow stack memory RANGE [start_address, end_address)
> > +   of the current thread.  In case shadow stack is not enabled for the
> > +   current thread, return -1.  */
> > +
> > +static long
> > +amd64_linux_get_shadow_stack_size
> > +  (gdbarch *gdbarch,
> > +   const std::optional<CORE_ADDR> ssp,
> > +   const std::pair<CORE_ADDR, CORE_ADDR> range) {
> > +  /* For x86, if we don't have a shadow stack pointer, we can assume
> > +     that the shadow stack is disabled for the current thread.  */
> > +  if (!ssp.has_value ())
> > +    return -1;
> 
> As I mention a bit below, the only caller of this function passes a value for the
> ssp argument, so this is dead code.
> 
> > +  const unsigned long shadow_stack_bytes = range.second - *ssp;
> > +
> > +  gdb_assert ((shadow_stack_bytes % 8) == 0);
> 
> I don't think this should be an assert. If it fails, it triggers an internal error in
> GDB.  In this case it could indeed mean an internal error (GDB somehow got
> the SSP or range wrong), but it could also be (and probably more likely) an
> inconsistent state of the inferior. This can happen in a program being
> debugged so GDB should be able to handle it gracefully, and if possible
> provide useful information to the user.
> 
> > +  return shadow_stack_bytes / 8;
> > +}
> 
>   ⋮
> 
> > +/* Read the memory at shadow stack pointer SSP and assign it to
> > +   RETURN_VALUE.  In case we cannot read the memory, set REASON to
> > +   ssp_unwind_stop_reason::memory_read_error and return false.  */
> > +
> > +static bool
> > +read_shadow_stack_memory (gdbarch *gdbarch, CORE_ADDR ssp,
> > +			  CORE_ADDR &return_value,
> > +			  ssp_unwind_stop_reason *reason)
> 
> The reason argument can also be a reference.
> 
> > +{
> > +  /* On x86 there can be a shadow stack token at bit 63.  For x32, the
> > +     address size is only 32 bit.  Thus, we still must use
> > +     gdbarch_shadow_stack_element_size_aligned (and not
> gdbarch_addr_bit)
> > +     to read the full element for x32 as well.  */
> > +  const int element_size
> > +    = gdbarch_shadow_stack_element_size_aligned (gdbarch);
> > +
> > +  const bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > +  if (!safe_read_memory_unsigned_integer (ssp, element_size, byte_order,
> > +					  &return_value))
> > +    {
> > +      *reason = ssp_unwind_stop_reason::memory_read_error;
> > +      return false;
> > +    }
> > +
> > +  return true;
> > +}
> > +
> > +/*  If possible, return the starting shadow stack frame info needed to
> handle
> > +    COUNT outermost frames.  FRAME should point to the innermost
> (newest)
> > +    element of the shadow stack.  RANGE is the shadow stack memory
> range
> > +    [start_address, end_address) corresponding to FRAME's shadow stack
> pointer.
> > +    If COUNT is bigger than the number of elements on the shadow stack,
> return
> > +    FRAME.  In case of failure, assign an appropriate
> ssp_unwind_stop_reason in
> > +    FRAME->UNWIND_stop_REASON.  */
> > +
> > +static std::optional<shadow_stack_frame_info>
> > +get_trailing_outermost_shadow_stack_frame_info
> > +  (gdbarch *gdbarch, const std::pair<CORE_ADDR, CORE_ADDR> range,
> > +   const ULONGEST count, shadow_stack_frame_info &frame) {
> > +  gdb_assert (gdbarch_get_shadow_stack_size_p (gdbarch));
> > +
> > +  const long shadow_stack_size
> > +    = gdbarch_get_shadow_stack_size (gdbarch,
> > +				     std::optional<CORE_ADDR> (frame.ssp),
> > +				     range);
> 
> This is the only caller of gdbarch_get_shadow_stack_size. Does its ssp
> argument need to be std::optional<CORE_ADDR>, or can it simply be a
> CORE_ADDR?
> 
> > +  /* We should only get here in case shadow stack is enabled for the
> > +     current thread.  */
> > +  gdb_assert (shadow_stack_size >= 0);
> > +
> > +  const long level = shadow_stack_size - count;
> > +
> > +  /* COUNT exceeds the number of elements on the shadow stack.  Return
> the
> > +     starting shadow stack frame info FRAME.  */  if (level <= 0)
> > +    return std::optional<shadow_stack_frame_info> (frame);
> > +
> > +  CORE_ADDR new_ssp = update_shadow_stack_pointer
> > +    (gdbarch, frame.ssp, level, ssp_update_direction::outer);
> > +
> > +  if (gdbarch_stack_grows_down (gdbarch))
> > +    gdb_assert (new_ssp < range.second);  else
> > +    gdb_assert (new_ssp >= range.first);
> > +
> > +  CORE_ADDR new_value;
> > +  if (!read_shadow_stack_memory (gdbarch, new_ssp, new_value,
> > +				 &frame.unwind_stop_reason))
> > +    return {};
> > +
> > +  return std::optional<shadow_stack_frame_info>
> > +    ({new_ssp, new_value, (unsigned long) level,
> > +      ssp_unwind_stop_reason::no_error});
> > +}
> 
>   ⋮
> 
> > diff --git a/gdb/shadow-stack.h b/gdb/shadow-stack.h index
> > 5f8395ec047..5370becfc9a 100644
> > --- a/gdb/shadow-stack.h
> > +++ b/gdb/shadow-stack.h
> > @@ -35,4 +35,10 @@ void shadow_stack_push (regcache *regcache, const
> > CORE_ADDR new_addr);  value *dwarf2_prev_ssp (const frame_info_ptr
> &this_frame,
> >  			void **this_cache, int regnum);
> >
> > +/* Implementation of "backtrace shadow" comand.  */
> > +
> > +void backtrace_shadow_command
> > +  (const frame_print_options &fp_opts,
> > +   const char *count_exp, int from_tty);
> > +
> >  #endif /* GDB_SHADOW_STACK_H */
> 
> This header needs to forward-declare "struct frame_print_options;", similarly
> to the "class regcache;" forward declaration.
> 
> Actually, one thing I missed in my review of patch 1 is that it also needs to
> forward-declare "class frame_info_ptr;" which is used in the prototype of
> dwarf2_prev_ssp.
> 
> --
> Thiago

Thank you for the feedback.

Together with the -past-main functionality, I'll also add some more changes to this patch.
I just realized recently that a shadow stack frame specific gdbarch is probably a good idea,
since we use whatever is current at the point of running this command.
I am not sure if it ever changes for IA, but it might be a good idea to add it to be safe.

But I'm still not sure about some opens for this series, which I have summarized in the cover letter.
https://sourceware.org/pipermail/gdb-patches/2026-January/224388.html

Kind Regards,
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