Re: [PATCH v4 07/11] gdb: Handle shadow stack pointer register unwinding for amd64 linux.

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

Hello Luis,

Luis Machado <luis.machado@arm.com> writes:

> On 6/23/25 16:00, Schimpe, Christina wrote:
>> Hi Luis, 
>> 
>> Thanks for the feedback. Please find my comments below.
>> 
>>> -----Original Message-----
>>> From: Luis Machado <luis.machado@arm.com>
>>> Sent: Thursday, June 19, 2025 11:25 AM
>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>>> patches@sourceware.org
>>> Cc: thiago.bauermann@linaro.org; eliz@gnu.org
>>> Subject: Re: [PATCH v4 07/11] gdb: Handle shadow stack pointer register
>>> unwinding for amd64 linux.
>>>
>>> On 6/17/25 13:11, Christina Schimpe wrote:
>>>> +	 Using /proc/PID/smaps we can only check if the current shadow
>>>> +	 stack pointer SSP points to shadow stack memory.  Only if this is
>>>> +	 the case a valid previous shadow stack pointer can be
>>>> +	 calculated.  */
>>>> +      std::pair<CORE_ADDR, CORE_ADDR> range;
>>>> +      if (linux_address_in_shadow_stack_mem_range (ssp, &range))
>>>> +	{
>>>> +	  /* The shadow stack grows downwards.  To compute the previous
>>>> +	     shadow stack pointer, we need to increment SSP.  */
>>>> +	  CORE_ADDR new_ssp
>>>> +	    = ssp + amd64_linux_shadow_stack_element_size_aligned (gdbarch);
>>>> +
>>>> +	  /* If NEW_SSP points to the end of or before (<=) the current
>>>> +	     shadow stack memory range we consider NEW_SSP as valid (but
>>>> +	     empty).  */
>>>
>>> I couldn't quite understand the difference between the empty case and the
>>> unavailable case. But maybe I just don't fully understand the feature.
>>>
>>> Would it be possible to make the comment a bit more clear?
>> 
>> Is this a bit clearer?
>> 
>> "There can be scenarios where we have a shadow stack pointer but the shadow stack
>> is empty, as no call instruction has been executed yet.  If NEW_SSP points to the end
>> of or before (<=) the current shadow stack memory range we consider NEW_SSP as
>> valid (but empty). "
>
> Yes, that clear it up. Thanks!
>
>> 
>> Please also see my answer to Thiago:
>> https://sourceware.org/pipermail/gdb-patches/2025-June/218908.html
>
> Thanks. It would be nice if both series were consistent in this regard (<= or <).

I just realised that AArch64 and Intel represent an empty stack
differently, so it's correct to have <= on Intel and < on AArch64:

https://inbox.sourceware.org/gdb-patches/877c12vy3h.fsf@linaro.org/

-- 
Thiago