Skip to content

Commit

Permalink
gdb: make it possible to restore selected user-created frames
Browse files Browse the repository at this point in the history
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<frame_command_core>::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
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
  • Loading branch information
simark committed Jan 20, 2023
1 parent d015d32 commit bc2cbe8
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 7 deletions.
30 changes: 23 additions & 7 deletions gdb/frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions gdb/testsuite/gdb.base/frame-view.c
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>. */

#include <pthread.h>
#include <assert.h>

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;
}
70 changes: 70 additions & 0 deletions gdb/testsuite/gdb.base/frame-view.exp
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.

# 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

0 comments on commit bc2cbe8

Please sign in to comment.