Re: [PATCH v4 11/11] gdb: Enable displaced stepping with shadow stack on amd64 linux.

Luis Machado
2025-06-24T08:07:10Z

On 6/23/25 19:24, Schimpe, Christina wrote:
> Hi Luis,
>
> Please find my comments below.
>
>> -----Original Message-----
>> From: Luis Machado <luis.machado@arm.com>
>> Sent: Thursday, June 19, 2025 11:27 AM
>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>> patches@sourceware.org
>> Cc: thiago.bauermann@linaro.org; eliz@gnu.org
>> Subject: Re: [PATCH v4 11/11] gdb: Enable displaced stepping with shadow stack
>> on amd64 linux.
>>
>> On 6/17/25 13:11, Christina Schimpe wrote:
>>> This patch enables displaced stepping to support Intel's Control-Flow
>>> Enforcement Technology (CET), which provides the shadow stack feature
>>> for the x86 architecture.
>>> Following the restriction of the linux kernel, enable displaced
>>> stepping for amd64 only.
>>>
>>> If displaced stepping is active and the single stepped instruction is
>>> a call instruction, the return address atop the stack is the address
>>> following the copied instruction. However, to allow normal program
>>> execution it has to be the address following the original instruction.
>>> Due to that reason, the return address is corrected in
>>> amd64_displaced_step_fixup and i386_displaced_step_fixup.
>>
>> I thought the description was slightly confusing. The above is the original
>> behavior, right?
>
> Yes.
>
>>>
>>> To avoid a control-protection exception if shadow stack is active, the
>>> shadow stack top address must be corrected as well.
>>
>> And this one is the new behavior?
>>
>> Might make sense to clarify it.
>
> I agree, does this sound better?
>
> [...], enable displaced stepping for amd64 only.
>
> Currently, if displaced stepping is active and the single stepped instruction is
> a call instruction, the return address atop the stack is the address following
> the copied instruction. However, to allow normal program execution it has
> to be the address following the original instruction. Due to that reason,
> the return address is corrected in amd64_displaced_step_fixup and
> i386_displaced_step_fixup.
>
> For programs that are shadow-stack enabled we see a control-protection
> exception, as the address on the shadow stack does not match the address
> atop the stack.
>
> Fix this by correcting the shadow stack top address as well.
>

That makes it better. Thanks!

>>
>>>
>>> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>>> ---
>>> gdb/NEWS | 3 +
>>> gdb/amd64-linux-tdep.c | 16 +++-
>>> gdb/amd64-tdep.c | 15 ++++
>>> gdb/doc/gdb.texinfo | 11 ++-
>>> gdb/i386-tdep.c | 15 ++++
>>> .../gdb.arch/amd64-shadow-stack-disp-step.exp | 90
>>> +++++++++++++++++++
>>> 6 files changed, 147 insertions(+), 3 deletions(-) create mode
>>> 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-disp-step.exp
>>>
>>> diff --git a/gdb/NEWS b/gdb/NEWS
>>> index b8fb7f0e484..15d2772230e 100644
>>> --- a/gdb/NEWS
>>> +++ b/gdb/NEWS
>>> @@ -3,6 +3,9 @@
>>>
>>> *** Changes since GDB 16
>>>
>>> +* Debugging Linux programs that use x86-64 or x86-64 with 32-bit
>>> +pointer
>>> + size (X32) Shadow Stacks are now supported.
>>
>> Should we mention CET somewhere?
>
> Hm, in theory, this should work for AMD64 in general. But I don't know about
> the state of AMD's shadow stack and am not sure how I could explain this in the
> NEWS properly.
>

Alright. This is OK then.

