From 45f25d6c83c31a48a01ef8293bb3978f5e58e653 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Fri, 18 May 2018 17:45:16 +0100 Subject: [PATCH] gdb: Restore selected frame in print_frame_local_vars PR gdb/23203 reports 'bt full' causing the currently selected frame to change, this issue is fixed in this commit. Add a new class scoped_restore_selected_frame that saves and restores the selected frame. Make use of this in print_frame_local_vars to restore the selected frame on exit. gdb/ChangeLog: PR gdb/23203 * frame.c (scoped_restore_selected_frame::scoped_restore_selected_frame): Define. (scoped_restore_selected_frame::~scoped_restore_selected_frame): Define. * frame.h (class scoped_restore_selected_frame): New class. * stack.c (print_frame_local_vars): Remove catching and rethrowing of any exception, use scoped_restore_selected_frame to restore the frame instead. gdb/testsuite/ChangeLog: PR gdb/23203 * gdb.base/bt-selected-frame.c: New file. * gdb.base/bt-selected-frame.exp: New file. * lib/gdb.exp (get_current_frame_number): New function. --- gdb/ChangeLog | 13 ++++ gdb/frame.c | 16 +++++ gdb/frame.h | 19 ++++++ gdb/stack.c | 24 ++----- gdb/testsuite/ChangeLog | 7 ++ gdb/testsuite/gdb.base/bt-selected-frame.c | 35 ++++++++++ gdb/testsuite/gdb.base/bt-selected-frame.exp | 72 ++++++++++++++++++++ gdb/testsuite/lib/gdb.exp | 16 +++++ 8 files changed, 182 insertions(+), 20 deletions(-) create mode 100644 gdb/testsuite/gdb.base/bt-selected-frame.c create mode 100644 gdb/testsuite/gdb.base/bt-selected-frame.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d700d6cdf2c..503b34f26a6 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2018-05-24 Andrew Burgess + + PR gdb/23203 + * frame.c + (scoped_restore_selected_frame::scoped_restore_selected_frame): + Define. + (scoped_restore_selected_frame::~scoped_restore_selected_frame): + Define. + * frame.h (class scoped_restore_selected_frame): New class. + * stack.c (print_frame_local_vars): Remove catching and rethrowing + of any exception, use scoped_restore_selected_frame to restore the + frame instead. + 2018-05-24 Pedro Alves * darwin-nat.h (darwin_nat_target::create_inferior): Mark with diff --git a/gdb/frame.c b/gdb/frame.c index 07fa2bc77d6..1d0eb725145 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -269,6 +269,22 @@ frame_stash_invalidate (void) htab_empty (frame_stash); } +/* See frame.h */ +scoped_restore_selected_frame::scoped_restore_selected_frame () +{ + m_fid = get_frame_id (get_selected_frame (NULL)); +} + +/* See frame.h */ +scoped_restore_selected_frame::~scoped_restore_selected_frame () +{ + frame_info *frame = frame_find_by_id (m_fid); + if (frame == NULL) + warning (_("Unable to restore previously selected frame.")); + else + select_frame (frame); +} + /* Flag to control debugging. */ unsigned int frame_debug; diff --git a/gdb/frame.h b/gdb/frame.h index d5800b78c25..a6f7fd8947d 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -164,6 +164,25 @@ struct frame_id int artificial_depth; }; +/* Save and restore the currently selected frame. */ + +class scoped_restore_selected_frame +{ +public: + /* Save the currently selected frame. */ + scoped_restore_selected_frame (); + + /* Restore the currently selected frame. */ + ~scoped_restore_selected_frame (); + + DISABLE_COPY_AND_ASSIGN (scoped_restore_selected_frame); + +private: + + /* The ID of the previously selected frame. */ + struct frame_id m_fid; +}; + /* Methods for constructing and comparing Frame IDs. */ /* For convenience. All fields are zero. This means "there is no frame". */ diff --git a/gdb/stack.c b/gdb/stack.c index 74c92537da4..98a21842e77 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -2018,7 +2018,6 @@ print_frame_local_vars (struct frame_info *frame, int num_tabs, struct print_variable_and_value_data cb_data; const struct block *block; CORE_ADDR pc; - struct gdb_exception except = exception_none; if (!get_frame_pc_if_available (frame, &pc)) { @@ -2042,27 +2041,12 @@ print_frame_local_vars (struct frame_info *frame, int num_tabs, /* Temporarily change the selected frame to the given FRAME. This allows routines that rely on the selected frame instead of being given a frame as parameter to use the correct frame. */ + scoped_restore_selected_frame restore_selected_frame; select_frame (frame); - TRY - { - iterate_over_block_local_vars (block, - do_print_variable_and_value, - &cb_data); - } - CATCH (ex, RETURN_MASK_ALL) - { - except = ex; - } - END_CATCH - - /* Restore the selected frame, and then rethrow if there was a problem. */ - select_frame (frame_find_by_id (cb_data.frame_id)); - if (except.reason < 0) - throw_exception (except); - - /* do_print_variable_and_value invalidates FRAME. */ - frame = NULL; + iterate_over_block_local_vars (block, + do_print_variable_and_value, + &cb_data); if (!cb_data.values_printed) fprintf_filtered (stream, _("No locals.\n")); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 426b43823b7..b2938b1bf1d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2018-05-24 Andrew Burgess + + PR gdb/23203 + * gdb.base/bt-selected-frame.c: New file. + * gdb.base/bt-selected-frame.exp: New file. + * lib/gdb.exp (get_current_frame_number): New function. + 2018-05-24 Maciej W. Rozycki Pedro Alves diff --git a/gdb/testsuite/gdb.base/bt-selected-frame.c b/gdb/testsuite/gdb.base/bt-selected-frame.c new file mode 100644 index 00000000000..708dc69b250 --- /dev/null +++ b/gdb/testsuite/gdb.base/bt-selected-frame.c @@ -0,0 +1,35 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2018 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 +breakpt () +{ + asm ("" ::: "memory"); +} + +void +func () +{ + breakpt (); +} + +int +main () +{ + func (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/bt-selected-frame.exp b/gdb/testsuite/gdb.base/bt-selected-frame.exp new file mode 100644 index 00000000000..a60962f6d6b --- /dev/null +++ b/gdb/testsuite/gdb.base/bt-selected-frame.exp @@ -0,0 +1,72 @@ +# Copyright 2018 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 . + +# Check that the selected stack frame doesn't change after a backtrace. + +standard_testfile + +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } { + return -1 +} + +proc check_selected_frame_after_bt { bt_cmd stack_pattern } { + global binfile + + clean_restart $binfile + + with_test_prefix $bt_cmd { + + if ![runto_main] then { + fail "can't run to main" + return 0 + } + + gdb_breakpoint "breakpt" + gdb_continue_to_breakpoint "breakpt" + + # Visit each frame in the stack and ensure that the selected + # frame doesn't change after executing the backtrace command + # in BT_CMD. + foreach_with_prefix frame {0 1 2} { + if { $frame == 0 } { + gdb_assert {[get_current_frame_number] == $frame} \ + "check frame" + } else { + gdb_test "frame $frame" "#$frame .*" "select frame" + } + + gdb_test "$bt_cmd" $stack_pattern \ + "perform backtrace command" + + gdb_assert {[get_current_frame_number] == $frame} \ + "check frame is still selected" + } + } +} + +check_selected_frame_after_bt "bt" \ + [multi_line \ + "#0 \[^\n\r\]+" \ + "#1 \[^\n\r\]+" \ + "#2 \[^\n\r\]+"] + +check_selected_frame_after_bt "bt full" \ + [multi_line \ + "#0 \[^\n\r\]+" \ + "No locals\." \ + "#1 \[^\n\r\]+" \ + "No locals\." \ + "#2 \[^\n\r\]+" \ + "No locals\."] diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 0f05d043f25..ee66a38e08f 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -5805,6 +5805,22 @@ proc get_var_address { var } { return "" } +# Return the frame number for the currently selected frame +proc get_current_frame_number {{test_name ""}} { + global gdb_prompt + + if { $test_name == "" } { + set test_name "get current frame number" + } + set frame_num -1 + gdb_test_multiple "frame" $test_name { + -re "#(\[0-9\]+) .*$gdb_prompt $" { + set frame_num $expect_out(1,string) + } + } + return $frame_num +} + # Get the current value for remotetimeout and return it. proc get_remotetimeout { } { global gdb_prompt -- 2.30.2