Re: [PATCH v4 10/13] gdb: Implement the hook 'is_no_return_shadow_stack_address' for amd64 linux.

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

Hello Christina,

"Joos, Christina" <christina.joos@intel.com> writes:

>> -----Original Message-----
>> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>> Sent: Donnerstag, 3. September 2026 09:50
>> To: Joos, Christina <christina.joos@intel.com>
>> Cc: gdb-patches@sourceware.org; tom@tromey.com
>> Subject: Re: [PATCH v4 10/13] gdb: Implement the hook
>> 'is_no_return_shadow_stack_address' for amd64 linux.
>> 
>> Christina Schimpe <christina.schimpe@intel.com> writes:
>> 
>> > diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index
>> > 42a62eaa975..d98415b8989 100644
>> > --- a/gdb/amd64-linux-tdep.c
>> > +++ b/gdb/amd64-linux-tdep.c
>> > @@ -1960,6 +1960,62 @@ amd64_linux_top_addr_empty_shadow_stack
>> >    return addr == range.second;
>> >  }
>> >
>> > +/* Return a shadow stack frame info, if the shadow stack pointer SSP
>> > +   belongs to a valid shadow stack frame while the element on the shadow
>> > +   stack VALUE does not refer to a return address.  This can happen, for
>> > +   instance, in case of signals.  The old shadow stack pointer is pushed
>> > +   in a special format with bit 63 set.  In case this is true, a valid
>> > +   shadow stack frame info is returned with its attributes frame_type and
>> > +   non_return_description configured to ssp_frame_type::non_return_frame
>> > +   and "<sigframe token>", respectively.  */
>> > +
>> > +static std::optional<shadow_stack_frame_info>
>> > +amd64_linux_is_no_return_shadow_stack_address
>> > +  (gdbarch *gdbarch, const CORE_ADDR ssp, const CORE_ADDR value,
>> > +   const unsigned long level)
>> > +{
>> > +  /* SSP must belong to the shadow stack memory range.  */
>> > +  std::pair<CORE_ADDR, CORE_ADDR> range;
>> > +  gdb_assert (gdbarch_address_in_shadow_stack_memory_range (gdbarch,
>> > +							    ssp,
>> > +							    &range));
>> 
>> I made this comment at another assert in a previous version of this series, but
>> it's valid here too:
>
> Hm, I don't remember it... :/ Did I fix it already ? Or can you point me to your comment?

It was during review of v2:

https://inbox.sourceware.org/gdb-patches/87bjh1y3xk.fsf@linaro.org/

The assert was in amd64_linux_get_shadow_stack_size, but that gdbarch
hook doesn't exist anymore.

Also, during review of a different patch series, Andrew Burgess said¹:

> We shouldn't assert on data from an outside source.  This should be
> either an error, or a warning if GDB is able to handle this and push
> on.

Which I think is a similar concern. Though in that case the outside
source was a /proc file, not inferior state.

>> Won't this make GDB crash with a misbehaving inferior? 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.
>> 
>> Maybe turn this into an error ()?
>
> Do you have an example program/debug session for this?
> Then it would be easier for me to write a meaningful error message.

I don't. While I read the patch I just wondered whether this assert
could trigger in a situation where the inferior is in a crazy or
corrupted state. <hand-wave>Maybe it tried to do some stack-switching
trick and got it wrong?</hand-wave>

An error message could be direct, maybe something like "SSP doesn't
point to a shadow stack memory region"...

-- 
Thiago
(he/him)

¹ https://inbox.sourceware.org/gdb-patches/87a4ppdq5a.fsf@redhat.com/