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

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

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

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

I understood it to mean that if new_ssp == range.second, then it points
to the top of the stack and there aren't any elements.

Whereas if new_ssp points outside of the shadow stack area, then it's
garbage and we failed to unwind it, hence the unavailable value.

Christina, please correct me if I'm wrong.

But now looking at this again, I think there's an off-by-one error:
range.second is the first address outside of the memory range, so the
comparison needs to be strictly less than. And the shadow stack will be
empty if new_ssp == range.second - 1 (there's no need to check for that,
though).

>> +	  if (new_ssp <= range.second)
>> +	    return frame_unwind_got_address (this_frame, regnum, new_ssp);
>> +	}
>> +    }
>> +
>> +  /* Return a value which is marked as unavailable in case we could not
>> +     calculate a valid previous shadow stack pointer.  */
>> +  value *retval
>> +    = value::allocate_register (get_next_frame_sentinel_okay (this_frame),
>> +				regnum, register_type (gdbarch, regnum));
>> +  retval->mark_bytes_unavailable (0, retval->type ()->length ());
>> +  return retval;
>> +}

-- 
Thiago