From patchwork Tue Feb 25 15:13:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 38308 Received: (qmail 73417 invoked by alias); 25 Feb 2020 15:13:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 73245 invoked by uid 89); 25 Feb 2020 15:13:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=Private, remembers, fence, 1.2 X-HELO: mail-wm1-f50.google.com Received: from mail-wm1-f50.google.com (HELO mail-wm1-f50.google.com) (209.85.128.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Feb 2020 15:13:46 +0000 Received: by mail-wm1-f50.google.com with SMTP id a6so3514593wme.2 for ; Tue, 25 Feb 2020 07:13:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=ORy4VgtjbuA4cacV5ToLJxjxkDb7D5zoeMtvP+wNVa0=; b=UuCisGdDguweqq5a54Kx4CIeCQRpj68dprxMtdGdHiJCbjbLTP651kSll1ymsIpXc3 bU1WoFRjA/Nu9sEKVAc3E+NpfywI706UQzJgXKycK+Bwps1Vjr5WQ7oYg1wG7bFXldvW 15KYc9Ga6kC26ZksY52hdOfqdOSKguT3WJHldxW/XiAQ7hvKRPKBZ0joKpDyk6NJhACc JZsGN0P0jQ3hovBySwtVzOLz1ocWJFW2bXvE+ARKHWK8i7aMZ5R2UBGSLlVkHWv/Bpr0 zv9SH54bzQwTNblVVqa2hrDTKDsKv9XMlvYND68Y+dOukfOYE8Sm6sZA2fADZ63EgWGk at4w== Return-Path: Received: from localhost (host86-186-80-160.range86-186.btcentralplus.com. [86.186.80.160]) by smtp.gmail.com with ESMTPSA id x21sm4218291wmi.30.2020.02.25.07.13.41 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 25 Feb 2020 07:13:41 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Richard Bunt , Andrew Burgess Subject: [RFC 2/2] gdb: Track the current frame for each thread Date: Tue, 25 Feb 2020 15:13:37 +0000 Message-Id: <609a13c03a900a06e3d32eb0afd8f240cd9188aa.1582643363.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes Currently in GDB the selected frame is only tracked while a user is within a selected thread. When the user switches thread, the selected frame is reset. After this commit GDB records the selected frame within its thread_info structure, and restores each threads selected frame when switching thread. If a thread executes at all then the selected frame is discarded, when the thread stops then the innermost frame is selected, this matches the existing behaviour of GDB. The original motivation behind this commit was so a user could switch threads using the 'thread ' command, and have their selected frame be remembered, however, once I started implementing this I realised that there were a couple of places in GDB where the sticky selected frame would possibly also appear that I hadn't originally considered. The first was 'info threads', and the other was 'thread apply all'. With 'info threads' the output contains a 'Frame' column. Previously, this was always the innermost frame, the info threads output was created by switching to each thread in turn and collecting information about the thread, this naturally placed us at the innermost frame. Now, the 'Frame' column displays the _selected_ frame for each thread. I struggled to decide if this change was good or not. In the end I went with the change, my thinking was that if a user visits a thread, changes the frame, moves to another thread, and then wants to find the frame they previously visited, then displaying the selected frame might make this easier. However, I'm still on the fence for if this is a good change or not, so would welcome opinions on this. For 'thread apply all', again, we used to always apply to the innermost frame. Now it's possible for a user to adjust which frame will be current when the 'thread apply all' runs - this feels like a useful change to me. Again, I'm happy to discuss this further if people dislike this change. Further, 'thread apply all' will now allow the current frame to be adjusted within it's applied command, and this change of frame is sticky. There is, of course, a new flag to turn this new behaviour off, the new commands are: set restore-selected-frame on|off show restore-selected-frame I've turned this new feature on by default for now, but again, I'm open to being convinced that it should be off by default. gdb/ChangeLog: * NEWS: Describe new feature. * frame.c (cache_selected_frame_on_thread): New function. (select_frame): Call new function. * gdbthread.h (class thread_info) : New member variable. (switch_to_thread): Extra parameter. * thread.c (switch_to_thread_if_alive): Extra parameter, passed to switch_to_thread. (scoped_restore_current_thread::restore): Restore the frame either from the thread, or from the local object. (set_executing_thread): Reset the currently selected frame. (restore_selected_frame_per_thread): New file level static variable. (show_restore_selected_frame_per_thread): New function. (print_thread_info_1): Pass extra parameter to switch_to_thread. (switch_to_thread): Take extra parameter, restore the previous frame if appropriate. (thread_apply_all_command): Pass extra parameter to switch_to_thread. (thread_apply_command): Likewise. (thread_select): Pass extra parameter to switch_to_thread_if_alive. (_initialize_thread): Add new set/show variable. gdb/doc/ChangeLog: * gdb.texinfo (Threads): Extend description of 'Frame' field in info threads output. Add description of how GDB tracks the previous frame. (Selection): Add description of how GDB tracks the selected frame per thread, and describe the new commands to disable this tracking. gdb/testsuite/ChangeLog: * gdb.mi/user-selected-context-sync.exp: Update expected results. * gdb.threads/restore-selected-frame.c: New file. * gdb.threads/restore-selected-frame.exp: New file. --- gdb/ChangeLog | 23 ++ gdb/NEWS | 12 + gdb/doc/ChangeLog | 9 + gdb/doc/gdb.texinfo | 33 ++- gdb/frame.c | 26 ++ gdb/gdbthread.h | 12 +- gdb/testsuite/ChangeLog | 6 + .../gdb.mi/user-selected-context-sync.exp | 16 +- gdb/testsuite/gdb.threads/restore-selected-frame.c | 85 ++++++ .../gdb.threads/restore-selected-frame.exp | 322 +++++++++++++++++++++ gdb/thread.c | 61 +++- 11 files changed, 584 insertions(+), 21 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.c create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.exp diff --git a/gdb/NEWS b/gdb/NEWS index e33d838dd18..88a936d8d5d 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -19,6 +19,13 @@ * TUI windows can now be arranged horizontally. +* GDB now tracks the currently selected frame for each thread. When + switching threads the last frame selected in that thread will be + restored. Executing a thread will cause the GDB to discard any + previously selected frame (GDB will select the inner most frame the + next time the thread stops). The 'info threads' commands now shows + the selected frame in its 'frame' field. + * New commands set exec-file-mismatch -- Set exec-file-mismatch handling (ask|warn|off). @@ -36,6 +43,11 @@ tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]... Define a new TUI layout, specifying its name and the windows that will be displayed. +set restore-selected-frame [on|off] +show restore-selected-frame + When switching threads, restore the previously selected frame in the + thread being switched too. This is on by default. + * New targets GNU/Linux/RISC-V (gdbserver) riscv*-*-linux* diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f1798e35b5f..123f3f80268 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -3459,7 +3459,12 @@ program itself. @item -the current stack frame summary for that thread +the current stack frame summary for that thread, this is the currently +selected stack frame (@pxref{Frames, ,Frames}, for a description of +stack frames) in this thread. When @code{restore-selected-frame} is +@code{off} (@pxref{set restore-selected-frame}) then all threads +except the currently selected thread will display their innermost +frame. @end enumerate @noindent @@ -3528,6 +3533,14 @@ @samp{Switching to} depends on your system's conventions for identifying threads. +@value{GDBN} will track the last frame selected (@pxref{Frames, +,Frames}, for a description of stack frames) in each thread and will +restore the frame when switching back to a thread. Any time that a +thread executes then the currently selected frame will be reset to the +inner most frame (@pxref{set restore-selected-frame,,set +restore-selected-frame} for how to disable restoring the last selected +frame). + @anchor{thread apply all} @kindex thread apply @cindex apply command to several threads @@ -8229,6 +8242,24 @@ distracting. @end table +@anchor{set restore-selected-frame} +@value{GDBN} remembers the currently selected frame for each thread +(@pxref{Threads, ,Threads}, for a description of threads) in the +inferior. When switching thread the previously selected frame in each +thread is restored. + +@table @code +@item set restore-selected-frame @r{[}on|off@r{]} +@itemx show restore-selected-frame +When @code{restore-selected-frame} is on, which it is by default, +@value{GDBN} will restore the previously selected frame when switching +to a different thread. + +If a thread has been running then when it stops the previously +selected frame is discarded, and the inner most frame is again +selected. +@end table + @node Frame Info @section Information About a Frame diff --git a/gdb/frame.c b/gdb/frame.c index 863df107009..f9fde2397ad 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1704,12 +1704,38 @@ deprecated_safe_get_selected_frame (void) return get_selected_frame (NULL); } +/* When RESTORE_SELECTED_FRAME_PER_THREAD is true, then update in the + current thread the information required to identify frame FI so the + frame can be selected again later if we switch threads. */ + +static void +cache_selected_frame_on_thread () +{ + struct frame_info *fi = selected_frame; + struct thread_info *tp + = find_thread_ptid (current_inferior (), inferior_ptid); + if (fi != nullptr && tp != nullptr) + { + /* We only record the selected frame if the level is greater than 0, + this avoids having to calculate the frame id when selecting the + innermost frame. When the cached selected frame is cleared then + we select the innermost frame anyway, so calculating the frame id + for frame #0 adds no value. */ + if (frame_relative_level (fi) > 0) + tp->selected_frame_info.reset (fi); + else + tp->selected_frame_info.reset (); + } +} + /* Select frame FI (or NULL - to invalidate the current frame). */ void select_frame (struct frame_info *fi) { selected_frame = fi; + cache_selected_frame_on_thread (); + /* NOTE: cagney/2002-05-04: FI can be NULL. This occurs when the frame is being invalidated. */ diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 26e572d1e98..0f046b09978 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -387,6 +387,11 @@ public: bp_longjmp_call_dummy. */ struct frame_id initiating_frame = null_frame_id; + /* Information for the last frame successfully selected in this thread. + If the user configurable setting is on then GDB will try to reselect + this frame when switching threads. */ + struct frame_id_and_level selected_frame_info; + /* Private data used by the target vector implementation. */ std::unique_ptr priv; @@ -581,8 +586,11 @@ extern int thread_count (process_stratum_target *proc_target); /* Return true if we have any thread in any inferior. */ extern bool any_thread_p (); -/* Switch context to thread THR. Also sets the STOP_PC global. */ -extern void switch_to_thread (struct thread_info *thr); +/* Switch context to thread THR. Also sets the STOP_PC global. When + RESTORE_PREVIOUS_FRAME is true then, if this thread has a previously + selected frame cached, the previous frame is restored. */ +extern void switch_to_thread (struct thread_info *thr, + bool restore_previous_frame = false); /* Switch context to no thread selected. */ extern void switch_to_no_thread (); diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp index 390df005542..9deac664f98 100644 --- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp +++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp @@ -888,8 +888,8 @@ proc_with_prefix test_mi_thread_select { mode } { # running on non-stop. if { $mode == "all-stop" } { - set mi_re [make_mi_re $mode 3 0 response] - set cli_re [make_cli_re $mode -1 1.3 0] + set mi_re [make_mi_re $mode 3 1 response] + set cli_re [make_cli_re $mode -1 1.3 1] } else { set mi_re [make_mi_re $mode 3 -1 response] set cli_re [make_cli_re $mode -1 1.3 -1] @@ -1056,8 +1056,8 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } { # Do the 'thread' command to select a stopped thread. set command [make_cli_in_mi_command $cli_in_mi_mode "thread 1.2"] - set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 1 -1 1.2 2 0] - set cli_re [make_cli_re $mode -1 1.2 0] + set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 1 -1 1.2 2 1] + set cli_re [make_cli_re $mode -1 1.2 1] with_spawn_id $mi_spawn_id { mi_gdb_test $command $mi_re "select thread" @@ -1070,7 +1070,7 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } { # Do the 'thread' command to select the same thread. We shouldn't # receive an event on CLI, since we won't actually switch thread. - set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.2 2 0] + set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.2 2 1] set cli_re "" with_spawn_id $mi_spawn_id { @@ -1103,8 +1103,8 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } { set command [make_cli_in_mi_command $cli_in_mi_mode "thread 1.3"] if { $mode == "all-stop" } { - set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 1 -1 1.3 3 0] - set cli_re [make_cli_re $mode -1 "1.3" 0] + set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 1 -1 1.3 3 1] + set cli_re [make_cli_re $mode -1 "1.3" 1] } else { set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 1 -1 1.3 3 -1] set cli_re [make_cli_re $mode -1 "1.3" -1] @@ -1122,7 +1122,7 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } { # shouldn't receive an event on MI. if { $mode == "all-stop" } { - set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.3 3 0] + set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.3 3 1] } else { set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.3 3 -1] } diff --git a/gdb/testsuite/gdb.threads/restore-selected-frame.c b/gdb/testsuite/gdb.threads/restore-selected-frame.c new file mode 100644 index 00000000000..c72b0b8b54b --- /dev/null +++ b/gdb/testsuite/gdb.threads/restore-selected-frame.c @@ -0,0 +1,85 @@ +#include +#include +#include +#include +#include + +volatile int loop_count = 10; +volatile int thread_count = 3; + +static void +thread_level_5 (int id, int count) +{ + printf ("Thread %d reached %s, #%d\n", + id, __PRETTY_FUNCTION__, count); +} + +static void +thread_level_4 (int id, int count) +{ + thread_level_5 (id, count); +} + +static void +thread_level_3 (int id, int count) +{ + thread_level_4 (id, count); +} + +static void +thread_level_2 (int id, int count) +{ + thread_level_3 (id, count); +} + +static void +thread_level_1 (int id, int count) +{ + thread_level_2 (id, count); +} + +static void * +thread_worker (void *arg) +{ + int i, max, id; + + id = *((int *) arg); + max = loop_count; + for (i = 0; i < max; ++i) + thread_level_1 (id, (i + 1)); + + return NULL; +} + +struct thread_info +{ + pthread_t thread; + int id; +}; + +int +main () +{ + int i, max = thread_count; + + struct thread_info *info = malloc (sizeof (struct thread_info) * max); + if (info == NULL) + abort (); + + for (i = 0; i < max; ++i) + { + struct thread_info *thr = &info[i]; + thr->id = i + 1; + if (pthread_create (&thr->thread, NULL, thread_worker, &thr->id) != 0) + abort (); + } + + for (i = 0; i < max; ++i) + { + struct thread_info *thr = &info[i]; + if (pthread_join (thr->thread, NULL) != 0) + abort (); + } + + free (info); +} diff --git a/gdb/testsuite/gdb.threads/restore-selected-frame.exp b/gdb/testsuite/gdb.threads/restore-selected-frame.exp new file mode 100644 index 00000000000..00e81bf449b --- /dev/null +++ b/gdb/testsuite/gdb.threads/restore-selected-frame.exp @@ -0,0 +1,322 @@ +# Copyright 2020 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 . + +# This tests GDB's tracking of the currently selected frame on a +# per-thread basis. +# +# We setup a couple of inferiors, each with mutliple theads, we then +# switch between threads and modify the current frame. We use 'info +# threads' to check that GDB is correctly tracking the current frame. +# +# Toward the end of the test we check that when a thread executes the +# currently selected frame is reset. +# +# Finally we disable tracking of the currently selected frame and +# ensure GDB no longer restores the current frame. + +standard_testfile + +set options { debug pthreads } +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ + $options] == -1} { + return -1 +} + +# Run the 'info threads' command, and check that the frame part of +# each threads output matches the corresponding pattern in FRAME_INFO, +# with thread 1 using entry 0 from FRAME_INFO, thread 2 using entry 1, +# and so on. +proc test_info_threads { testname frame_info } { + global decimal hex gdb_prompt + + set thread_count 0 + gdb_test_multiple "info threads" ${testname} { + -re ".* Id\\s+Target Id\\s+Frame\\s*\r\n" { + # Discard the info threads header line as well as any + # output before it in the expect buffer. + exp_continue + } + + -re "^\[* \]\\s+(($decimal)\.)?($decimal)\\s+Thread $hex \\(LWP $decimal\\) \"\[^\"\]+\"\\s+(\[^\r\n\]*)\r\n" { + if {[info exists expect_out(2,string)]} { + set id "$expect_out(2,string).$expect_out(3,string)" + set index [expr [expr [expr $expect_out(2,string) - 1] * 4] \ + + [expr $expect_out(3,string) - 1]] + } else { + set id $expect_out(3,string) + set index [expr $id - 1] + } + set frame $expect_out(4,string) + set pattern [lindex $frame_info $index] + gdb_assert {[regexp -- $pattern $frame]} \ + "$testname: thread $id matches" + incr thread_count + exp_continue + } + -re "^$gdb_prompt " { + } + } + gdb_assert {$thread_count == [llength $frame_info]} \ + "$testname: all threads seen" +} + +# Run 'thread THREAD_NUM' and check that we switch thread. +proc switch_thread { thread_num } { + gdb_test "thread ${thread_num}" \ + "Switching to thread (2\.)?${thread_num} .*" +} + +# Used during startup, continue the inferior and wait for all threads +# to stop at the breakpoint. +proc run_all_threads_to_breakpoint { } { + global gdb_prompt + + set stopped_thread_count 0 + gdb_test_multiple "continue" "wait for worker threads to stop" { + -re "Thread (2\.)?\[234\] \"\[^\"\]+\" hit Breakpoint" { + incr stopped_thread_count + if {$stopped_thread_count < 3} { + exp_continue + } + } + + -re "$gdb_prompt" { + exp_continue + } + } + + gdb_assert {$stopped_thread_count == 3} \ + "all worker threads stopped" +} + +# Switch to thread #1, and interrupt it. +proc switch_to_and_stop_thread_1 {} { + global gdb_prompt + # There's a bit of a wart here in that after sending "interrupt" + # the output seems to appear out of order this is probably a + # consequence of being in non-stop mode, so this is what I'd like + # to see: + # + # (gdb) interrupt + # Thread 1 "...." stopped. + # (gdb) + # + # But what we actually see is: + # + # (gdb) interrupt + # (gdb) + # Thread 1 "...." stopped. + # + # What happens of course is that GDB processes the interrupt, + # sends a SIGSTOP to the inferior and then returns to the prompt, + # at this point we process the stop event from the inferior and + # print the stopped message. + # + # It would be nice if GDB could be smart enough to reprint the + # prompt after the stop message though. + # + # The first 'interrupt\n' here causes the interior to stop, while + # the following lone '\n' causes the prompt to be reprinted. This + # allows us to match all the output up to the final prompt, + # ensuring we don't leave any stray output in expect's output + # buffer. + switch_thread 1 + send_gdb "interrupt\n" + gdb_test_multiple "" "wait for thread 1 to stop" { + -re "Thread (2\.)?1 \"\[^\"\]+\" stopped\." { + send_gdb "\n" + gdb_test_multiple "" \ + "wait for prompt after thread 1 stopped" { + -re ".*$gdb_prompt " { + pass $gdb_test_name + } + } + } + } +} + +# Setup for this test. Place GDB in non-stop mode, create an initial +# breakpoint, run all of the threads to the breakpoint, then stop +# thread 1 (which doesn't hit the breakpoint). +proc setup_for_test {} { + gdb_test_no_output "set non-stop on" + + if ![runto_main] { + fail "runto main" + return + } + + gdb_breakpoint "thread_level_5" + + with_test_prefix "setup inferior 1" { + # Now run the inferior, and wait for all of the expected threads + # to hit the thread_level_5 breakpoint. + run_all_threads_to_breakpoint + + # The main thread will still be running at this point, waiting for + # the stopped threads to finish so it can join with them. Lets go + # and interrupt it. + switch_to_and_stop_thread_1 + } +} + +setup_for_test + +set frame_info [list "$hex in pthread_join" \ + "thread_level_5" \ + "thread_level_5" \ + "thread_level_5" ] + + +# We now have all threads stopped in known locations. Lets check that +# everyone is where we expect them to be. +test_info_threads "info threads #1" $frame_info + +# First, lets move thread 1. Then check that the info threads output +# reflects this. +gdb_test "up" ".*" +set frame_info [lreplace $frame_info 0 0 "$hex in main"] +test_info_threads "info threads #2" $frame_info + +# Now lets change the other threads, one at a time, checking the +# output of info threads after each change. +foreach spec [list [list 2 5 "$hex in thread_worker"] \ + [list 3 3 "$hex in thread_level_2"] \ + [list 4 1 "$hex in thread_level_4"] ] { + set thr [lindex $spec 0] + with_test_prefix "change frame for thread $thr" { + switch_thread $thr + gdb_test "frame [lindex $spec 1]" ".*" + set idx [expr $thr - 1] + set frame_info [lreplace $frame_info $idx $idx [lindex $spec 2]] + test_info_threads "info threads #3" $frame_info + } +} + +# Start a new inferior, and runto main. +gdb_test "add-inferior" "Added inferior 2 .*" \ + "add empty inferior 2" +gdb_test "inferior 2" "Switching to inferior 2 .*" \ + "switch to inferior 2" +gdb_test "file ${binfile}" ".*" "load file in inferior 2" + +with_test_prefix "start inferior 2" { + # Disable deleting of breakpoints. + proc delete_breakpoints {} {} + runto_main +} + +with_test_prefix "setup inferior 2" { + run_all_threads_to_breakpoint + switch_to_and_stop_thread_1 +} + +set frame_info [concat $frame_info [list "$hex in pthread_join" \ + "thread_level_5" \ + "thread_level_5" \ + "thread_level_5" ]] +test_info_threads "info threads #4" $frame_info + +# Now lets change the other threads, one at a time, checking the +# output of info threads after each change. +foreach spec [list [list 2 2 "$hex in thread_level_3"] \ + [list 3 2 "$hex in thread_level_3"] \ + [list 4 2 "$hex in thread_level_3"] ] { + set thr [lindex $spec 0] + with_test_prefix "change frame for thread $thr" { + switch_thread "2.$thr" + gdb_test "frame [lindex $spec 1]" ".*" + set idx [expr 4 + $thr - 1] + set frame_info [lreplace $frame_info $idx $idx [lindex $spec 2]] + test_info_threads "info threads #5" $frame_info + } +} + +# Now step one of the threads. The thread that is stepped should +# discard its stored selected frame, but all other threads should +# retain their selected frame. +switch_thread "2.2" +gdb_test "step" ".*" \ + "step in thread 2.2" +set frame_info [lreplace $frame_info 5 5 "thread_level_5"] +test_info_threads "info threads #6" $frame_info + +# Same again for a thread in inferior #1. +switch_thread "1.3" +gdb_test "step" ".*" \ + "step in thread 1.3" +set frame_info [lreplace $frame_info 2 2 "thread_level_5"] +test_info_threads "info threads #7" $frame_info + +# Now switch to another thread that already has a frame other than its +# innermost selected. +switch_thread "1.2" + +# Now disable restoring of the selected frame. +gdb_test_no_output "set restore-selected-frame off" + +# And check to see which frame each thread has selected. Our current +# thread shouldn't change. +set frame_info [list "$hex in pthread_join" \ + "thread_worker" \ + "thread_level_5" \ + "thread_level_5" \ + "$hex in pthread_join" \ + "thread_level_5" \ + "thread_level_5" \ + "thread_level_5"] +test_info_threads "info threads #8" $frame_info + +# Now switch to some other thread, at this point GDB should forget the +# selected frame for thread 1.2. +switch_thread "1.4" +set frame_info [lreplace $frame_info 1 1 "thread_level_5"] +test_info_threads "info threads #9" $frame_info + +# A new test that will cover 'thread apply all'. This test ensures +# that any changes to the selected thread in 'thread apply all' are +# sticky outside of the 'thread apply all'. +with_test_prefix "thr apply all" { + clean_restart $binfile + setup_for_test + + # Move all threads up a frame. + gdb_test "thread apply all -- up" ".*" + set frame_info [list "$hex in main" \ + "$hex in thread_level_4" \ + "$hex in thread_level_4" \ + "$hex in thread_level_4" ] + test_info_threads "info threads #10" $frame_info + + # Move every thread back to frame 0. + gdb_test "thread apply all -- frame 0" ".*" + set frame_info [list "$hex in pthread_join" \ + "thread_level_5" \ + "thread_level_5" \ + "thread_level_5" ] + test_info_threads "info threads #11" $frame_info + + # Disable restoring the current frame. + gdb_test_no_output "set restore-selected-frame off" + + # Move all threads up a frame, no frame should change after this + # though. + gdb_test "thread apply all -- up" ".*" + set frame_info [list "$hex in pthread_join" \ + "thread_level_5" \ + "thread_level_5" \ + "thread_level_5" ] + test_info_threads "info threads #12" $frame_info +} diff --git a/gdb/thread.c b/gdb/thread.c index d0c795c5864..684ad6f93b0 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -698,7 +698,7 @@ thread_alive (thread_info *tp) switched, false otherwise. */ static bool -switch_to_thread_if_alive (thread_info *thr) +switch_to_thread_if_alive (thread_info *thr, bool restore_previous_frame) { scoped_restore_current_thread restore_thread; @@ -708,7 +708,7 @@ switch_to_thread_if_alive (thread_info *thr) if (thread_alive (thr)) { - switch_to_thread (thr); + switch_to_thread (thr, restore_previous_frame); restore_thread.dont_restore (); return true; } @@ -880,7 +880,10 @@ set_executing_thread (thread_info *thr, bool executing) { thr->executing = executing; if (executing) - thr->suspend.stop_pc = ~(CORE_ADDR) 0; + { + thr->suspend.stop_pc = ~(CORE_ADDR) 0; + thr->selected_frame_info.reset (); + } } void @@ -1043,6 +1046,23 @@ thread_target_id_str (thread_info *tp) return target_id; } +/* When this is true GDB restore the threads previously selected frame + each time the current thread is changed (when possible). */ + +static bool restore_selected_frame_per_thread = true; + +/* Implement 'show restore-selected-frame'. */ + +static void +show_restore_selected_frame_per_thread (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + fprintf_filtered (file, + _("Restoring the selected frame is currently %s.\n"), + value); +} + /* Like print_thread_info, but in addition, GLOBAL_IDS indicates whether REQUESTED_THREADS is a list of global or per-inferior thread ids. */ @@ -1155,7 +1175,8 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, uiout->field_signed ("id", tp->global_num); /* Switch to the thread (and inferior / target). */ - switch_to_thread (tp); + switch_to_thread (tp, (tp == current_thread + || restore_selected_frame_per_thread)); /* For the CLI, we stuff everything into the target-id field. This is a gross hack to make the output come out looking @@ -1335,7 +1356,7 @@ switch_to_no_thread () /* See gdbthread.h. */ void -switch_to_thread (thread_info *thr) +switch_to_thread (thread_info *thr, bool restore_previous_frame) { gdb_assert (thr != NULL); @@ -1345,6 +1366,10 @@ switch_to_thread (thread_info *thr) switch_to_thread_no_regs (thr); reinit_frame_cache (); + + if (restore_previous_frame && thr->selected_frame_info.level () > -1) + restore_selected_frame (thr->selected_frame_info.id (), + thr->selected_frame_info.level ()); } /* See gdbsupport/common-gdbthread.h. */ @@ -1368,13 +1393,16 @@ scoped_restore_current_thread::restore () in the mean time exited (or killed, detached, etc.), then don't revert back to it, but instead simply drop back to no thread selected. */ && m_inf->pid != 0) - switch_to_thread (m_thread); + switch_to_thread (m_thread, restore_selected_frame_per_thread); else switch_to_inferior_no_thread (m_inf); /* The running state of the originally selected thread may have - changed, so we have to recheck it here. */ + changed, so we have to recheck it here. We only restore the frame + here if we didn't restore the threads selected frame when switching + thread above (see use of RESTORE_SELECTED_FRAME_PER_THREAD). */ if (inferior_ptid != null_ptid + && !restore_selected_frame_per_thread && m_was_stopped && m_thread->state == THREAD_STOPPED && target_has_registers @@ -1639,7 +1667,7 @@ thread_apply_all_command (const char *cmd, int from_tty) scoped_restore_current_thread restore_thread; for (thread_info *thr : thr_list_cpy) - if (switch_to_thread_if_alive (thr)) + if (switch_to_thread_if_alive (thr, restore_selected_frame_per_thread)) thr_try_catch_cmd (thr, cmd, from_tty, flags); } } @@ -1796,7 +1824,7 @@ thread_apply_command (const char *tidlist, int from_tty) continue; } - if (!switch_to_thread_if_alive (tp)) + if (!switch_to_thread_if_alive (tp, restore_selected_frame_per_thread)) { warning (_("Thread %s has terminated."), print_thread_id (tp)); continue; @@ -1964,7 +1992,7 @@ show_print_thread_events (struct ui_file *file, int from_tty, void thread_select (const char *tidstr, thread_info *tp) { - if (!switch_to_thread_if_alive (tp)) + if (!switch_to_thread_if_alive (tp, restore_selected_frame_per_thread)) error (_("Thread ID %s has terminated."), tidstr); annotate_thread_changed (); @@ -2231,6 +2259,19 @@ Show printing of thread events (such as thread start and exit)."), NULL, show_print_thread_events, &setprintlist, &showprintlist); + add_setshow_boolean_cmd ("restore-selected-frame", + class_stack, &restore_selected_frame_per_thread, + _("\ +Set whether GDB restores the selected frame when switching threads."), _("\ +Show whether GDB restores the selected frame when switching threads."), _("\ +When this option is on GDB will record the currently selected frame for\n\ +each thread, and restore the selected frame whenever GDB switches thread.\n\ +Causing a thread to execute will invalidate the selected frame."), + nullptr, + show_restore_selected_frame_per_thread, + &setlist, + &showlist); + create_internalvar_type_lazy ("_thread", &thread_funcs, NULL); create_internalvar_type_lazy ("_gthread", >hread_funcs, NULL); }