From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id C6BC43872564 for ; Wed, 14 Dec 2022 03:36:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C6BC43872564 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 2BE3ajV4029861 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 13 Dec 2022 22:36:49 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 2BE3ajV4029861 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1670989010; bh=m2CEHTw6qRrHHKUL+o5876ZxgXxFqbchn8MtD63+dBE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mCUgAiK+AdnhtCbLKJND+B5C0wKhM1boVftbRSUKZeu/dXfZiWrPKhry/DzDiGIMl 66+yWCULm0s9yNyFL2Rc5tmI8WYu+WB5LIRuJsyrDWROckxsPDuNhISk6aJ3U00+eU v/M0Zkv6ELczvFltccNI2bK7afGU9jWb3BByrKOo= Received: from simark.localdomain (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 26E481E126; Tue, 13 Dec 2022 22:36:45 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH v2 10/13] gdb: make it possible to restore selected user-created frames Date: Tue, 13 Dec 2022 22:34:38 -0500 Message-Id: <20221214033441.499512-11-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221214033441.499512-1-simon.marchi@polymtl.ca> References: <20221214033441.499512-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 14 Dec 2022 03:36:45 +0000 X-Spam-Status: No, score=-3189.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: From: Simon Marchi I would like to improve frame_info_ptr to automatically grab the information needed to reinflate a frame, and automatically reinflate it as needed. One thing that is in the way is the fact that some frames can be created out of thin air by the create_new_frame function. These frames are not the fruit of unwinding from the target's current frame. These frames are created by the "select-frame view" command. These frames are not correctly handled by the frame save/restore functions, save_selected_frame, restore_selected_frame and lookup_selected_frame. This can be observed here, using the test included in this patch: $ ./gdb --data-directory=data-directory -nx -q testsuite/outputs/gdb.base/frame-view/frame-view Reading symbols from testsuite/outputs/gdb.base/frame-view/frame-view... (gdb) break thread_func Breakpoint 1 at 0x11a2: file /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c, line 42. (gdb) run Starting program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/frame-view/frame-view [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1". [New Thread 0x7ffff7cc46c0 (LWP 4171134)] [Switching to Thread 0x7ffff7cc46c0 (LWP 4171134)] Thread 2 "frame-view" hit Breakpoint 1, thread_func (p=0x0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42 42 foo (11); (gdb) info frame Stack level 0, frame at 0x7ffff7cc3ee0: rip = 0x5555555551a2 in thread_func (/home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42); saved rip = 0x7ffff7d4e8fd called by frame at 0x7ffff7cc3f80 source language c. Arglist at 0x7ffff7cc3ed0, args: p=0x0 Locals at 0x7ffff7cc3ed0, Previous frame's sp is 0x7ffff7cc3ee0 Saved registers: rbp at 0x7ffff7cc3ed0, rip at 0x7ffff7cc3ed8 (gdb) thread 1 [Switching to thread 1 (Thread 0x7ffff7cc5740 (LWP 4171122))] #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 Here, we create a custom frame for thread 1 (using the stack from thread 2, for convenience): (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2 The first calls to "frame" looks good: (gdb) frame #0 thread_func (p=0x7ffff7d4e630) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42 42 foo (11); But not the second one: (gdb) frame #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 This second "frame" command shows the current target frame instead of the user-created frame. It's not totally clear how the "select-frame view" feature is expected to behave, especially since it's not tested. I heard accounts that it used to be possible to select a frame like this and do "up" and "down" to navigate the backtrace starting from that frame. The fact that create_new_frame calls frame_unwind_find_by_frame to install the right unwinder suggest that it used to be possible. But that doesn't work today: (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2 (gdb) up Initial frame selected; you cannot go up. (gdb) down Bottom (innermost) frame selected; you cannot go down. and "backtrace" always shows the actual thread's backtrace, it ignores the user-created frame: (gdb) bt #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 #1 0x00007ffff7d50403 in ?? () from /usr/lib/libc.so.6 #2 0x000055555555521a in main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:56 I don't want to address all the `select-frame view` issues , but I think we can agree that the "frame" command changing the selected frame, as shown above, is a bug. I would expect that command to show the currently selected frame and not change it. This happens because of the scoped_restore_selected_frame object in print_frame_args. The frame information is saved in the constructor (the backtrace below), and restored in the destructor. #0 save_selected_frame (frame_id=0x7ffdc0020ad0, frame_level=0x7ffdc0020af0) at /home/simark/src/binutils-gdb/gdb/frame.c:1682 #1 0x00005631390242f0 in scoped_restore_selected_frame::scoped_restore_selected_frame (this=0x7ffdc0020ad0) at /home/simark/src/binutils-gdb/gdb/frame.c:324 #2 0x000056313993581e in print_frame_args (fp_opts=..., func=0x62100023bde0, frame=..., num=-1, stream=0x60b000000300) at /home/simark/src/binutils-gdb/gdb/stack.c:755 #3 0x000056313993ad49 in print_frame (fp_opts=..., frame=..., print_level=1, print_what=SRC_AND_LOC, print_args=1, sal=...) at /home/simark/src/binutils-gdb/gdb/stack.c:1401 #4 0x000056313993835d in print_frame_info (fp_opts=..., frame=..., print_level=1, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1126 #5 0x0000563139932e0b in print_stack_frame (frame=..., print_level=1, print_what=SRC_AND_LOC, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:368 #6 0x0000563139932bbe in print_stack_frame_to_uiout (uiout=0x611000016840, frame=..., print_level=1, print_what=SRC_AND_LOC, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:346 #7 0x0000563139b0641e in print_selected_thread_frame (uiout=0x611000016840, selection=...) at /home/simark/src/binutils-gdb/gdb/thread.c:1993 #8 0x0000563139940b7f in frame_command_core (fi=..., ignored=true) at /home/simark/src/binutils-gdb/gdb/stack.c:1871 #9 0x000056313994db9e in frame_command_helper::base_command (arg=0x0, from_tty=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1976 Since the user-created frame has level 0 (identified by the saved level -1), lookup_selected_frame just reselects the target's current frame, and the user-created frame is lost. My goal here is to fix this particular problem. Currently, select_frame does not set selected_frame_id and selected_frame_level for frames with level 0. It leaves them at null_frame_id / -1, indicating to restore_selected_frame to use the target's current frame. User-created frames also have level 0, so add a special case them such that select_frame saves their selected id and level. save_selected_frame does not need any change. Change the assertion in restore_selected_frame that checks `frame_level != 0` to account for the fact that we can restore user-created frames, which have level 0. Finally, change lookup_selected_frame to make it able to re-create user-created frame_info objects from selected_frame_level and selected_frame_id. Add a minimal test case for the case described above, that is the "select-frame view" command followed by the "frame" command twice. In order to have a known stack frame to switch to, the test spawns a second thread, and tells the first thread to use the other thread's top frame. Change-Id: Ifc77848dc465fbd21324b9d44670833e09fe98c7 --- gdb/frame.c | 30 ++++++++--- gdb/testsuite/gdb.base/frame-view.c | 74 +++++++++++++++++++++++++++ gdb/testsuite/gdb.base/frame-view.exp | 70 +++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 7 deletions(-) create mode 100644 gdb/testsuite/gdb.base/frame-view.c create mode 100644 gdb/testsuite/gdb.base/frame-view.exp diff --git a/gdb/frame.c b/gdb/frame.c index b9132d0f35a7..9fba29ec13eb 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -69,6 +69,7 @@ set_backtrace_options user_set_backtrace_options; static frame_info_ptr get_prev_frame_raw (frame_info_ptr this_frame); static const char *frame_stop_reason_symbol_string (enum unwind_stop_reason reason); +static frame_info_ptr create_new_frame (frame_id id); /* Status of some values cached in the frame_info object. */ @@ -1669,9 +1670,12 @@ get_current_frame (void) If SELECTED_FRAME_ID / SELECTED_FRAME_LEVEL are null_frame_id / -1, and the target has stack and is stopped, the selected frame is the - current (innermost) frame. This means that SELECTED_FRAME_LEVEL is - never 0 and SELECTED_FRAME_ID is never the ID of the innermost - frame. + current (innermost) target frame. SELECTED_FRAME_ID is never the ID + of the current (innermost) target frame. SELECTED_FRAME_LEVEL may + only be 0 if the selected frame is a user-created one (created and + selected through the "select-frame view" command), in which case + SELECTED_FRAME_ID is the frame id derived from the user-provided + addresses. If SELECTED_FRAME_ID / SELECTED_FRAME_LEVEL are null_frame_id / -1, and the target has no stack or is executing, then there's no @@ -1699,9 +1703,9 @@ void restore_selected_frame (frame_id frame_id, int frame_level) noexcept { - /* save_selected_frame never returns level == 0, so we shouldn't see - it here either. */ - gdb_assert (frame_level != 0); + /* Unless it is a user-created frame, save_selected_frame never returns + level == 0, so we shouldn't see it here either. */ + gdb_assert (frame_level != 0 || frame_id.user_created_p); /* FRAME_ID can be null_frame_id only IFF frame_level is -1. */ gdb_assert ((frame_level == -1 && !frame_id_p (frame_id)) @@ -1735,6 +1739,15 @@ lookup_selected_frame (struct frame_id a_frame_id, int frame_level) return; } + /* This means the selected frame was a user-created one. Create a new one + using the user-provided addresses, which happen to be in the frame id. */ + if (frame_level == 0) + { + gdb_assert (a_frame_id.user_created_p); + select_frame (create_new_frame (a_frame_id)); + return; + } + /* select_frame never saves 0 in SELECTED_FRAME_LEVEL, so we shouldn't see it here. */ gdb_assert (frame_level > 0); @@ -1859,7 +1872,10 @@ select_frame (frame_info_ptr fi) selected_frame = fi; selected_frame_level = frame_relative_level (fi); - if (selected_frame_level == 0) + + /* If the frame is a user-created one, save its level and frame id just like + any other non-level-0 frame. */ + if (selected_frame_level == 0 && !fi->this_id.value.user_created_p) { /* Treat the current frame especially -- we want to always save/restore it without warning, even if the frame ID changes diff --git a/gdb/testsuite/gdb.base/frame-view.c b/gdb/testsuite/gdb.base/frame-view.c new file mode 100644 index 000000000000..7b1cbd6abd42 --- /dev/null +++ b/gdb/testsuite/gdb.base/frame-view.c @@ -0,0 +1,74 @@ +/* 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 . */ + +#include +#include + +struct type_1 +{ + int m; +}; + +struct type_2 +{ + int n; +}; + +static int +baz (struct type_1 z1, struct type_2 z2) +{ + return z1.m + z2.n; +} + +static int +bar (struct type_1 y1, struct type_2 y2) +{ + return baz (y1, y2); +} + +static int +foo (struct type_1 x1, struct type_2 x2) +{ + return bar (x1, x2); +} + +static void * +thread_func (void *p) +{ + struct type_1 t1; + struct type_2 t2; + t1.m = 11; + t2.n = 11; + foo (t1, t2); + + return NULL; +} + +int +main (void) +{ + pthread_t thread; + int res; + + res = pthread_create (&thread, NULL, thread_func, NULL); + assert (res == 0); + + res = pthread_join (thread, NULL); + assert (res == 0); + + return 0; +} diff --git a/gdb/testsuite/gdb.base/frame-view.exp b/gdb/testsuite/gdb.base/frame-view.exp new file mode 100644 index 000000000000..d2dba143448d --- /dev/null +++ b/gdb/testsuite/gdb.base/frame-view.exp @@ -0,0 +1,70 @@ +# 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 . + +# Test the "frame view" family of commands. + +standard_testfile + +if { [build_executable "failed to prepare" \ + ${testfile} ${srcfile}] } { + return +} + +proc test_select_frame_view {} { + clean_restart $::binfile + + if { ![runto_main] } { + return + } + + # Stop thread 2 at a baz. + gdb_test "break baz" + gdb_test "continue" "Thread 2.*hit Breakpoint $::decimal, baz .*" + + # Grab the stack pointer and pc of thread 2's frame. + set frame_sp "" + set frame_pc "" + + gdb_test_multiple "info frame" "" { + -re -wrap ".*frame at ($::hex):.*" { + set frame_sp $expect_out(1,string) + pass $gdb_test_name + } + } + + gdb_test_multiple "print/x \$pc" "" { + -re -wrap " = ($::hex)" { + set frame_pc $expect_out(1,string) + pass $gdb_test_name + } + } + + if { $frame_sp == "" || $frame_pc == "" } { + # Something must have failed and logged a failure above. + return + } + + # Select thread 2's frame in thread 1. + gdb_test "thread 1" "Switching to thread 1 .*" + gdb_test_no_output "select-frame view $frame_sp $frame_pc" + + # Verify that the "frame" command does not change the selected frame. + # There used to be a bug where the "frame" command would lose the + # selection of user-created frames. + gdb_test "frame" "#0 baz \\(z1=.*, z2=.*\\).*" "frame" + gdb_test "frame" "#0 baz \\(z1=.*, z2=.*\\).*" "frame again" +} + +test_select_frame_view -- 2.38.1