RE: [PATCH v4 00/13] Add new command to print the shadow stack backtrace

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

Hi Thiago, 

Thanks for the feedback!

> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Donnerstag, 3. September 2026 08:45
> To: Joos, Christina <christina.joos@intel.com>
> Cc: gdb-patches@sourceware.org; tom@tromey.com
> Subject: Re: [PATCH v4 00/13] Add new command to print the shadow stack
> backtrace
> 
> Hello Christina,
> 
> Thank you for this new version and for addressing the review comments.
> 
> Also thank you for this detailed cover letter. It's very helpful.
> 
> Christina Schimpe <christina.schimpe@intel.com> writes:
> 
> > Hi all,
> >
> > this is my v4 of the series
> > "Add new command to print the shadow stack backtrace".
> >
> > ***Diff of v4 to v3***:
> >
> > - Minor fixup for ARM compilation
> > - The commit
> >   "gdb: Provide gdbarch hook to distinguish shadow stack backtrace
> elements."
> >   was accidentially merged into the previous one
> >   ("gdb: Add command option 'bt -shadow' to print the shadow stack
> backtrace.").
> >   Isolate it again.
> >
> > Other than that, I am keeping the full cover letter summarizing the
> > changes for v3 (diff of v3 to v2). It has not been reviewed yet, since
> > I posted it just a couple days ago.
> >
> > ***Diff of v3 to v2***:
> >
> > It now includes:
> > - the implementation of the -past-main command line option
> > - support for inferior calls
> > (printing of <function called from gdb> instead of the shadow stack
> > element)
> > - full support for signals
> > (printing of <signal handler called> instead of the shadow stack
> > element)
> 
> Great improvements!
> 
> > Due to this some larger refactoring was required; I summarized it here:
> > https://sourceware.org/pipermail/gdb-patches/2026-May/227481.html
> >
> > The refactoring mostly affected the following commits:
> > "gdb: Provide gdbarch hook to distinguish shadow stack backtrace elements."
> > "gdb: Add command option 'bt -shadow' to print the shadow stack
> backtrace."
> > "gdb: Implement the hook 'is_no_return_shadow_stack_address' for amd64
> linux."
> > I did not add any Reviewed-By or Approved-By tags for the 3 commits,
> > since they changed significantly.
> >
> > Furthermore, I addressed (hopefully all) the comments of Tom, Thiago and
> Eli:
> > - Remove annotations
> > - Some better code reuse (especially for the mi patch)
> > - A new patch "aarch64: Implement gdbarch function
> top_addr_empty_shadow_stack."
> >   This also allows to remove some target dependent GCS code in
> > aarch64-*.c
> 
> Nice! Thanks.
> 
> > - Various smaller issues and nits
> > - Fixes for check-gdbarch.py
> >
> > And finally some smaller issues I noted myself (mostly for patch #1
> > "gdb: Generalize handling of the shadow stack pointer."):
> > - Remove unused gdbarch parameter in some of the introduced hooks
> > - Changes in gdb/aarch64-tdep.c when calling shadow_stack_push, since
> > some code for getting the shadow stack pointer and checking the
> > enablement state was duplicated.
> >
> > Opens:
> > 1) Thiago suggested changing the frame numbering so that it always
> > starts at #1, since for the shadow stack we don't have frame #0
> > printed by the normal backtrace.
> > 2) Or, consider printing frame #0 similarly to what the normal
> > backtrace does
> 
> I still prefer option 1, but I'm also fine if some other option is chosen.

I'd stay with what I have for now. I hope this is all right. Changing it should be fast. 

> > 3) Consider printing frame arguments (but I believe, if possible, this
> > should better be added in a follow-up series)
> 
> IMHO it's not necessary, but I agree it's for a follow-up series if it is
> implemented.
> 
> > 4) Show the selected frame, for details see here:
> > https://sourceware.org/pipermail/gdb-patches/2026-June/227720.html
> 
> It would be nice to show the selected frame. If the selected frame is frame 0,
> then I think it's fine to simply not show it.

So far, we don't have this feature also for the normal backtrace, so I'd go ahead without that for now. 
Dependent on which will make it earlier, I might reconsider this then. 😊

I'll have a closer look at your remaining comments tomorrow and hopefully will be able to post the next version soon.

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.