From ac4a4f1cd7dceeeb17d0b8c077c874f2247acbf0 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 6 May 2020 12:01:37 -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/ChangeLog | 5 ++ gdb/amd64-tdep.c | 19 ++++++ gdb/testsuite/ChangeLog | 5 ++ .../amd64-prologue-skip-cf-protection.c | 21 ++++++ .../amd64-prologue-skip-cf-protection.exp | 65 +++++++++++++++++++ gdb/testsuite/lib/gdb.exp | 11 ++++ 6 files changed, 126 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/ChangeLog b/gdb/ChangeLog index dd29c753495..27cdbc84f53 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2020-05-06 Simon Marchi + + * amd64-tdep.c (amd64_analyze_prologue): Check for `endbr64` + instruction, skip it if it's there. + 2020-05-05 Simon Marchi * gdbtypes.h (struct main_type) : Remove. 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/ChangeLog b/gdb/testsuite/ChangeLog index 443c3d35d12..a2e0d87e89d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-05-06 Simon Marchi + + * gdb.arch/amd64-prologue-skip-cf-protection.exp: New file. + * gdb.arch/amd64-prologue-skip-cf-protection.c: New file. + 2020-05-06 Tom de Vries * gdb.reverse/consecutive-precsave.exp: Handle if instruction after 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..a6505857e17 --- /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 . */ + +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 00000000000..3c51fd30352 --- /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 . + +# 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 239871114d2..3c6f0d76d67 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -7019,6 +7019,17 @@ gdb_caching_proc supports_mpx_check_pointer_bounds { } executable $flags] } +# 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.30.2