* [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue @ 2020-05-01 5:28 Simon Marchi 2020-05-01 9:52 ` Tom de Vries 0 siblings, 1 reply; 4+ messages in thread From: Simon Marchi @ 2020-05-01 5:28 UTC (permalink / raw) To: gdb-patches; +Cc: Simon Marchi Some GCCs now enable -fcf-protection by default. This is the case, for example, with GCC 9.3.0 on Ubuntu 20.04. Enabling it causes the `endbr64` instruction to be inserted at the beginning of all functions and that breaks GDB's prologue analysis. I noticed this because it gives many failures in gdb.base/break.exp. But let's take this dummy program and put a breakpoint on main: int main(void) { return 0; } Without -fcf-protection, the breakpoint is correctly put after the prologue: $ gcc test.c -g3 -O0 -fcf-protection=none $ ./gdb -q -nx --data-directory=data-directory a.out Reading symbols from a.out... (gdb) disassemble main Dump of assembler code for function main: 0x0000000000001129 <+0>: push %rbp 0x000000000000112a <+1>: mov %rsp,%rbp 0x000000000000112d <+4>: mov $0x0,%eax 0x0000000000001132 <+9>: pop %rbp 0x0000000000001133 <+10>: retq End of assembler dump. (gdb) b main Breakpoint 1 at 0x112d: file test.c, line 3. With -fcf-protection, the breakpoint is incorrectly put on the first byte of the function: $ gcc test.c -g3 -O0 -fcf-protection=full $ ./gdb -q -nx --data-directory=data-directory a.out Reading symbols from a.out... (gdb) disassemble main Dump of assembler code for function main: 0x0000000000001129 <+0>: endbr64 0x000000000000112d <+4>: push %rbp 0x000000000000112e <+5>: mov %rsp,%rbp 0x0000000000001131 <+8>: mov $0x0,%eax 0x0000000000001136 <+13>: pop %rbp 0x0000000000001137 <+14>: retq End of assembler dump. (gdb) b main Breakpoint 1 at 0x1129: file test.c, line 2. Stepping in amd64_skip_prologue, we can see that the prologue analysis, for GCC-compiled programs, is done in amd64_analyze_prologue by decoding the instructions and looking for typical patterns. This patch changes the analysis to check for a prologue starting with the `endbr64` instruction, and skip it if it's there. gdb/ChangeLog: * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` instruction, skip it if it's there. gdb/testsuite/ChangeLog: * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. --- gdb/amd64-tdep.c | 19 +++++++ .../amd64-prologue-skip-cf-protection.c | 21 +++++++ .../amd64-prologue-skip-cf-protection.exp | 56 +++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 5c56a970d8c..c846447a8e0 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2362,6 +2362,9 @@ amd64_x32_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, pushq %rbp 0x55 movl %esp, %ebp 0x89 0xe5 (or 0x8b 0xec) + The `endbr64` instruction can be found before these sequences, and will be + skipped if found. + Any function that doesn't start with one of these sequences will be assumed to have no prologue and thus no valid frame pointer in %rbp. */ @@ -2372,6 +2375,8 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, struct amd64_frame_cache *cache) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + /* The `endbr64` instruction. */ + static const gdb_byte endbr64[4] = { 0xf3, 0x0f, 0x1e, 0xfa }; /* There are two variations of movq %rsp, %rbp. */ static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 }; static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec }; @@ -2392,6 +2397,20 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, op = read_code_unsigned_integer (pc, 1, byte_order); + /* Check for the `endbr64` instruction, skip it if found. */ + if (op == endbr64[0]) + { + read_code (pc + 1, buf, 3); + + if (memcmp (buf, &endbr64[1], 3) == 0) + pc += 4; + + op = read_code_unsigned_integer (pc, 1, byte_order); + } + + if (current_pc <= pc) + return current_pc; + if (op == 0x55) /* pushq %rbp */ { /* Take into account that we've executed the `pushq %rbp' that diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c new file mode 100644 index 00000000000..968f1832d8c --- /dev/null +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c @@ -0,0 +1,21 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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/>. */ + +int foo (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp new file mode 100644 index 00000000000..c3348054ae0 --- /dev/null +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp @@ -0,0 +1,56 @@ +# Copyright 2020 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 skipping a prologue that was generated with gcc's -fcf-protection=full +# (control flow protection) option. +# +# This option places an `endbr64` instruction at the start of all functions, +# which can interfere with prologue analysis. + +standard_testfile .c +set binfile ${binfile}.o + +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { + verbose "Skipping ${testfile}." + return +} + +set opts {debug additional_flags=-fcf-protection=full} + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object $opts] != "" } { + untested "failed to compile" + return +} + +clean_restart ${binfile} + +# Get start address of function foo. +set foo_addr [get_integer_valueof &foo -1] +gdb_assert {$foo_addr != -1} + +# Put breakpoint on foo, get the address where the breakpoint was installed. +gdb_test_multiple "break foo" "break on foo, get address" { + -re "Breakpoint $decimal at ($hex)" { + set bp_addr $expect_out(1,string) + + # Convert to decimal. + set bp_addr [expr $bp_addr] + + pass $gdb_test_name + } +} + +# Make sure some prologue was skipped. +gdb_assert {$bp_addr > $foo_addr} -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue 2020-05-01 5:28 [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue Simon Marchi @ 2020-05-01 9:52 ` Tom de Vries 2020-05-01 14:22 ` Simon Marchi 0 siblings, 1 reply; 4+ messages in thread From: Tom de Vries @ 2020-05-01 9:52 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 01-05-2020 07:28, Simon Marchi via Gdb-patches wrote: > Some GCCs now enable -fcf-protection by default. This is the case, for > example, with GCC 9.3.0 on Ubuntu 20.04. Enabling it causes the > `endbr64` instruction to be inserted at the beginning of all functions > and that breaks GDB's prologue analysis. > > I noticed this because it gives many failures in gdb.base/break.exp. > But let's take this dummy program and put a breakpoint on main: > > int main(void) > { > return 0; > } > > Without -fcf-protection, the breakpoint is correctly put after the prologue: > > $ gcc test.c -g3 -O0 -fcf-protection=none > $ ./gdb -q -nx --data-directory=data-directory a.out > Reading symbols from a.out... > (gdb) disassemble main > Dump of assembler code for function main: > 0x0000000000001129 <+0>: push %rbp > 0x000000000000112a <+1>: mov %rsp,%rbp > 0x000000000000112d <+4>: mov $0x0,%eax > 0x0000000000001132 <+9>: pop %rbp > 0x0000000000001133 <+10>: retq > End of assembler dump. > (gdb) b main > Breakpoint 1 at 0x112d: file test.c, line 3. > > With -fcf-protection, the breakpoint is incorrectly put on the first > byte of the function: > > $ gcc test.c -g3 -O0 -fcf-protection=full > $ ./gdb -q -nx --data-directory=data-directory a.out > Reading symbols from a.out... > (gdb) disassemble main > Dump of assembler code for function main: > 0x0000000000001129 <+0>: endbr64 > 0x000000000000112d <+4>: push %rbp > 0x000000000000112e <+5>: mov %rsp,%rbp > 0x0000000000001131 <+8>: mov $0x0,%eax > 0x0000000000001136 <+13>: pop %rbp > 0x0000000000001137 <+14>: retq > End of assembler dump. > (gdb) b main > Breakpoint 1 at 0x1129: file test.c, line 2. > > Stepping in amd64_skip_prologue, we can see that the prologue analysis, > for GCC-compiled programs, is done in amd64_analyze_prologue by decoding > the instructions and looking for typical patterns. This patch changes > the analysis to check for a prologue starting with the `endbr64` > instruction, and skip it if it's there. > > gdb/ChangeLog: > > * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` > instruction, skip it if it's there. > > gdb/testsuite/ChangeLog: > > * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. > * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. > --- > gdb/amd64-tdep.c | 19 +++++++ > .../amd64-prologue-skip-cf-protection.c | 21 +++++++ > .../amd64-prologue-skip-cf-protection.exp | 56 +++++++++++++++++++ > 3 files changed, 96 insertions(+) > create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c > create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp > > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c > index 5c56a970d8c..c846447a8e0 100644 > --- a/gdb/amd64-tdep.c > +++ b/gdb/amd64-tdep.c > @@ -2362,6 +2362,9 @@ amd64_x32_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, > pushq %rbp 0x55 > movl %esp, %ebp 0x89 0xe5 (or 0x8b 0xec) > > + The `endbr64` instruction can be found before these sequences, and will be > + skipped if found. > + > Any function that doesn't start with one of these sequences will be > assumed to have no prologue and thus no valid frame pointer in > %rbp. */ > @@ -2372,6 +2375,8 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, > struct amd64_frame_cache *cache) > { > enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > + /* The `endbr64` instruction. */ > + static const gdb_byte endbr64[4] = { 0xf3, 0x0f, 0x1e, 0xfa }; > /* There are two variations of movq %rsp, %rbp. */ > static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 }; > static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec }; > @@ -2392,6 +2397,20 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, > > op = read_code_unsigned_integer (pc, 1, byte_order); > > + /* Check for the `endbr64` instruction, skip it if found. */ > + if (op == endbr64[0]) > + { > + read_code (pc + 1, buf, 3); Should we use target_read_code here and handle the case the memory could not be read? > + > + if (memcmp (buf, &endbr64[1], 3) == 0) > + pc += 4; > + > + op = read_code_unsigned_integer (pc, 1, byte_order); > + } > + > + if (current_pc <= pc) > + return current_pc; > + > if (op == 0x55) /* pushq %rbp */ > { > /* Take into account that we've executed the `pushq %rbp' that > diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c > new file mode 100644 > index 00000000000..968f1832d8c > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c > @@ -0,0 +1,21 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020 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/>. */ > + > +int foo (void) > +{ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp > new file mode 100644 > index 00000000000..c3348054ae0 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp > @@ -0,0 +1,56 @@ > +# Copyright 2020 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 skipping a prologue that was generated with gcc's -fcf-protection=full > +# (control flow protection) option. > +# > +# This option places an `endbr64` instruction at the start of all functions, > +# which can interfere with prologue analysis. > + > +standard_testfile .c > +set binfile ${binfile}.o > + I'm rather curious, why are we using a .o file here instead of an executable? > +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { > + verbose "Skipping ${testfile}." > + return > +} > + When using an older compiler, I see: ... Running gdb.arch/amd64-prologue-skip-cf-protection.exp ... gdb compile failed, gcc: error: unrecognized command line option \ '-fcf-protection=full'; did you mean '-fstack-protector-all'? === gdb Summary === # of untested testcases 1 ... Can we get rid of this verbose compile fail by using something like: ... if { ![supports_fcf_protection] } { untested "-fcf-protection not supported" return } ... in combination with this in gdb.exp: ... # Return 1 if compiler supports -fcf-protection=. Otherwise, # return 0. gdb_caching_proc supports_fcf_protection { return [gdb_can_simple_compile supports_statement_frontiers { int main () { return 0; } } executable "additional_flags=-fcf-protection=full"] } ... ? > +set opts {debug additional_flags=-fcf-protection=full} > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object $opts] != "" } { > + untested "failed to compile" > + return > +} > + > +clean_restart ${binfile} > + > +# Get start address of function foo. > +set foo_addr [get_integer_valueof &foo -1] > +gdb_assert {$foo_addr != -1} > + > +# Put breakpoint on foo, get the address where the breakpoint was installed. > +gdb_test_multiple "break foo" "break on foo, get address" { > + -re "Breakpoint $decimal at ($hex)" { > + set bp_addr $expect_out(1,string) > + > + # Convert to decimal. > + set bp_addr [expr $bp_addr] > + > + pass $gdb_test_name > + } > +} > + > +# Make sure some prologue was skipped. > +gdb_assert {$bp_addr > $foo_addr} > You're not using -wrap to get the gdb prompt, which is probably a good idea: ... -re -wrap "Breakpoint $decimal at ($hex).*" { ... Also, if the match fails for some reason, bp_addr will not be set, and cause a tcl error when accessed in the gdb_assert. We can move the gdb_assert after the pass, or set bp_addr to -1 before the gdb_test_multple, and do something like: ... if { $bp_addr != -1 } { # Make sure some prologue was skipped. gdb_assert {$bp_addr > $foo_addr} } ... Otherwise, LGTM. Thanks, - Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue 2020-05-01 9:52 ` Tom de Vries @ 2020-05-01 14:22 ` Simon Marchi 2020-05-06 16:31 ` Simon Marchi 0 siblings, 1 reply; 4+ messages in thread From: Simon Marchi @ 2020-05-01 14:22 UTC (permalink / raw) To: Tom de Vries, gdb-patches On 2020-05-01 5:52 a.m., Tom de Vries wrote: > On 01-05-2020 07:28, Simon Marchi via Gdb-patches wrote: >> Some GCCs now enable -fcf-protection by default. This is the case, for >> example, with GCC 9.3.0 on Ubuntu 20.04. Enabling it causes the >> `endbr64` instruction to be inserted at the beginning of all functions >> and that breaks GDB's prologue analysis. >> >> I noticed this because it gives many failures in gdb.base/break.exp. >> But let's take this dummy program and put a breakpoint on main: >> >> int main(void) >> { >> return 0; >> } >> >> Without -fcf-protection, the breakpoint is correctly put after the prologue: >> >> $ gcc test.c -g3 -O0 -fcf-protection=none >> $ ./gdb -q -nx --data-directory=data-directory a.out >> Reading symbols from a.out... >> (gdb) disassemble main >> Dump of assembler code for function main: >> 0x0000000000001129 <+0>: push %rbp >> 0x000000000000112a <+1>: mov %rsp,%rbp >> 0x000000000000112d <+4>: mov $0x0,%eax >> 0x0000000000001132 <+9>: pop %rbp >> 0x0000000000001133 <+10>: retq >> End of assembler dump. >> (gdb) b main >> Breakpoint 1 at 0x112d: file test.c, line 3. >> >> With -fcf-protection, the breakpoint is incorrectly put on the first >> byte of the function: >> >> $ gcc test.c -g3 -O0 -fcf-protection=full >> $ ./gdb -q -nx --data-directory=data-directory a.out >> Reading symbols from a.out... >> (gdb) disassemble main >> Dump of assembler code for function main: >> 0x0000000000001129 <+0>: endbr64 >> 0x000000000000112d <+4>: push %rbp >> 0x000000000000112e <+5>: mov %rsp,%rbp >> 0x0000000000001131 <+8>: mov $0x0,%eax >> 0x0000000000001136 <+13>: pop %rbp >> 0x0000000000001137 <+14>: retq >> End of assembler dump. >> (gdb) b main >> Breakpoint 1 at 0x1129: file test.c, line 2. >> >> Stepping in amd64_skip_prologue, we can see that the prologue analysis, >> for GCC-compiled programs, is done in amd64_analyze_prologue by decoding >> the instructions and looking for typical patterns. This patch changes >> the analysis to check for a prologue starting with the `endbr64` >> instruction, and skip it if it's there. >> >> gdb/ChangeLog: >> >> * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` >> instruction, skip it if it's there. >> >> gdb/testsuite/ChangeLog: >> >> * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. >> * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. >> --- >> gdb/amd64-tdep.c | 19 +++++++ >> .../amd64-prologue-skip-cf-protection.c | 21 +++++++ >> .../amd64-prologue-skip-cf-protection.exp | 56 +++++++++++++++++++ >> 3 files changed, 96 insertions(+) >> create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c >> create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp >> >> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c >> index 5c56a970d8c..c846447a8e0 100644 >> --- a/gdb/amd64-tdep.c >> +++ b/gdb/amd64-tdep.c >> @@ -2362,6 +2362,9 @@ amd64_x32_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, >> pushq %rbp 0x55 >> movl %esp, %ebp 0x89 0xe5 (or 0x8b 0xec) >> >> + The `endbr64` instruction can be found before these sequences, and will be >> + skipped if found. >> + >> Any function that doesn't start with one of these sequences will be >> assumed to have no prologue and thus no valid frame pointer in >> %rbp. */ >> @@ -2372,6 +2375,8 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, >> struct amd64_frame_cache *cache) >> { >> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >> + /* The `endbr64` instruction. */ >> + static const gdb_byte endbr64[4] = { 0xf3, 0x0f, 0x1e, 0xfa }; >> /* There are two variations of movq %rsp, %rbp. */ >> static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 }; >> static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec }; >> @@ -2392,6 +2397,20 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, >> >> op = read_code_unsigned_integer (pc, 1, byte_order); >> >> + /* Check for the `endbr64` instruction, skip it if found. */ >> + if (op == endbr64[0]) >> + { >> + read_code (pc + 1, buf, 3); > > Should we use target_read_code here and handle the case the memory could > not be read? read_code will throw an exception if the memory can't be read. It's not clear to me, however, if it will end up correctly handled or not. I'll use read_code in this patch, since that's what the function already uses, but we can look into improving error handling separately, if necessary. >> diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp >> new file mode 100644 >> index 00000000000..c3348054ae0 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp >> @@ -0,0 +1,56 @@ >> +# Copyright 2020 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 skipping a prologue that was generated with gcc's -fcf-protection=full >> +# (control flow protection) option. >> +# >> +# This option places an `endbr64` instruction at the start of all functions, >> +# which can interfere with prologue analysis. >> + >> +standard_testfile .c >> +set binfile ${binfile}.o >> + > > I'm rather curious, why are we using a .o file here instead of an > executable? Huh, because I copied stuff from amd64-prologue-skip.exp. This one is written in assembly and checks specific addresses. Using a .o file allows that, since we know it starts at 0 and we know the size of each instruction. Using an executable means that we can't hardcode addresses in the test. I've changed it to `executable`, and renamed the test function to `main`. >> +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { >> + verbose "Skipping ${testfile}." >> + return >> +} >> + > > When using an older compiler, I see: > ... > Running gdb.arch/amd64-prologue-skip-cf-protection.exp ... > gdb compile failed, gcc: error: unrecognized command line option \ > '-fcf-protection=full'; did you mean '-fstack-protector-all'? > > === gdb Summary === > > # of untested testcases 1 > ... > > Can we get rid of this verbose compile fail by using something like: > ... > if { ![supports_fcf_protection] } { > untested "-fcf-protection not supported" > return > } > ... > in combination with this in gdb.exp: > ... > # Return 1 if compiler supports -fcf-protection=. Otherwise, > > # return 0. > > > gdb_caching_proc supports_fcf_protection { > return [gdb_can_simple_compile supports_statement_frontiers { > int main () { > return 0; > } > } executable "additional_flags=-fcf-protection=full"] > } > ... > ? Yes, especially since you provide the code, that makes it easy :). I validated that it does what it's intended to do on a host without -fcf-protection support. > >> +set opts {debug additional_flags=-fcf-protection=full} >> + >> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object $opts] != "" } { >> + untested "failed to compile" >> + return >> +} >> + >> +clean_restart ${binfile} >> + >> +# Get start address of function foo. >> +set foo_addr [get_integer_valueof &foo -1] >> +gdb_assert {$foo_addr != -1} >> + >> +# Put breakpoint on foo, get the address where the breakpoint was installed. >> +gdb_test_multiple "break foo" "break on foo, get address" { >> + -re "Breakpoint $decimal at ($hex)" { >> + set bp_addr $expect_out(1,string) >> + >> + # Convert to decimal. >> + set bp_addr [expr $bp_addr] >> + >> + pass $gdb_test_name >> + } >> +} >> + >> +# Make sure some prologue was skipped. >> +gdb_assert {$bp_addr > $foo_addr} >> > > You're not using -wrap to get the gdb prompt, which is probably a good idea: > ... > -re -wrap "Breakpoint $decimal at ($hex).*" { > ... Done. > Also, if the match fails for some reason, bp_addr will not be set, and > cause a tcl error when accessed in the gdb_assert. > > We can move the gdb_assert after the pass, or set bp_addr to -1 before > the gdb_test_multple, and do something like: > ... > if { $bp_addr != -1 } { > # Make sure some prologue was skipped. > > gdb_assert {$bp_addr > $foo_addr} > } > ... Done (the `$p_addr != -1` way). If we fail to get the breakpoint address, there will already be a FAIL, it fine not to execute the gdb_assert. Thanks for the review, see updated patch below. From 6d8f6c2297db840ed5a4b3e79c06a2cc14e2c0d9 Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@efficios.com> Date: Thu, 30 Apr 2020 16:05:21 -0400 Subject: [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue v2: - test: build full executable instead of object - test: add and use supports_fcf_protection - test: use gdb_test_multiple's -wrap option - test: don't execute gdb_assert if failed to get breakpoint address Some GCCs now enable -fcf-protection by default. This is the case, for example, with GCC 9.3.0 on Ubuntu 20.04. Enabling it causes the `endbr64` instruction to be inserted at the beginning of all functions and that breaks GDB's prologue analysis. I noticed this because it gives many failures in gdb.base/break.exp. But let's take this dummy program and put a breakpoint on main: int main(void) { return 0; } Without -fcf-protection, the breakpoint is correctly put after the prologue: $ gcc test.c -g3 -O0 -fcf-protection=none $ ./gdb -q -nx --data-directory=data-directory a.out Reading symbols from a.out... (gdb) disassemble main Dump of assembler code for function main: 0x0000000000001129 <+0>: push %rbp 0x000000000000112a <+1>: mov %rsp,%rbp 0x000000000000112d <+4>: mov $0x0,%eax 0x0000000000001132 <+9>: pop %rbp 0x0000000000001133 <+10>: retq End of assembler dump. (gdb) b main Breakpoint 1 at 0x112d: file test.c, line 3. With -fcf-protection, the breakpoint is incorrectly put on the first byte of the function: $ gcc test.c -g3 -O0 -fcf-protection=full $ ./gdb -q -nx --data-directory=data-directory a.out Reading symbols from a.out... (gdb) disassemble main Dump of assembler code for function main: 0x0000000000001129 <+0>: endbr64 0x000000000000112d <+4>: push %rbp 0x000000000000112e <+5>: mov %rsp,%rbp 0x0000000000001131 <+8>: mov $0x0,%eax 0x0000000000001136 <+13>: pop %rbp 0x0000000000001137 <+14>: retq End of assembler dump. (gdb) b main Breakpoint 1 at 0x1129: file test.c, line 2. Stepping in amd64_skip_prologue, we can see that the prologue analysis, for GCC-compiled programs, is done in amd64_analyze_prologue by decoding the instructions and looking for typical patterns. This patch changes the analysis to check for a prologue starting with the `endbr64` instruction, and skip it if it's there. gdb/ChangeLog: * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` instruction, skip it if it's there. gdb/testsuite/ChangeLog: * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. --- gdb/amd64-tdep.c | 19 ++++++ .../amd64-prologue-skip-cf-protection.c | 21 ++++++ .../amd64-prologue-skip-cf-protection.exp | 65 +++++++++++++++++++ gdb/testsuite/lib/gdb.exp | 11 ++++ 4 files changed, 116 insertions(+) create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 5c56a970d8c4..c846447a8e0b 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2362,6 +2362,9 @@ amd64_x32_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, pushq %rbp 0x55 movl %esp, %ebp 0x89 0xe5 (or 0x8b 0xec) + The `endbr64` instruction can be found before these sequences, and will be + skipped if found. + Any function that doesn't start with one of these sequences will be assumed to have no prologue and thus no valid frame pointer in %rbp. */ @@ -2372,6 +2375,8 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, struct amd64_frame_cache *cache) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + /* The `endbr64` instruction. */ + static const gdb_byte endbr64[4] = { 0xf3, 0x0f, 0x1e, 0xfa }; /* There are two variations of movq %rsp, %rbp. */ static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 }; static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec }; @@ -2392,6 +2397,20 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, op = read_code_unsigned_integer (pc, 1, byte_order); + /* Check for the `endbr64` instruction, skip it if found. */ + if (op == endbr64[0]) + { + read_code (pc + 1, buf, 3); + + if (memcmp (buf, &endbr64[1], 3) == 0) + pc += 4; + + op = read_code_unsigned_integer (pc, 1, byte_order); + } + + if (current_pc <= pc) + return current_pc; + if (op == 0x55) /* pushq %rbp */ { /* Take into account that we've executed the `pushq %rbp' that diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c new file mode 100644 index 000000000000..a6505857e176 --- /dev/null +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c @@ -0,0 +1,21 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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/>. */ + +int main (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp new file mode 100644 index 000000000000..3c51fd303525 --- /dev/null +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp @@ -0,0 +1,65 @@ +# Copyright 2020 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 skipping a prologue that was generated with gcc's -fcf-protection=full +# (control flow protection) option. +# +# This option places an `endbr64` instruction at the start of all functions, +# which can interfere with prologue analysis. + +standard_testfile .c +set binfile ${binfile} + +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { + verbose "Skipping ${testfile}." + return +} + +if { ![supports_fcf_protection] } { + untested "-fcf-protection not supported" + return +} + +set opts {debug additional_flags=-fcf-protection=full} + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $opts] != "" } { + untested "failed to compile" + return +} + +clean_restart ${binfile} + +# Get start address of function main. +set main_addr [get_integer_valueof &main -1] +gdb_assert {$main_addr != -1} + +set bp_addr -1 + +# Put breakpoint on main, get the address where the breakpoint was installed. +gdb_test_multiple "break main" "break on main, get address" { + -re -wrap "Breakpoint $decimal at ($hex).*" { + set bp_addr $expect_out(1,string) + + # Convert to decimal. + set bp_addr [expr $bp_addr] + + pass $gdb_test_name + } +} + +if { $bp_addr != -1 } { + # Make sure some prologue was skipped. + gdb_assert {$bp_addr > $main_addr} +} diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index b72ce0cda7fa..ab0dd44da7bc 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -7007,6 +7007,17 @@ gdb_caching_proc supports_statement_frontiers { } executable "additional_flags=-gstatement-frontiers"] } +# Return 1 if compiler supports -fcf-protection=. Otherwise, +# return 0. + +gdb_caching_proc supports_fcf_protection { + return [gdb_can_simple_compile supports_fcf_protection { + int main () { + return 0; + } + } executable "additional_flags=-fcf-protection=full"] +} + # Return 1 if symbols were read in using -readnow. Otherwise, return 0. proc readnow { } { -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue 2020-05-01 14:22 ` Simon Marchi @ 2020-05-06 16:31 ` Simon Marchi 0 siblings, 0 replies; 4+ messages in thread From: Simon Marchi @ 2020-05-06 16:31 UTC (permalink / raw) To: Simon Marchi, Tom de Vries, gdb-patches On 2020-05-01 10:22 a.m., Simon Marchi via Gdb-patches wrote: > On 2020-05-01 5:52 a.m., Tom de Vries wrote: >> On 01-05-2020 07:28, Simon Marchi via Gdb-patches wrote: >>> Some GCCs now enable -fcf-protection by default. This is the case, for >>> example, with GCC 9.3.0 on Ubuntu 20.04. Enabling it causes the >>> `endbr64` instruction to be inserted at the beginning of all functions >>> and that breaks GDB's prologue analysis. >>> >>> I noticed this because it gives many failures in gdb.base/break.exp. >>> But let's take this dummy program and put a breakpoint on main: >>> >>> int main(void) >>> { >>> return 0; >>> } >>> >>> Without -fcf-protection, the breakpoint is correctly put after the prologue: >>> >>> $ gcc test.c -g3 -O0 -fcf-protection=none >>> $ ./gdb -q -nx --data-directory=data-directory a.out >>> Reading symbols from a.out... >>> (gdb) disassemble main >>> Dump of assembler code for function main: >>> 0x0000000000001129 <+0>: push %rbp >>> 0x000000000000112a <+1>: mov %rsp,%rbp >>> 0x000000000000112d <+4>: mov $0x0,%eax >>> 0x0000000000001132 <+9>: pop %rbp >>> 0x0000000000001133 <+10>: retq >>> End of assembler dump. >>> (gdb) b main >>> Breakpoint 1 at 0x112d: file test.c, line 3. >>> >>> With -fcf-protection, the breakpoint is incorrectly put on the first >>> byte of the function: >>> >>> $ gcc test.c -g3 -O0 -fcf-protection=full >>> $ ./gdb -q -nx --data-directory=data-directory a.out >>> Reading symbols from a.out... >>> (gdb) disassemble main >>> Dump of assembler code for function main: >>> 0x0000000000001129 <+0>: endbr64 >>> 0x000000000000112d <+4>: push %rbp >>> 0x000000000000112e <+5>: mov %rsp,%rbp >>> 0x0000000000001131 <+8>: mov $0x0,%eax >>> 0x0000000000001136 <+13>: pop %rbp >>> 0x0000000000001137 <+14>: retq >>> End of assembler dump. >>> (gdb) b main >>> Breakpoint 1 at 0x1129: file test.c, line 2. >>> >>> Stepping in amd64_skip_prologue, we can see that the prologue analysis, >>> for GCC-compiled programs, is done in amd64_analyze_prologue by decoding >>> the instructions and looking for typical patterns. This patch changes >>> the analysis to check for a prologue starting with the `endbr64` >>> instruction, and skip it if it's there. >>> >>> gdb/ChangeLog: >>> >>> * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` >>> instruction, skip it if it's there. >>> >>> gdb/testsuite/ChangeLog: >>> >>> * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. >>> * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. >>> --- >>> gdb/amd64-tdep.c | 19 +++++++ >>> .../amd64-prologue-skip-cf-protection.c | 21 +++++++ >>> .../amd64-prologue-skip-cf-protection.exp | 56 +++++++++++++++++++ >>> 3 files changed, 96 insertions(+) >>> create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c >>> create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp >>> >>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c >>> index 5c56a970d8c..c846447a8e0 100644 >>> --- a/gdb/amd64-tdep.c >>> +++ b/gdb/amd64-tdep.c >>> @@ -2362,6 +2362,9 @@ amd64_x32_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, >>> pushq %rbp 0x55 >>> movl %esp, %ebp 0x89 0xe5 (or 0x8b 0xec) >>> >>> + The `endbr64` instruction can be found before these sequences, and will be >>> + skipped if found. >>> + >>> Any function that doesn't start with one of these sequences will be >>> assumed to have no prologue and thus no valid frame pointer in >>> %rbp. */ >>> @@ -2372,6 +2375,8 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, >>> struct amd64_frame_cache *cache) >>> { >>> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >>> + /* The `endbr64` instruction. */ >>> + static const gdb_byte endbr64[4] = { 0xf3, 0x0f, 0x1e, 0xfa }; >>> /* There are two variations of movq %rsp, %rbp. */ >>> static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 }; >>> static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec }; >>> @@ -2392,6 +2397,20 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, >>> >>> op = read_code_unsigned_integer (pc, 1, byte_order); >>> >>> + /* Check for the `endbr64` instruction, skip it if found. */ >>> + if (op == endbr64[0]) >>> + { >>> + read_code (pc + 1, buf, 3); >> >> Should we use target_read_code here and handle the case the memory could >> not be read? > > read_code will throw an exception if the memory can't be read. It's not > clear to me, however, if it will end up correctly handled or not. I'll use > read_code in this patch, since that's what the function already uses, but > we can look into improving error handling separately, if necessary. > >>> diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp >>> new file mode 100644 >>> index 00000000000..c3348054ae0 >>> --- /dev/null >>> +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp >>> @@ -0,0 +1,56 @@ >>> +# Copyright 2020 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 skipping a prologue that was generated with gcc's -fcf-protection=full >>> +# (control flow protection) option. >>> +# >>> +# This option places an `endbr64` instruction at the start of all functions, >>> +# which can interfere with prologue analysis. >>> + >>> +standard_testfile .c >>> +set binfile ${binfile}.o >>> + >> >> I'm rather curious, why are we using a .o file here instead of an >> executable? > > Huh, because I copied stuff from amd64-prologue-skip.exp. This one is written in > assembly and checks specific addresses. Using a .o file allows that, since we know > it starts at 0 and we know the size of each instruction. Using an executable means > that we can't hardcode addresses in the test. > > I've changed it to `executable`, and renamed the test function to `main`. > >>> +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { >>> + verbose "Skipping ${testfile}." >>> + return >>> +} >>> + >> >> When using an older compiler, I see: >> ... >> Running gdb.arch/amd64-prologue-skip-cf-protection.exp ... >> gdb compile failed, gcc: error: unrecognized command line option \ >> '-fcf-protection=full'; did you mean '-fstack-protector-all'? >> >> === gdb Summary === >> >> # of untested testcases 1 >> ... >> >> Can we get rid of this verbose compile fail by using something like: >> ... >> if { ![supports_fcf_protection] } { >> untested "-fcf-protection not supported" >> return >> } >> ... >> in combination with this in gdb.exp: >> ... >> # Return 1 if compiler supports -fcf-protection=. Otherwise, >> >> # return 0. >> >> >> gdb_caching_proc supports_fcf_protection { >> return [gdb_can_simple_compile supports_statement_frontiers { >> int main () { >> return 0; >> } >> } executable "additional_flags=-fcf-protection=full"] >> } >> ... >> ? > > Yes, especially since you provide the code, that makes it easy :). > > I validated that it does what it's intended to do on a host without -fcf-protection > support. > >> >>> +set opts {debug additional_flags=-fcf-protection=full} >>> + >>> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object $opts] != "" } { >>> + untested "failed to compile" >>> + return >>> +} >>> + >>> +clean_restart ${binfile} >>> + >>> +# Get start address of function foo. >>> +set foo_addr [get_integer_valueof &foo -1] >>> +gdb_assert {$foo_addr != -1} >>> + >>> +# Put breakpoint on foo, get the address where the breakpoint was installed. >>> +gdb_test_multiple "break foo" "break on foo, get address" { >>> + -re "Breakpoint $decimal at ($hex)" { >>> + set bp_addr $expect_out(1,string) >>> + >>> + # Convert to decimal. >>> + set bp_addr [expr $bp_addr] >>> + >>> + pass $gdb_test_name >>> + } >>> +} >>> + >>> +# Make sure some prologue was skipped. >>> +gdb_assert {$bp_addr > $foo_addr} >>> >> >> You're not using -wrap to get the gdb prompt, which is probably a good idea: >> ... >> -re -wrap "Breakpoint $decimal at ($hex).*" { >> ... > > Done. > >> Also, if the match fails for some reason, bp_addr will not be set, and >> cause a tcl error when accessed in the gdb_assert. >> >> We can move the gdb_assert after the pass, or set bp_addr to -1 before >> the gdb_test_multple, and do something like: >> ... >> if { $bp_addr != -1 } { >> # Make sure some prologue was skipped. >> >> gdb_assert {$bp_addr > $foo_addr} >> } >> ... > > Done (the `$p_addr != -1` way). If we fail to get the breakpoint address, > there will already be a FAIL, it fine not to execute the gdb_assert. > > Thanks for the review, see updated patch below. > > From 6d8f6c2297db840ed5a4b3e79c06a2cc14e2c0d9 Mon Sep 17 00:00:00 2001 > From: Simon Marchi <simon.marchi@efficios.com> > Date: Thu, 30 Apr 2020 16:05:21 -0400 > Subject: [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue > > v2: > - test: build full executable instead of object > - test: add and use supports_fcf_protection > - test: use gdb_test_multiple's -wrap option > - test: don't execute gdb_assert if failed to get breakpoint address > > Some GCCs now enable -fcf-protection by default. This is the case, for > example, with GCC 9.3.0 on Ubuntu 20.04. Enabling it causes the > `endbr64` instruction to be inserted at the beginning of all functions > and that breaks GDB's prologue analysis. > > I noticed this because it gives many failures in gdb.base/break.exp. > But let's take this dummy program and put a breakpoint on main: > > int main(void) > { > return 0; > } > > Without -fcf-protection, the breakpoint is correctly put after the prologue: > > $ gcc test.c -g3 -O0 -fcf-protection=none > $ ./gdb -q -nx --data-directory=data-directory a.out > Reading symbols from a.out... > (gdb) disassemble main > Dump of assembler code for function main: > 0x0000000000001129 <+0>: push %rbp > 0x000000000000112a <+1>: mov %rsp,%rbp > 0x000000000000112d <+4>: mov $0x0,%eax > 0x0000000000001132 <+9>: pop %rbp > 0x0000000000001133 <+10>: retq > End of assembler dump. > (gdb) b main > Breakpoint 1 at 0x112d: file test.c, line 3. > > With -fcf-protection, the breakpoint is incorrectly put on the first > byte of the function: > > $ gcc test.c -g3 -O0 -fcf-protection=full > $ ./gdb -q -nx --data-directory=data-directory a.out > Reading symbols from a.out... > (gdb) disassemble main > Dump of assembler code for function main: > 0x0000000000001129 <+0>: endbr64 > 0x000000000000112d <+4>: push %rbp > 0x000000000000112e <+5>: mov %rsp,%rbp > 0x0000000000001131 <+8>: mov $0x0,%eax > 0x0000000000001136 <+13>: pop %rbp > 0x0000000000001137 <+14>: retq > End of assembler dump. > (gdb) b main > Breakpoint 1 at 0x1129: file test.c, line 2. > > Stepping in amd64_skip_prologue, we can see that the prologue analysis, > for GCC-compiled programs, is done in amd64_analyze_prologue by decoding > the instructions and looking for typical patterns. This patch changes > the analysis to check for a prologue starting with the `endbr64` > instruction, and skip it if it's there. > > gdb/ChangeLog: > > * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` > instruction, skip it if it's there. > > gdb/testsuite/ChangeLog: > > * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. > * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. > --- > gdb/amd64-tdep.c | 19 ++++++ > .../amd64-prologue-skip-cf-protection.c | 21 ++++++ > .../amd64-prologue-skip-cf-protection.exp | 65 +++++++++++++++++++ > gdb/testsuite/lib/gdb.exp | 11 ++++ > 4 files changed, 116 insertions(+) > create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c > create mode 100644 gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp > > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c > index 5c56a970d8c4..c846447a8e0b 100644 > --- a/gdb/amd64-tdep.c > +++ b/gdb/amd64-tdep.c > @@ -2362,6 +2362,9 @@ amd64_x32_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc, > pushq %rbp 0x55 > movl %esp, %ebp 0x89 0xe5 (or 0x8b 0xec) > > + The `endbr64` instruction can be found before these sequences, and will be > + skipped if found. > + > Any function that doesn't start with one of these sequences will be > assumed to have no prologue and thus no valid frame pointer in > %rbp. */ > @@ -2372,6 +2375,8 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, > struct amd64_frame_cache *cache) > { > enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > + /* The `endbr64` instruction. */ > + static const gdb_byte endbr64[4] = { 0xf3, 0x0f, 0x1e, 0xfa }; > /* There are two variations of movq %rsp, %rbp. */ > static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 }; > static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec }; > @@ -2392,6 +2397,20 @@ amd64_analyze_prologue (struct gdbarch *gdbarch, > > op = read_code_unsigned_integer (pc, 1, byte_order); > > + /* Check for the `endbr64` instruction, skip it if found. */ > + if (op == endbr64[0]) > + { > + read_code (pc + 1, buf, 3); > + > + if (memcmp (buf, &endbr64[1], 3) == 0) > + pc += 4; > + > + op = read_code_unsigned_integer (pc, 1, byte_order); > + } > + > + if (current_pc <= pc) > + return current_pc; > + > if (op == 0x55) /* pushq %rbp */ > { > /* Take into account that we've executed the `pushq %rbp' that > diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c > new file mode 100644 > index 000000000000..a6505857e176 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.c > @@ -0,0 +1,21 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020 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/>. */ > + > +int main (void) > +{ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp > new file mode 100644 > index 000000000000..3c51fd303525 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-prologue-skip-cf-protection.exp > @@ -0,0 +1,65 @@ > +# Copyright 2020 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 skipping a prologue that was generated with gcc's -fcf-protection=full > +# (control flow protection) option. > +# > +# This option places an `endbr64` instruction at the start of all functions, > +# which can interfere with prologue analysis. > + > +standard_testfile .c > +set binfile ${binfile} > + > +if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { > + verbose "Skipping ${testfile}." > + return > +} > + > +if { ![supports_fcf_protection] } { > + untested "-fcf-protection not supported" > + return > +} > + > +set opts {debug additional_flags=-fcf-protection=full} > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $opts] != "" } { > + untested "failed to compile" > + return > +} > + > +clean_restart ${binfile} > + > +# Get start address of function main. > +set main_addr [get_integer_valueof &main -1] > +gdb_assert {$main_addr != -1} > + > +set bp_addr -1 > + > +# Put breakpoint on main, get the address where the breakpoint was installed. > +gdb_test_multiple "break main" "break on main, get address" { > + -re -wrap "Breakpoint $decimal at ($hex).*" { > + set bp_addr $expect_out(1,string) > + > + # Convert to decimal. > + set bp_addr [expr $bp_addr] > + > + pass $gdb_test_name > + } > +} > + > +if { $bp_addr != -1 } { > + # Make sure some prologue was skipped. > + gdb_assert {$bp_addr > $main_addr} > +} > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index b72ce0cda7fa..ab0dd44da7bc 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -7007,6 +7007,17 @@ gdb_caching_proc supports_statement_frontiers { > } executable "additional_flags=-gstatement-frontiers"] > } > > +# Return 1 if compiler supports -fcf-protection=. Otherwise, > +# return 0. > + > +gdb_caching_proc supports_fcf_protection { > + return [gdb_can_simple_compile supports_fcf_protection { > + int main () { > + return 0; > + } > + } executable "additional_flags=-fcf-protection=full"] > +} > + > # Return 1 if symbols were read in using -readnow. Otherwise, return 0. > > proc readnow { } { > -- > 2.26.2 > I pushed this patch. Simon ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-06 16:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-01 5:28 [PATCH] gdb: handle endbr64 instruction in amd64_analyze_prologue Simon Marchi 2020-05-01 9:52 ` Tom de Vries 2020-05-01 14:22 ` Simon Marchi 2020-05-06 16:31 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).