From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1879) id 85043385842D; Fri, 20 Jan 2023 19:52:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 85043385842D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674244377; bh=HzxCZs0B8C6LqXeIWuMgcK54wWI5XDwIYkT0/E4RFtk=; h=From:To:Subject:Date:From; b=B9X4Lbv9VilpCtuCzPWipVlgNm4yKa1xfCdIBuRzrnjNPSI41o5OAWy2lPWrSwHPq mOqIkG8UF98dXPbY/9C/Tc0dtRLDst/ckv4TP6ygMykd9j1TUcgQdaA64SuFPNAAjw zRUb3v4S4ZNx+lMsbbBhx/eZLbHrW5YrqzQLxoLA= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Simon Marchi To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb: make it possible to restore selected user-created frames X-Act-Checkin: binutils-gdb X-Git-Author: Simon Marchi X-Git-Refname: refs/heads/master X-Git-Oldrev: d015d3206e11c6926c4afce723d8366afc965b97 X-Git-Newrev: bc2cbe815bdbac3bd027bf4acc94c554c29b0189 Message-Id: <20230120195257.85043385842D@sourceware.org> Date: Fri, 20 Jan 2023 19:52:57 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Dbc2cbe815bdb= ac3bd027bf4acc94c554c29b0189 commit bc2cbe815bdbac3bd027bf4acc94c554c29b0189 Author: Simon Marchi Date: Tue Dec 13 22:34:38 2022 -0500 gdb: make it possible to restore selected user-created frames =20 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. =20 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: =20 $ ./gdb --data-directory=3Ddata-directory -nx -q testsuite/outputs/= gdb.base/frame-view/frame-view Reading symbols from testsuite/outputs/gdb.base/frame-view/frame-vi= ew... (gdb) break thread_func Breakpoint 1 at 0x11a2: file /home/simark/src/binutils-gdb/gdb/test= suite/gdb.base/frame-view.c, line 42. (gdb) run Starting program: /home/simark/build/binutils-gdb/gdb/testsuite/out= puts/gdb.base/frame-view/frame-view =20 [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)] =20 Thread 2 "frame-view" hit Breakpoint 1, thread_func (p=3D0x0) at /h= ome/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 =3D 0x5555555551a2 in thread_func (/home/simark/src/binutils-g= db/gdb/testsuite/gdb.base/frame-view.c:42); saved rip =3D 0x7ffff7d4e8fd called by frame at 0x7ffff7cc3f80 source language c. Arglist at 0x7ffff7cc3ed0, args: p=3D0x0 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 =20 Here, we create a custom frame for thread 1 (using the stack from thread 2, for convenience): =20 (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2 =20 The first calls to "frame" looks good: =20 (gdb) frame #0 thread_func (p=3D0x7ffff7d4e630) at /home/simark/src/binutils-g= db/gdb/testsuite/gdb.base/frame-view.c:42 42 foo (11); =20 But not the second one: =20 (gdb) frame #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 =20 This second "frame" command shows the current target frame instead of the user-created frame. =20 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: =20 (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. =20 and "backtrace" always shows the actual thread's backtrace, it ignores the user-created frame: =20 (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 =20 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. =20 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. =20 #0 save_selected_frame (frame_id=3D0x7ffdc0020ad0, frame_level=3D0= x7ffdc0020af0) at /home/simark/src/binutils-gdb/gdb/frame.c:1682 #1 0x00005631390242f0 in scoped_restore_selected_frame::scoped_res= tore_selected_frame (this=3D0x7ffdc0020ad0) at /home/simark/src/binutils-gd= b/gdb/frame.c:324 #2 0x000056313993581e in print_frame_args (fp_opts=3D..., func=3D0= x62100023bde0, frame=3D..., num=3D-1, stream=3D0x60b000000300) at /home/sim= ark/src/binutils-gdb/gdb/stack.c:755 #3 0x000056313993ad49 in print_frame (fp_opts=3D..., frame=3D..., = print_level=3D1, print_what=3DSRC_AND_LOC, print_args=3D1, sal=3D...) at /h= ome/simark/src/binutils-gdb/gdb/stack.c:1401 #4 0x000056313993835d in print_frame_info (fp_opts=3D..., frame=3D= ..., print_level=3D1, print_what=3DSRC_AND_LOC, print_args=3D1, set_current= _sal=3D1) at /home/simark/src/binutils-gdb/gdb/stack.c:1126 #5 0x0000563139932e0b in print_stack_frame (frame=3D..., print_lev= el=3D1, print_what=3DSRC_AND_LOC, set_current_sal=3D1) at /home/simark/src/= binutils-gdb/gdb/stack.c:368 #6 0x0000563139932bbe in print_stack_frame_to_uiout (uiout=3D0x611= 000016840, frame=3D..., print_level=3D1, print_what=3DSRC_AND_LOC, set_curr= ent_sal=3D1) at /home/simark/src/binutils-gdb/gdb/stack.c:346 #7 0x0000563139b0641e in print_selected_thread_frame (uiout=3D0x61= 1000016840, selection=3D...) at /home/simark/src/binutils-gdb/gdb/thread.c:= 1993 #8 0x0000563139940b7f in frame_command_core (fi=3D..., ignored=3Dt= rue) at /home/simark/src/binutils-gdb/gdb/stack.c:1871 #9 0x000056313994db9e in frame_command_helper:= :base_command (arg=3D0x0, from_tty=3D1) at /home/simark/src/binutils-gdb/gd= b/stack.c:1976 =20 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. =20 My goal here is to fix this particular problem. =20 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. =20 save_selected_frame does not need any change. =20 Change the assertion in restore_selected_frame that checks `frame_level !=3D 0` to account for the fact that we can restore user-created frames, which have level 0. =20 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. =20 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. =20 Change-Id: Ifc77848dc465fbd21324b9d44670833e09fe98c7 Reviewed-By: Bruno Larsen Diff: --- 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(-) diff --git a/gdb/frame.c b/gdb/frame.c index 0909109c1f4..9ab8fa0310e 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -69,6 +69,7 @@ set_backtrace_options user_set_backtrace_options; =20 static frame_info_ptr get_prev_frame_raw (frame_info_ptr this_frame); static const char *frame_stop_reason_symbol_string (enum unwind_stop_reaso= n reason); +static frame_info_ptr create_new_frame (frame_id id); =20 /* Status of some values cached in the frame_info object. */ =20 @@ -1669,9 +1670,12 @@ get_current_frame (void) =20 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. =20 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 =3D=3D 0, so we shouldn't see - it here either. */ - gdb_assert (frame_level !=3D 0); + /* Unless it is a user-created frame, save_selected_frame never returns + level =3D=3D 0, so we shouldn't see it here either. */ + gdb_assert (frame_level !=3D 0 || frame_id.user_created_p); =20 /* FRAME_ID can be null_frame_id only IFF frame_level is -1. */ gdb_assert ((frame_level =3D=3D -1 && !frame_id_p (frame_id)) @@ -1735,6 +1739,15 @@ lookup_selected_frame (struct frame_id a_frame_id, i= nt frame_level) return; } =20 + /* This means the selected frame was a user-created one. Create a new o= ne + using the user-provided addresses, which happen to be in the frame id= . */ + if (frame_level =3D=3D 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) =20 selected_frame =3D fi; selected_frame_level =3D frame_relative_level (fi); - if (selected_frame_level =3D=3D 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 =3D=3D 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/f= rame-view.c new file mode 100644 index 00000000000..7b1cbd6abd4 --- /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 =3D 11; + t2.n =3D 11; + foo (t1, t2); + + return NULL; +} + +int +main (void) +{ + pthread_t thread; + int res; + + res =3D pthread_create (&thread, NULL, thread_func, NULL); + assert (res =3D=3D 0); + + res =3D pthread_join (thread, NULL); + assert (res =3D=3D 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 00000000000..d2dba143448 --- /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 " =3D ($::hex)" { + set frame_pc $expect_out(1,string) + pass $gdb_test_name + } + } + + if { $frame_sp =3D=3D "" || $frame_pc =3D=3D "" } { + # 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=3D.*, z2=3D.*\\).*" "frame" + gdb_test "frame" "#0 baz \\(z1=3D.*, z2=3D.*\\).*" "frame again" +} + +test_select_frame_view