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

Andrew Burgess
2025-07-30T14:00:31Z

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

> 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.
>
> Reviewed-By: Luis Machado <luis.machado@arm.com>
> 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 | 92 +++++++++++++++++++
> 6 files changed, 149 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 ba555f0dea1..60510fefea4 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.
> +
> * 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 899fe2df02c..782b66f1467 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1936,8 +1936,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)
> @@ -1955,6 +1957,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;
> }
>
> @@ -1965,8 +1970,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);
> +
> + /* For amd64/Linux, if SSP has a value that means shadow stack is
> + enabled. */

It feels like this statement should be associated with an assert. I'd
suggest:

if (!ssp.has_value ())
return;
else
gdb_assert (shadow_stack_enabled);

> if (!ssp.has_value ())
> return;
>
> @@ -2122,6 +2132,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 450dbc38047..8afb3a7abba 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)

Given the strengthening of the comment on
gdbarch_get_shadow_stack_pointer that I suggest in the previous patch, I
think this should become:

if (shadow_stack_enabled)
{
gdb_assert (ssp.has_value ());

... etc ...
}

> + {
> + 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 b5120b78426..488816d5ca2 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -27059,12 +27059,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 to the shadow
> +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
> @@ -41741,6 +41749,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 8eb5b4fac86..3b05ace2142 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)

Same comment here as for amd64.

> + {
> + 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..47bb4df8cfe
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-disp-step.exp
> @@ -0,0 +1,92 @@
> +# Copyright 2025 Free Software Foundation, Inc.
> +
> +# 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

All of the 'return -1' in the global scope should just be 'return'. The
-1 is not checked or used.

> + }
> +
> + # 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.

I think you mean:

# Get the address of the call to the call1 function.

> + 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.

As above:

# Get the address of the call to the call2 function.

> + 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"
> +
> + # Depending on instruction generation we might end up in the call
> + # instruction after "runto_main". Only resume until call1 instruction
> + # in case the first instruction we're stopped at is not yet the call1
> + # instruction.

Could you not just add some filler to the test program to avoid needing
to do this? I don't really object (to this) but using filler might be
simpler.

> + set stop_addr [get_valueof "/x" "\$pc" "" "value of pc after runto_main"]
> + if {[eval expr "$stop_addr < $call1_addr"]} {

Is the 'eval expr' really needed here? There are plenty of the places
in the testsuite where we just write something like:

if { $stop_addr < $call1_addr } { ... }

so I would have expected that to work.

Thanks,
Andrew

> + gdb_test "continue" \
> + "Breakpoint $decimal, $call1_addr in main ().*" \
> + "continue until call1 instruction"
> + }
> + 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"
> +}
> --
> 2.43.0