From: Pawel Kupczak Date: Tue, 17 Jun 2025 14:08:15 +0000 (+0000) Subject: gdb: correct endbr64 instruction handling in amd64_analyze_prologue X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6461dc05f1b807a50c5152528b8d0f0b4b78b5bd;p=thirdparty%2Fbinutils-gdb.git gdb: correct endbr64 instruction handling in amd64_analyze_prologue Compilers can put a sequence aligning the stack at the entry of a function. However with -fcf-protection enabled, "endbr64" is generated before. Current implementation of amd64 prologue analyzer first checks for stack alignment and then for "endbr64", which is not correct. This behavior was introduced with patch "gdb: handle endbr64 instruction in amd64_analyze_prologue". In case both are generated, prologue will not be skipped. This patch swaps the order so that "endbr64" is checked first and adds a regression test. i386-tdep implementation also already had those checked in the correct order, that is stack alignment is after endbr64. Given such source compiled with gcc 11.4.0 via: gcc -O0 main.c -o main ``` #include void foo (int id) { volatile __attribute__ ((__aligned__ (64))) int a; volatile char *p = (char *) alloca (id * 12); p[2] = 'b'; } int main (int argc, char **argv) { foo (argc + 1); return 1; } ``` we get such function entry for foo (generated with objdump -d): ``` 0000000000001149 : 1149: f3 0f 1e fa endbr64 114d: 4c 8d 54 24 08 lea 0x8(%rsp),%r10 1152: 48 83 e4 c0 and $0xffffffffffffffc0,%rsp 1156: 41 ff 72 f8 push -0x8(%r10) 115a: 55 push %rbp 115b: 48 89 e5 mov %rsp,%rbp 115e: 41 52 push %r10 1160: 48 81 ec a8 00 00 00 sub $0xa8,%rsp 1167: 89 7d 8c mov %edi,-0x74(%rbp) ... ``` The 3 instructions following endbr64 align the stack. If we were to set a breakpoint on foo, gdb would set it at function's entry: ``` (gdb) b foo Breakpoint 1 at 0x1149 (gdb) r ... Breakpoint 1, 0x0000555555555149 in foo () (gdb) disassemble Dump of assembler code for function foo: => 0x0000555555555149 <+0>: endbr64 0x000055555555514d <+4>: lea 0x8(%rsp),%r10 0x0000555555555152 <+9>: and $0xffffffffffffffc0,%rsp 0x0000555555555156 <+13>: push -0x8(%r10) 0x000055555555515a <+17>: push %rbp 0x000055555555515b <+18>: mov %rsp,%rbp 0x000055555555515e <+21>: push %r10 0x0000555555555160 <+23>: sub $0xa8,%rsp 0x0000555555555167 <+30>: mov %edi,-0x74(%rbp) ... ``` With this patch fixing the order of checked instructions, gdb can properly analyze the prologue: ``` (gdb) b foo Breakpoint 1 at 0x115e (gdb) r ... Breakpoint 1, 0x000055555555515e in foo () (gdb) disassemble Dump of assembler code for function foo: 0x0000555555555149 <+0>: endbr64 0x000055555555514d <+4>: lea 0x8(%rsp),%r10 0x0000555555555152 <+9>: and $0xffffffffffffffc0,%rsp 0x0000555555555156 <+13>: push -0x8(%r10) 0x000055555555515a <+17>: push %rbp 0x000055555555515b <+18>: mov %rsp,%rbp => 0x000055555555515e <+21>: push %r10 0x0000555555555160 <+23>: sub $0xa8,%rsp 0x0000555555555167 <+30>: mov %edi,-0x74(%rbp) ... ``` Approved-By: Andrew Burgess --- diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 44c590e38cc..7c3f5c2808a 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2581,15 +2581,16 @@ amd64_analyze_prologue (gdbarch *gdbarch, CORE_ADDR pc, CORE_ADDR current_pc, if (current_pc <= pc) return current_pc; + /* If generated, 'endbr64' will be placed before stack alignment too. */ + pc = amd64_skip_endbr (gdbarch, pc); + if (current_pc <= pc) + return current_pc; + if (gdbarch_ptr_bit (gdbarch) == 32) pc = amd64_x32_analyze_stack_align (pc, current_pc, cache); else pc = amd64_analyze_stack_align (pc, current_pc, cache); - pc = amd64_skip_endbr (gdbarch, pc); - if (current_pc <= pc) - return current_pc; - return amd64_analyze_frame_setup (gdbarch, pc, current_pc, cache); } diff --git a/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection-stackalign.c b/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection-stackalign.c new file mode 100644 index 00000000000..f55cee5c250 --- /dev/null +++ b/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection-stackalign.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + 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 . */ + +#include + +int +main (int argc, char **argv) +{ + volatile __attribute__ ((__aligned__ (64))) int a; + volatile char *p = (char *) alloca (argc * 12); + p[2] = 'b'; + return 1; +} diff --git a/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection.exp index eb931278c57..06285ce8521 100644 --- a/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection.exp +++ b/gdb/testsuite/gdb.arch/i386-prologue-skip-cf-protection.exp @@ -19,41 +19,65 @@ # This option places an `endbr32`/`endbr64` instruction at the start of # all functions, which can interfere with prologue analysis. -standard_testfile .c -set binfile ${binfile} +standard_testfile .c -stackalign.c require {is_any_target x86_64-*-* i?86-*-*} - require supports_fcf_protection -set opts {debug additional_flags=-fcf-protection=full} +# Tests if breakpoint set on main is placed past main's entry. +proc test_run {} { + # Get start address of function main. + set main_addr [get_integer_valueof &main -1] + gdb_assert {$main_addr != -1} -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $opts] != "" } { - untested "failed to compile" - return -} + set bp_addr -1 -clean_restart ${binfile} + # Put breakpoint on main, get the address where the breakpoint was installed. + gdb_test_multiple "break -q main" "break on main, get address" { + -re -wrap "Breakpoint $::decimal at ($::hex).*" { + set bp_addr $expect_out(1,string) -# Get start address of function main. -set main_addr [get_integer_valueof &main -1] -gdb_assert {$main_addr != -1} + # Convert to decimal. + set bp_addr [expr $bp_addr] -set bp_addr -1 + pass $gdb_test_name + } + } -# Put breakpoint on main, get the address where the breakpoint was installed. -gdb_test_multiple "break -q main" "break on main, get address" { - -re -wrap "Breakpoint $decimal at ($hex).*" { - set bp_addr $expect_out(1,string) + # Make sure some prologue was skipped. + gdb_assert {$bp_addr != -1 && $bp_addr > $main_addr} \ + "breakpoint placed past main's entry" +} - # Convert to decimal. - set bp_addr [expr $bp_addr] +with_test_prefix "skip-cf-protection" { + set opts {debug additional_flags=-fcf-protection=full} - pass $gdb_test_name + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \ + $opts] != "" } { + untested "failed to compile" + return } + + clean_restart ${binfile} + + test_run } -if { $bp_addr != -1 } { - # Make sure some prologue was skipped. - gdb_assert {$bp_addr > $main_addr} +# Now, make sure that the prologue analysis does not end up at function's entry +# when stack alignment sequence is generated right after 'endbr64'/'endbr32'. +# That could happen if GDB handled those incorrectly - there was a bug that +# checked for those two in incorrect order, which caused such issue. +with_test_prefix "skip-cf-protection-stackalign" { + # gcc is easier to make it produce the sequence of interest. + if { ![is_c_compiler_gcc] } { + unsupported "stackalign test part requires gcc compiler" + return + } + + if { [prepare_for_testing "failed to prepare" "${testfile}-stackalign" \ + $srcfile2 [list optimize=-O0 additional_flags=-fcf-protection=full]] } { + return + } + + test_run }