Re: [PATCH 7/9] gdb: Provide gdbarch hook to distinguish shadow stack backtrace elements.

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

Hello Christina,

"Schimpe, Christina" <christina.schimpe@intel.com> writes:

>> -----Original Message-----
>> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>> Sent: Sonntag, 2. November 2025 22:20
>> To: Schimpe, Christina <christina.schimpe@intel.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>; gdb-patches@sourceware.org
>> Subject: Re: [PATCH 7/9] gdb: Provide gdbarch hook to distinguish shadow
>> stack backtrace elements.
>> 
>> "Schimpe, Christina" <christina.schimpe@intel.com> writes:
>> 
>> >> -----Original Message-----
>> >> From: Eli Zaretskii <eliz@gnu.org>
>> >> Sent: Tuesday, September 23, 2025 1:50 PM
>> >> To: Schimpe, Christina <christina.schimpe@intel.com>
>> >> Cc: gdb-patches@sourceware.org
>> >> Subject: Re: [PATCH 7/9] gdb: Provide gdbarch hook to distinguish
>> >> shadow stack backtrace elements.
>> >>
>> >> > From: Christina Schimpe <christina.schimpe@intel.com>
>> >> > Date: Tue, 23 Sep 2025 11:18:40 +0000
>> >> >
>> >> > On x86 with CET there can be elements on the shadow stack which are
>> >> > not return addresses.  In this case, we just want to print the
>> >> > element itself in the shadow stack backtrace, but no further information.
>> >> >
>> >> > Provide a gdbarch hook to distinguish between return and non-return
>> >> > addresses and use it to print the shadow stack backtrace as
>> >> > described above.
>> >> > ---
>> >> >  gdb/doc/gdb.texinfo       | 19 ++++++++++++
>> >> >  gdb/gdbarch-gen.c         | 32 ++++++++++++++++++++
>> >> >  gdb/gdbarch-gen.h         | 15 +++++++++
>> >> >  gdb/gdbarch.h             |  1 +
>> >> >  gdb/gdbarch_components.py | 17 +++++++++++
>> >> >  gdb/shadow-stack.c        | 64 +++++++++++++++++----------------------
>> >> >  gdb/shadow-stack.h        | 37 ++++++++++++++++++++++
>> >> >  7 files changed, 148 insertions(+), 37 deletions(-)
>> >>
>> >> Thanks.
>> >>
>> >> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index
>> >> > ebda4546b58..a0fde385a8e 100644
>> >> > --- a/gdb/doc/gdb.texinfo
>> >> > +++ b/gdb/doc/gdb.texinfo
>> >> > @@ -8887,6 +8887,25 @@ This is how a shadow stack backtrace looks
>> >> > like
>> >> on amd64:
>> >> >  @end group
>> >> >  @end smallexample
>> >> >
>> >> > +There can be elements on the shadow stack which are not return
>> >> > +addresses, for example on x86 with the Intel Control-Flow
>> >> > +Enforcement Technology (@xref{CET}).  In case of signals, the old
>> >> > +shadow stack pointer is pushed
>> >>                ^
>> >> A cross-reference is missing here.
>> >
>> > Thanks will fix.
>> >
>> >> > +in a special format with bit 63 set.  For such shadow stack
>> >> > +elements, the shadow stack frame just contains the level and the
>> >> > +address on the shadow stack, as shown in the following example by frame 1:
>> >> > +
>> >> > +@smallexample
>> >> > +@group
>> >> > +(gdb) bt shadow 4
>> >> > +#0  0x00007ffff7c54d90 in __restore_rt from /lib64/libc.so.6
>> >> > +#1  0x80007ffff79fffd8
>> >> > +#2  0x00007ffff7c54ce6 in __GI_raise at
>> >> > +../sysdeps/posix/raise.c:27
>> >> > +#3  0x000000000040115d in main at /tmp/amd64-shadow-stack-signal.c:32
>> >> > +(More shadow stack frames follow...) @end group @end smallexample
>> >>
>> >> Would it make sense to show something like "<signal caught>", instead
>> >> of a frame with only an address?
>> >
>> > Yeah, this is a good idea, I wondered about a similar thing actually
>> > but wanted to discuss the general direction for handling those specific
>> > elements on the shadow stack first.
>> > Maybe there are more options on other architectures that we have to
>> > consider.
>> > Let's wait for more feedback on this, I added Thiago again in cc here.
>> 
>> Yes, I would like to show a "<signal handler called>" message in the
>> corresponding shadow stack entry, as is done in the regular stack:
>> 
>>   (gdb) bt
>>   #0  handler (signo=10) at aarch64-gcs-signal.c:59
>>   #1  <signal handler called>
>>   #2  __pthread_kill_implementation (threadid=281474842447584,
>> signo=signo@entry=10, no_tid=no_tid@entry=0)
>>       at ./nptl/pthread_kill.c:44
>>   #3  0x0000fffff7e77670 in __pthread_kill_internal (signo=10,
>> threadid=<optimized out>) at ./nptl/pthread_kill.c:78
>>   #4  0x0000fffff7e2cb3c in __GI_raise (sig=10) at ../sysdeps/posix/raise.c:26
>>   #5  0x0000aaaaaaaa0b24 in main () at aarch64-gcs-signal.c:96
>> 
>> Here is the corresponding GCS backtrace:
>> 
>>   (gdb) bt shadow
>>   #0  0x0000fffff7ffa85c in __kernel_rt_sigreturn
>>   #1  0x0000fffff7bff000 in ??
>>   #2  0x0000fffff7e2cb3c in __GI_raise at ../sysdeps/posix/raise.c:27
>>   #3  0x0000aaaaaaaa0b24 in main at aarch64-gcs-signal.c:99
>> 
>> In this example "<signal handler called>" message would replace the VDSO
>> function name in entry #0. Also note that it's an actual return address but we
>> still want to show a custom message for it.
>> Finally, notice that there's GCS entry #1, which isn't a return address. It's a "cap
>> record", which marks that the stack is currently not in use. When a signal is
>> delivered, the kernel pushes two entries to the thread's GCS: first the cap
>> record, then the address to the signal trampoline (__kernel_rt_sigreturn). For
>> this entry, I'd like to show "<cap record>".
>> One suggestion to handle that is to make do_print_shadow_stack_frame_info
>> call a new hook gdbarch_print_shadow_stack_frame_info which would allow
>> the architecture to print the shadow stack frame however it wants. If the hook
>> returns false, then do_print_shadow_stack_frame_info would print the
>> information itself, as it does in your patch. What do you think?
>
> If possible, I'd like to keep it as generic as possible. The <signal handler called>
> message is useful for CET, too. The CET equivalent for "cap record" should be "sigframe
> token".
>
> |1...old SSP| - Pointer to old pre-signal ssp in sigframe token format
>                 (bit 63 set to 1)
> (https://docs.kernel.org/next/x86/shstk.html)

Agreed, it should be as generic as possible.

> In the commit message I described that "bt shadow" always prints the address:
>
> "Similar to the ordinary backtrace command 'bt shadow' can be configured
> using COUNT and the command line option -frame-info.  However, we always
> print the address and the command is not affected by the setting
> "print address" as well as the setting "print frame-info location-and-address".
> Also we do not print the frame arguments."
>
> Do you think we should align with the ordinary backtrace for the printing of shadow stack
> addresses ?
> When implementing this I thought it would be nice to always print addresses, but now I am
> not so sure anymore.

I understand why it makes sense to ignore the "print address" setting:
essentially the only things the shadow stack contains are return
addresses, and we're not going to print them? My inclination is to agree
with always printing them.

But thinking a bit more about it, the frame information that GDB prints
is good even if the address isn't printed along with it so I think it's
better to respect the setting. Also it's good to be consistent with the
regular backtrace output.

> Based on your suggestion we could print it as follows and replace the shadow stack
> addresses:
>
> (gdb) bt shadow
> #0  <signal handler called>
> #1  <sigframe token >
> #2  0x00007ffff7c4527e in __GI_raise at ../sysdeps/posix/raise.c:27
> #3  0x0000555555555175 in main at /tmp/amd64-shadow-stack-signal.c:30
> #4 0x00007ffff7c2a1ca in __libc_start_call_main at
> ../sysdeps/nptl/libc_start_call_main.h:74
> #5  0x00007ffff7c2a28b in __libc_start_main_impl at ../csu/libc-start.c:128
> #6  0x0000555555555085 in _start
>
> For GCS we show "cap record" instead of "sigframe token".

I agree. I like the output above.

My only comment is that there a also the "set backtrace past-main" and
"set backtrace past-entry" options, which "bt shadow" should also
respect.  So if the former setting is on, the output above should stop
at frame #3.

> In case the gdbarch hook "is_no_return_address" returns true, we could additionally return
> a string,
> which should be displayed inside the <...>. So in our case this would be "sigframe token"
> or "cap record" .
>
> What do you think?

I think that is a good idea, and should be enough for GCS.

-- 
Thiago