From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7862) id BC4CD3854171; Fri, 21 Oct 2022 10:49:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BC4CD3854171 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666349381; bh=5EA4HQ8Zz6GLpuZiqHEjWl/K2sRCYqYWemjDnQ4zwfw=; h=From:To:Subject:Date:From; b=FUtkqNb0L7+xCgowSt3QK/c7DK9FGHmfZEPO0a89uFZNgEAsWj2tTaEe8yoqnneE0 Jtp/0P9UP8bYr1soAxjKij8cRlu35ZnqyiAfE+rgUJDuR8XmS6vJocaFmLK8ux2FdR iIKHK2SRFS2HnAPE6VDTPk+5kziCO0IvsC6UKpTI= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Bruno Larsen To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Change calculation of frame_id by amd64 epilogue unwinder X-Act-Checkin: binutils-gdb X-Git-Author: Bruno Larsen X-Git-Refname: refs/heads/master X-Git-Oldrev: 7c0cca765e4630a5b3b8df285c7b0f90b6cb41cc X-Git-Newrev: 49d7cd733a7f1b87aa1d40318b3d7c2b65aca5ac Message-Id: <20221021104941.BC4CD3854171@sourceware.org> Date: Fri, 21 Oct 2022 10:49:41 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D49d7cd733a7f= 1b87aa1d40318b3d7c2b65aca5ac commit 49d7cd733a7f1b87aa1d40318b3d7c2b65aca5ac Author: Bruno Larsen Date: Fri Aug 19 15:11:28 2022 +0200 Change calculation of frame_id by amd64 epilogue unwinder =20 When GDB is stopped at a ret instruction and no debug information is available for unwinding, GDB defaults to the amd64 epilogue unwinder, to be able to generate a decent backtrace. However, when calculating the frame id, the epilogue unwinder generates information as if the return instruction was the whole frame. =20 This was an issue especially when attempting to reverse debug, as GDB would place a step_resume_breakpoint from the epilogue of a function if we were to attempt to skip that function, and this breakpoint should ideally have the current function's frame_id to avoid other problems such as PR record/16678. =20 This commit changes the frame_id calculation for the amd64 epilogue, so that it is always the same as the dwarf2 unwinder's frame_id. =20 It also adds a test to confirm that the frame_id will be the same, regardless of using the epilogue unwinder or not, thanks to Andrew Burgess. =20 Co-Authored-By: Andrew Burgess Diff: --- gdb/amd64-tdep.c | 10 +- gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c | 22 ++++ gdb/testsuite/gdb.base/unwind-on-each-insn.c | 25 ++++ gdb/testsuite/gdb.base/unwind-on-each-insn.exp | 154 +++++++++++++++++++= ++++ 4 files changed, 206 insertions(+), 5 deletions(-) diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index a6894d82dbc..bbfc509319c 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2937,18 +2937,18 @@ amd64_epilogue_frame_cache (frame_info_ptr this_fra= me, void **this_cache) =20 try { - /* Cache base will be %esp plus cache->sp_offset (-8). */ + /* Cache base will be %rsp plus cache->sp_offset (-8). */ get_frame_register (this_frame, AMD64_RSP_REGNUM, buf); cache->base =3D extract_unsigned_integer (buf, 8, byte_order) + cache->sp_offset; =20 /* Cache pc will be the frame func. */ - cache->pc =3D get_frame_pc (this_frame); + cache->pc =3D get_frame_func (this_frame); =20 - /* The saved %esp will be at cache->base plus 16. */ + /* The previous value of %rsp is cache->base plus 16. */ cache->saved_sp =3D cache->base + 16; =20 - /* The saved %eip will be at cache->base plus 8. */ + /* The saved %rip will be at cache->base plus 8. */ cache->saved_regs[AMD64_RIP_REGNUM] =3D cache->base + 8; =20 cache->base_p =3D 1; @@ -2986,7 +2986,7 @@ amd64_epilogue_frame_this_id (frame_info_ptr this_fra= me, if (!cache->base_p) (*this_id) =3D frame_id_build_unavailable_stack (cache->pc); else - (*this_id) =3D frame_id_build (cache->base + 8, cache->pc); + (*this_id) =3D frame_id_build (cache->base + 16, cache->pc); } =20 static const struct frame_unwind amd64_epilogue_frame_unwind =3D diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c b/gdb/testsui= te/gdb.base/unwind-on-each-insn-foo.c new file mode 100644 index 00000000000..e5e5ebfd013 --- /dev/null +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 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 . = */ + +void +foo () +{ + /* Nothing. */ +} diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.c b/gdb/testsuite/g= db.base/unwind-on-each-insn.c new file mode 100644 index 00000000000..3e4cc2675f2 --- /dev/null +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.c @@ -0,0 +1,25 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 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 . = */ + +extern void foo (); + +int +main () +{ + foo (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite= /gdb.base/unwind-on-each-insn.exp new file mode 100644 index 00000000000..46bea800c1b --- /dev/null +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp @@ -0,0 +1,154 @@ +# Copyright 2022 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 . */ + +# Single step through a simple (empty) function that was compiled +# without DWARF debug information. +# +# At each instruction check that the frame-id, and frame base address, +# are calculated correctly. +# +# Additionally, check we can correctly unwind to the previous frame, +# and that the previous stack-pointer value, and frame base address +# value, can be calculated correctly. + +standard_testfile .c -foo.c + +if {[prepare_for_testing_full "failed to prepare" \ + [list ${testfile} debug \ + $srcfile {debug} $srcfile2 {nodebug}]]} { + return -1 +} + +if ![runto_main] then { + return 0 +} + +# Return a two element list, the first element is the stack-pointer +# value (from the $sp register), and the second element is the frame +# base address (from the 'info frame' output). +proc get_sp_and_fba { testname } { + with_test_prefix "get \$sp and frame base $testname" { + set sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*"] + + set fba "" + gdb_test_multiple "info frame" "" { + -re "^info frame\r\n" { + exp_continue + } + + -re "^Stack level ${::decimal}, frame at ($::hex):\r\n.*$::gdb_prompt= $" { + set fba $expect_out(1,string) + } + } + + return [list $sp $fba] + } +} + +# Return the frame-id of the current frame, collected using the 'maint +# print frame-id' command. +proc get_fid { } { + set fid "" + gdb_test_multiple "maint print frame-id" "" { + -re "^maint print frame-id\r\n" { + exp_continue + } + + -re "^frame-id for frame #${::decimal}: (\[^\r\n\]+).*" { + set fid $expect_out(1,string) + } + } + return $fid +} + +# Record the current stack-pointer, and the frame base address. +lassign [get_sp_and_fba "in main"] main_sp main_fba +set main_fid [get_fid] + +# Now enter the foo function. +gdb_breakpoint "*foo" +gdb_continue_to_breakpoint "enter foo" + +# Figure out the range of addresses covered by this function. +set last_addr_in_foo "" +gdb_test_multiple "disassemble foo" "" { + -re "^disassemble foo\r\n" { + exp_continue + } + + -re "^Dump of assembler code for function foo:\r\n" { + exp_continue + } + + -re "^...($hex) \[^\r\n\]+\r\n" { + set last_addr_in_foo $expect_out(1,string) + exp_continue + } + + -wrap -re "^End of assembler dump\\." { + gdb_assert { ![string equal $last_addr_in_foo ""] } \ + "found some addresses in foo" + pass $gdb_test_name + } +} + +# Record the current stack-pointer, and the frame base address. +lassign [get_sp_and_fba "in foo"] foo_sp foo_fba +set foo_fid [get_fid] + +for { set i_count 1 } { true } { incr i_count } { + with_test_prefix "instruction ${i_count}" { + + # The current stack-pointer value can legitimately change + # throughout the lifetime of a function, so we don't check the + # current stack-pointer value. But the frame base address + # should not change, so we do check for that. + lassign [get_sp_and_fba "for foo"] sp_value fba_value + gdb_assert { $fba_value =3D=3D $foo_fba } + + # The frame-id should never change within a function, so check + # that now. + set fid [get_fid] + gdb_assert { [string equal $fid $foo_fid] } \ + "check frame-id matches" + + # Check that the previous frame is 'main'. + gdb_test "bt 2" "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*" + + # Move up the stack (to main). + gdb_test "up" \ + "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*" + + # Check we can unwind the stack-pointer and the frame base + # address correctly. + lassign [get_sp_and_fba "for main"] sp_value fba_value + gdb_assert { $sp_value =3D=3D $main_sp } + gdb_assert { $fba_value =3D=3D $main_fba } + + # Check we have a consistent value for main's frame-id. + set fid [get_fid] + gdb_assert { [string equal $fid $main_fid] } + + # Move back to the inner most frame. + gdb_test "frame 0" ".*" + + set pc [get_hexadecimal_valueof "\$pc" "*UNKNOWN*"] + if { $pc =3D=3D $last_addr_in_foo } { + break + } + + gdb_test "stepi" ".*" + } +}