From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 9B4B738276FB for ; Wed, 31 Aug 2022 12:16:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9B4B738276FB Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-547-BKHjftTDPBaWblHlz0qcDQ-1; Wed, 31 Aug 2022 08:16:32 -0400 X-MC-Unique: BKHjftTDPBaWblHlz0qcDQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E75C6185A7B2; Wed, 31 Aug 2022 12:16:31 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.105]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 24AF04C819; Wed, 31 Aug 2022 12:16:31 +0000 (UTC) From: Bruno Larsen To: gdb-patches@sourceware.org Cc: pedro@palves.net, aburgess@redhat.com, Bruno Larsen Subject: [PATCH v3 1/2] Change calculation of frame_id by amd64 epilogue unwinder Date: Wed, 31 Aug 2022 14:07:27 +0200 Message-Id: <20220831120727.2742360-2-blarsen@redhat.com> In-Reply-To: <20220831120727.2742360-1-blarsen@redhat.com> References: <20220831120727.2742360-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Aug 2022 12:16:37 -0000 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. 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. 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. 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. Co-Authored-By: Andrew Burgess --- gdb/amd64-tdep.c | 10 +- .../gdb.base/unwind-on-each-insn-foo.c | 22 +++ gdb/testsuite/gdb.base/unwind-on-each-insn.c | 25 +++ .../gdb.base/unwind-on-each-insn.exp | 154 ++++++++++++++++++ 4 files changed, 206 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c create mode 100644 gdb/testsuite/gdb.base/unwind-on-each-insn.c create mode 100644 gdb/testsuite/gdb.base/unwind-on-each-insn.exp diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index d89e06d27cb..078cf3ce4c0 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2937,18 +2937,18 @@ amd64_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache) 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 = extract_unsigned_integer (buf, 8, byte_order) + cache->sp_offset; /* Cache pc will be the frame func. */ - cache->pc = get_frame_pc (this_frame); + cache->pc = get_frame_func (this_frame); - /* The saved %esp will be at cache->base plus 16. */ + /* The previous value of %rsp is cache->base plus 16. */ cache->saved_sp = cache->base + 16; - /* 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] = cache->base + 8; cache->base_p = 1; @@ -2986,7 +2986,7 @@ amd64_epilogue_frame_this_id (struct frame_info *this_frame, if (!cache->base_p) (*this_id) = frame_id_build_unavailable_stack (cache->pc); else - (*this_id) = frame_id_build (cache->base + 8, cache->pc); + (*this_id) = frame_id_build (cache->base + 16, cache->pc); } static const struct frame_unwind amd64_epilogue_frame_unwind = diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c b/gdb/testsuite/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/gdb.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 == $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 == $main_sp } + gdb_assert { $fba_value == $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 == $last_addr_in_foo } { + break + } + + gdb_test "stepi" ".*" + } +} -- 2.37.2