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

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

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. 

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

-- 
Thiago
(he/him)