>>
>>> +
>>> * Support for the shadow stack pointer register on x86-64 or x86-64 with
>>> 32-bit pointer size (X32) GNU/Linux.
>>>
>>> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index
>>> d847248659a..f989cfb3bf8 100644
>>> --- a/gdb/amd64-linux-tdep.c
>>> +++ b/gdb/amd64-linux-tdep.c
>>> @@ -1935,8 +1935,10 @@
>> amd64_linux_shadow_stack_element_size_aligned (gdbarch *gdbarch)
>>> possible. */
>>>
>>> static std::optional<CORE_ADDR>
>>> -amd64_linux_get_shadow_stack_pointer (gdbarch *gdbarch, regcache
>>> *regcache)
>>> +amd64_linux_get_shadow_stack_pointer (gdbarch *gdbarch, regcache
>> *regcache,
>>> + bool &shadow_stack_enabled)
>>> {
>>> + shadow_stack_enabled = false;
>>> const i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep>
>>> (gdbarch);
>>>
>>> if (tdep == nullptr || tdep->ssp_regnum < 0) @@ -1954,6 +1956,9 @@
>>> amd64_linux_get_shadow_stack_pointer (gdbarch *gdbarch, regcache
>> *regcache)
>>> if (ssp == 0x0)
>>> return {};
>>>
>>> + /* In case there is a shadow stack pointer available which is non-null,
>>> + the shadow stack feature is enabled. */ shadow_stack_enabled =
>>> + true;
>>> return ssp;
>>> }
>>>
>>> @@ -1964,8 +1969,13 @@ static void
>>> amd64_linux_shadow_stack_push (gdbarch *gdbarch, CORE_ADDR
>> new_addr,
>>> regcache *regcache)
>>> {
>>> + bool shadow_stack_enabled;
>>> std::optional<CORE_ADDR> ssp
>>> - = amd64_linux_get_shadow_stack_pointer (gdbarch, regcache);
>>> + = amd64_linux_get_shadow_stack_pointer (gdbarch, regcache,
>>> + shadow_stack_enabled);
>>> +
>>> + /* It's enough to check if SSP is valid as for amd64 linux shadow stack
>>> + is always enabled if SSP has a value. */
>>
>> Is my understanding correct that for amd64's shadow stack support, whenever
>> SSP has a value, then shadow stack is enabled?
>
> Yes, exactly.
>
>>
>> If so, maybe rephrase it as...
>>
>> "For amd64/Linux, if SSP has a value that means shadow stack is enabled."
>>
>> What do you think?
>
> Yes, this makes it clearer. Thank you.
>
>>> if (!ssp.has_value ())
>>> return;
>>>
>>> @@ -2121,6 +2131,8 @@ amd64_linux_init_abi_common(struct
>> gdbarch_info info, struct gdbarch *gdbarch,
>>> (gdbarch, amd64_linux_remove_non_address_bits_watchpoint);
>>>
>>> set_gdbarch_shadow_stack_push (gdbarch,
>>> amd64_linux_shadow_stack_push);
>>> + set_gdbarch_get_shadow_stack_pointer (gdbarch,
>>> +
>> amd64_linux_get_shadow_stack_pointer);
>>> dwarf2_frame_set_init_reg (gdbarch, amd64_init_reg); }
>>>
>>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index
>>> 79f7e427841..6c54957ae75 100644
>>> --- a/gdb/amd64-tdep.c
>>> +++ b/gdb/amd64-tdep.c
>>> @@ -1917,6 +1917,21 @@ amd64_displaced_step_fixup (struct gdbarch
>> *gdbarch,
>>> displaced_debug_printf ("relocated return addr at %s to %s",
>>> paddress (gdbarch, rsp),
>>> paddress (gdbarch, retaddr));
>>> +
>>> + /* If shadow stack is enabled, we need to correct the return address
>>> + on the shadow stack too. */
>>> + bool shadow_stack_enabled;
>>> + std::optional<CORE_ADDR> ssp
>>> + = gdbarch_get_shadow_stack_pointer (gdbarch, regs,
>>> + shadow_stack_enabled);
>>> + if (ssp.has_value () && shadow_stack_enabled)
>>> + {
>>> + write_memory_unsigned_integer (*ssp, retaddr_len, byte_order,
>>> + retaddr);
>>> + displaced_debug_printf ("relocated shadow stack return addr at %s "
>>> + "to %s", paddress (gdbarch, *ssp),
>>> + paddress (gdbarch, retaddr));
>>> + }
>>> }
>>> }
>>>
>>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index
>>> cf152bd1e6f..589fd50345f 100644
>>> --- a/gdb/doc/gdb.texinfo
>>> +++ b/gdb/doc/gdb.texinfo
>>> @@ -27055,12 +27055,20 @@ the program stream must be an
>> @code{ENDBR}
>>> instruction, otherwise the processor signals a control protection exception.
>>> @end itemize
>>>
>>> -Impact on Call/Print:
>>> +Impact on GDB commands:
>>> +@itemize @bullet
>>> +@item Call/Print:
>>> Inferior calls in @value{GDBN} reset the current PC to the beginning
>>> of the function that is called. No call instruction is executed, but
>>> the @code{RET} instruction actually is. To avoid a control
>>> protection exception due to the missing return address on the shadow
>>> stack, @value{GDBN} pushes the new return address to the shadow stack and
>> updates the shadow stack pointer.
>>> +@item Step:
>>> +With displaced stepping, @value{GDBN} may run an out of line copy of
>>> +a call instruction. In this case, the wrong return address is pushed
>>> +on the shadow
>>
>> s/pushed on/pushed to
>
> Will fix.
>
>>
>>> +stack. @value{GDBN} corrects this value to avoid a control
>>> +protection exception. For more details on displaced stepping, see
>> @ref{displaced-stepping}.
>>> +@end itemize
>>>
>>> @node Alpha
>>> @subsection Alpha
>>> @@ -41736,6 +41744,7 @@ GLOBAL Disassembler_2 (Matches
>> current architecture)
>>> @cindex out-of-line single-stepping
>>> @item set displaced-stepping
>>> @itemx show displaced-stepping
>>> +@anchor{displaced-stepping}
>>> Control whether or not @value{GDBN} will do @dfn{displaced stepping}
>>> if the target supports it. Displaced stepping is a way to single-step
>>> over breakpoints without removing them from the inferior, by executing
>>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index
>>> f3fa4e511e6..d83fdc0c85e 100644
>>> --- a/gdb/i386-tdep.c
>>> +++ b/gdb/i386-tdep.c
>>> @@ -899,6 +899,21 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
>>> displaced_debug_printf ("relocated return addr at %s to %s",
>>> paddress (gdbarch, esp),
>>> paddress (gdbarch, retaddr));
>>> +
>>> + /* If shadow stack is enabled, we need to correct the return address
>>> + on the shadow stack too. */
>>> + bool shadow_stack_enabled;
>>> + std::optional<CORE_ADDR> ssp
>>> + = gdbarch_get_shadow_stack_pointer (gdbarch, regs,
>>> + shadow_stack_enabled);
>>> + if (ssp.has_value () && shadow_stack_enabled)
>>> + {
>>> + write_memory_unsigned_integer (*ssp, retaddr_len, byte_order,
>>> + retaddr);
>>> + displaced_debug_printf ("relocated shadow stack return addr at %s "
>>> + "to %s", paddress (gdbarch, *ssp),
>>> + paddress (gdbarch, retaddr));
>>> + }
>>> }
>>> }
>>>
>>> diff --git a/gdb/testsuite/gdb.arch/amd64-shadow-stack-disp-step.exp
>>> b/gdb/testsuite/gdb.arch/amd64-shadow-stack-disp-step.exp
>>> new file mode 100644
>>> index 00000000000..b5f168c2c42
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-disp-step.exp
>>> @@ -0,0 +1,90 @@
>>> +# Copyright 2024 Free Software Foundation, Inc.
>>
>> s/2024/2025
>
> Will fix.
>
>>> +
>>> +# This program is free software; you can redistribute it and/or
>>> +modify # it under the terms of the GNU General Public License as
>>> +published by # the Free Software Foundation; either version 3 of the
>>> +License, or # (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful, #
>>> +but WITHOUT ANY WARRANTY; without even the implied warranty of #
>>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU
>>> +General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License #
>>> +along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Test continue from call instructions with shadow stack and
>>> +displaced # stepping being enabled.
>>> +
>>> +require allow_ssp_tests support_displaced_stepping
>>> +
>>> +standard_testfile amd64-shadow-stack.c
>>> +
>>> +save_vars { ::env(GLIBC_TUNABLES) } {
>>> +
>>> + append_environment GLIBC_TUNABLES "glibc.cpu.hwcaps" "SHSTK"
>>> +
>>> + if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
>>> + additional_flags="-fcf-protection=return"] } {
>>> + return -1
>>> + }
>>> +
>>> + # Enable displaced stepping.
>>> + gdb_test_no_output "set displaced-stepping on"
>>> + gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*"
>>> +
>>> + if { ![runto_main] } {
>>> + return -1
>>> + }
>>> +
>>> + # Get the address of the call1 instruction.
>>> + set call1_addr -1
>>> + gdb_test_multiple "disassemble main" "" {
>>> + -re -wrap "($hex) <\\+($decimal)>:\\s*call\\s*0x.*<call1>.*" {
>>> + set call1_addr $expect_out(1,string)
>>> + pass $gdb_test_name
>>> + }
>>> + }
>>> +
>>> + if { $call1_addr == -1 } {
>>> + return -1
>>> + }
>>> +
>>> + # Get the address of the call2 instruction.
>>> + set call2_addr -1
>>> + gdb_test_multiple "disassemble call1" "" {
>>> + -re -wrap "($hex) <\\+($decimal)>:\\s*call\\s*0x.*<call2>.*" {
>>> + set call2_addr $expect_out(1,string)
>>> + pass $gdb_test_name
>>> + }
>>> + }
>>> +
>>> + if { $call2_addr == -1 } {
>>> + return -1
>>> + }
>>> +
>>> + gdb_test "break *$call1_addr" \
>>> + "Breakpoint $decimal at $hex.*" \
>>> + "break at the address of the call1 instruction"
>>> +
>>> + gdb_test "break *$call2_addr" \
>>> + "Breakpoint $decimal at $hex.*" \
>>> + "break at the address of the call2 instruction"
>>> +
>>> + # We only resume until call1 instruction in case the first instruction
>>> + # we're stopped at is not yet the call1 instruction.
>>> + set stop_addr [get_valueof "/x" "\$pc" "" "value of pc after runto_main"]
>>> + if {[eval expr "$stop_addr < $call1_addr"]} {
>>> + gdb_test "continue" \
>>> + "Breakpoint $decimal, $call1_addr in main ().*" \
>>> + "continue until call1 instruction"
>>> + }
>>
>> It was particularly clear why we need the check above. Is this due to how the
>> compiler might generate code and then we could risk stopping at the
>> instruction we're interested in when we "run to main"?
>
> I assume you mean "It was not clear" in
> "It was particularly clear why we need the check above." :

Yes, sorry. I messed that one up.

>
> Yes, this is related to compiler behaviour.
> With gcc 15 the assembler changed a bit and with "run to main" we stopped already
> at the call instruction.
>
> I agree, the comment might be confusing. Would such a comment be better?
>
> # For gcc 15 we already stop at the call instruction when we "run to "main".
> # Only resume in case the first instruction we're stopped at is not yet the call
> # instruction.
>

I think it is enough to mention that depending on instruction generation we might end up
in the call instruction after "runto_main". Then we don't make any references to versions,
since they change in behavior sometimes.

>>
>>> + gdb_assert {$call1_addr == [get_valueof "/x" "\$pc" ""]}
>>> +
>>> + # Test continue from breakpoint at call1 and call2 instructions.
>>> + gdb_test "continue" \
>>> + "Breakpoint $decimal, $call2_addr in call1 ().*" \
>>> + "continue from call1 instruction"
>>> +
>>> + gdb_continue_to_end "continue from call2 instruction"
>>> +}
>>
>> Reviewed-By: Luis Machado <luis.machado@arm.com>
>
> Thanks for all the review efforts!
> Christina
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928