[PATCHv4 1/2] gdb: Restore previously selected thread when switching inferior

Andrew Burgess
2020-11-06T23:02:25Z

This commit adds a new option that allows the user to control how GDB
behaves when switching between multi-threaded inferiors.

Currently (and this remains the default after this commit) when
switching between inferiors GDB would select the first non-exited
thread from the inferior being switched to.

This commit adds the following new commands:

set restore-selected-thread on|off
show restore-selected-thread

This option is off by default in order to retain the existing
behaviour, but, when switched on GDB will remember which thread was
selected in each inferior. As the user switches between inferiors GDB
will attempt to restore the previously selected thread.

If the previously selected thread is no longer available, for example,
if the thread has exited, then GDB will fall back on the old
behaviour.

I did consider, but eventually didn't implemented, adding a warning
when switching inferiors if the previously selected thread is no
longer available. My reasoning here is that GDB should already have
informed the user that the thread has exited, and there is already a
message indicating which thread has been switched too, so adding an
extra warning felt like unneeded clutter.

In order to store the thread within the inferior I store a pointer to
the thread_info object of the previously selected thread. When
fetching the thread_info it is important that we do actually have a
current thread otherwise this happens:

$ gdb
(gdb) add-inferior
(gdb) inferior 2
./gdb/thread.c:95: internal-error: thread_info* inferior_thread(): Assertion `current_thread_ != nullptr' failed.

To avoid this I added a check that inferior_ptid is not null_ptid.
Though it is not always the case, there are plenty of places in GDB
where a call to inferior_thread () is guarded by such a check.

There's a new test for this functionality.

gdb/ChangeLog:

* inferior.c (inferior_command): Store current thread_info before
switching inferiors. Reselect the previous thread_info if
possible after switching to the new inferior.
(initialize_inferiors): Register restore-selected-thread option.
* inferior.h (class inferior) <previous_thread_info>: New member
variable.
* NEWS: Mention new feature.

gdb/testsuite/ChangeLog:

* gdb.threads/restore-thread.c: New file.
* gdb.threads/restore-thread.exp: New file.

gdb/doc/ChangeLog:

* gdb.texinfo (Inferiors Connections and Programs): Mention thread
tracking within the inferior command.
(Threads): Mention thread tracking in the general thread
discussion.
---
gdb/ChangeLog | 10 +
gdb/NEWS | 9 +
gdb/doc/ChangeLog | 7 +
gdb/doc/gdb.texinfo | 19 +-
gdb/inferior.c | 58 ++++-
gdb/inferior.h | 10 +
gdb/testsuite/ChangeLog | 5 +
gdb/testsuite/gdb.threads/restore-thread.c | 248 +++++++++++++++++++
gdb/testsuite/gdb.threads/restore-thread.exp | 219 ++++++++++++++++
9 files changed, 583 insertions(+), 2 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/restore-thread.c
create mode 100644 gdb/testsuite/gdb.threads/restore-thread.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 3e08aee7c6f..5a900b8a678 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,15 @@ set debug event-loop
show debug event-loop
Control the display of debug output about GDB's event loop.

+set restore-selected-thread on|off
+show restore-selected-thread
+ This new option is off by default. When turned on GDB will record
+ the currently selected thread in each inferior. When switching
+ between inferiors GDB will attempt to restore the previously
+ selected thread in the inferior being switched too. If the
+ previously selected thread is no longer available then GDB falls
+ back to selecting the first non-exited thread.
+
* Changed commands

break [PROBE_MODIFIER] [LOCATION] [thread THREADNUM]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 52701560006..c5819bde7ae 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3247,11 +3247,25 @@
To switch focus between inferiors, use the @code{inferior} command:

@table @code
+@anchor{inferior command}
@kindex inferior @var{infno}
@item inferior @var{infno}
Make inferior number @var{infno} the current inferior. The argument
@var{infno} is the inferior number assigned by @value{GDBN}, as shown
in the first field of the @samp{info inferiors} display.
+
+When switching between inferiors with multiple threads
+(@pxref{Threads}) @value{GDBN} will select the first non-exited thread
+in the inferior being switched to and make this the current thread.
+
+@kindex set restore-selected-thread
+@kindex show restore-selected-thread
+@item set restore-selected-thread @r{[}on|off@r{]}
+@item show restore-selected-thread
+When this option is on @value{GDBN} will record the currently selected
+thread in each inferior. When switching between inferior @value{GDBN}
+will try to restore the previously selected thread in the inferior
+being switched to. This option is off by default.
@end table

@vindex $_inferior@r{, convenience variable}
@@ -3633,7 +3647,10 @@

If you're debugging multiple inferiors, @value{GDBN} displays thread
IDs using the qualified @var{inferior-num}.@var{thread-num} format.
-Otherwise, only @var{thread-num} is shown.
+Otherwise, only @var{thread-num} is shown. When switching between
+inferiors @value{GDBN} will select a suitable thread in the inferior
+being switched to, see @ref{inferior command,,the @code{inferior}
+command} for further details on how to control this behaviour.

If you specify the @samp{-gid} option, @value{GDBN} displays a column
indicating each thread's global thread ID:
diff --git a/gdb/inferior.c b/gdb/inferior.c
index d4a783b3e6d..4961bc467fd 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -631,6 +631,23 @@ switch_to_inferior_no_thread (inferior *inf)
set_current_program_space (inf->pspace);
}

+/* When this is true GDB restores the inferiors previously selected thread
+ each time the inferior is changed (where possible). */
+
+static bool restore_selected_thread_per_inferior = false;
+
+/* Implement 'show restore-selected-thread'. */
+
+static void
+show_restore_selected_thread_per_inferior (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ fprintf_filtered (file,
+ _("Restoring the selected thread is currently %s.\n"),
+ value);
+}
+
static void
inferior_command (const char *args, int from_tty)
{
@@ -643,11 +660,38 @@ inferior_command (const char *args, int from_tty)
if (inf == NULL)
error (_("Inferior ID %d not known."), num);

+ /* We can only call INFERIOR_THREAD if the inferior is known to have an
+ active thread, which it wont if the inferior is currently exited. So,
+ first check if we currently have a thread selected. */
+ if (inferior_ptid != null_ptid)
+ {
+ /* Now take a strong reference to the current thread_info and store
+ it within the inferior, this prevents the thread_info from being
+ deleted until the inferior has released the reference. */
+ thread_info *tp = inferior_thread ();
+ tp->incref ();
+ current_inferior ()->previous_thread_info.reset (tp);
+ }
+
if (inf->pid != 0)
{
if (inf != current_inferior ())
{
- thread_info *tp = any_thread_of_inferior (inf);
+ thread_info *tp = nullptr;
+
+ if (restore_selected_thread_per_inferior
+ && inf->previous_thread_info != nullptr)
+ {
+ /* Release the reference to the previous thread. We don't
+ switch back to this thread if it is already exited
+ though. */
+ tp = inf->previous_thread_info.release ();
+ tp->decref ();
+ if (tp->state == THREAD_EXITED)
+ tp = nullptr;
+ }
+ if (tp == nullptr)
+ tp = any_thread_of_inferior (inf);
if (tp == NULL)
error (_("Inferior has no threads."));

@@ -1025,5 +1069,17 @@ Show printing of inferior events (such as inferior start and exit)."), NULL,
show_print_inferior_events,
&setprintlist, &showprintlist);

+ add_setshow_boolean_cmd ("restore-selected-thread",
+ no_class, &restore_selected_thread_per_inferior,
+ _("\
+Set whether GDB restores the selected thread when switching inferiors."), _("\
+Show whether GDB restores the selected thread when switching inferiors."), _("\
+When this option is on GDB will record the currently selected thread for\n\
+each inferior, and restore the selected thread whenever GDB switches inferiors."),
+ nullptr,
+ show_restore_selected_thread_per_inferior,
+ &setlist,
+ &showlist);
+
create_internalvar_type_lazy ("_inferior", &inferior_funcs, NULL);
}
diff --git a/gdb/inferior.h b/gdb/inferior.h
index d016161fb05..517180e48e2 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -543,6 +543,16 @@ class inferior : public refcounted_object
/* Data related to displaced stepping. */
displaced_step_inferior_state displaced_step_state;

+ /* This field is updated when GDB switches away from this inferior to
+ some other inferior, a reference to a thread_info is stored in here,
+ the ref count for the thread_info should be non-zero to prevent the
+ thread_info being deleted.
+
+ When the user switches back to this inferior the thread_info is taken
+ out of this reference and used to (possibly) switch back to this
+ thread. */
+ thread_info_ref previous_thread_info;
+
/* Per inferior data-pointers required by other GDB modules. */
REGISTRY_FIELDS;

diff --git a/gdb/testsuite/gdb.threads/restore-thread.c b/gdb/testsuite/gdb.threads/restore-thread.c
new file mode 100644
index 00000000000..3eb1f722199
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/restore-thread.c
@@ -0,0 +1,248 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ 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 <http://www.gnu.org/licenses/>. */
+
+#include <pthread.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <errno.h>
+
+/* The number of threads to create. */
+volatile int thread_count = 3;
+
+/* This is initialised with our pid. GDB will read and print this value
+ from the Dejagnu test script, the test script will then use the pid to
+ send signals to this process. */
+pid_t global_pid;
+
+/* Holds one end of two different pipes. Things written to READ will not
+ appear on WRITE. */
+struct pipe_fds
+{
+ int read;
+ int write;
+};
+
+/* Information passed into each thread. */
+struct thread_info
+{
+ /* Just a numeric id for the thread. */
+ int id;
+
+ /* File handles with which the worker thread can communicate with the
+ master thread. */
+ struct pipe_fds fds;
+};
+
+/* The control information held by the master thread, one of these for each
+ worker thread. */
+struct thread_ctrl
+{
+ /* The actual pthread handle, used to join the threads. */
+ pthread_t thread;
+
+ /* File handles with which the master thread can communicate with the
+ worker threads. */
+ struct pipe_fds fds;
+
+ /* The information that is passed into the worker thread. */
+ struct thread_info info;
+};
+
+/* Wait for a single byte of the read file handle in FDS. */
+static void
+wait_on_byte (struct pipe_fds *fds)
+{
+ ssize_t rtn;
+ char c;
+
+ while ((rtn = read (fds->read, &c, 1)) != 1)
+ {
+ if (rtn != -1 || errno != EINTR)
+ abort ();
+ }
+}
+
+/* Send a single byte to the write file handle in FDS. */
+static void
+send_byte (struct pipe_fds *fds)
+{
+ ssize_t rtn;
+ char c = 'x';
+ while ((rtn = write (fds->write, &c, 1)) != 1)
+ {
+ if (rtn != -1 || errno != EINTR)
+ abort ();
+ }
+}
+
+/* Create a function used to mark a breakpoint location. */
+#define BREAKPOINT_FUNC(N) \
+ static void \
+ breakpt_ ## N () \
+ { \
+ printf ("Hit breakpt_" #N "\n"); \
+ }
+
+BREAKPOINT_FUNC (0) /* breakpt_0 */
+BREAKPOINT_FUNC (1) /* breakpt_1 */
+BREAKPOINT_FUNC (2) /* breakpt_2 */
+
+/* The worker thread entry point. */
+static void *
+thread_worker (void *arg)
+{
+ struct thread_info *info = (struct thread_info *) arg;
+ int id = info->id;
+
+ printf ("Thread %d created.\n", id);
+ breakpt_0 ();
+
+ /* Let the main thread know that this thread is now running. */
+ send_byte (&info->fds);
+
+ /* The thread with id #2 is special, it waits here for a nudge from the
+ main thread. */
+ if (id == 2)
+ {
+ wait_on_byte (&info->fds);
+ breakpt_2 ();
+ send_byte (&info->fds);
+ }
+
+ /* Now wait for an incoming message indicating that the thread should
+ exit. */
+ wait_on_byte (&info->fds);
+ printf ("In thread %d, exiting...\n", id);
+ return NULL;
+}
+
+/* Initialise CTRL for thread ID, this includes setting up all of the pipe
+ file handles. */
+static void
+thread_ctrl_init (struct thread_ctrl *ctrl, int id)
+{
+ int fds[2];
+
+ ctrl->info.id = id;
+ if (pipe (fds))
+ abort ();
+ ctrl->info.fds.read = fds[0];
+ ctrl->fds.write = fds[1];
+
+ if (pipe (fds))
+ abort ();
+ ctrl->fds.read = fds[0];
+ ctrl->info.fds.write = fds[1];
+}
+
+/* Wait for a SIGUSR1 to arrive. Assumes that SIGUSR1 is blocked on entry
+ to this function. */
+static void
+wait_for_sigusr1 (void)
+{
+ int signo;
+ sigset_t set;
+
+ sigemptyset (&set);
+ sigaddset (&set, SIGUSR1);
+
+ /* Wait for a SIGUSR1. */
+ if (sigwait (&set, &signo) != 0)
+ abort ();
+ if (signo != SIGUSR1)
+ abort ();
+}
+
+/* Main program. */
+int
+main ()
+{
+ sigset_t set;
+ int i, max = thread_count;
+
+ /* Set an alarm in case the testsuite crashes, don't leave the test
+ running forever. */
+ alarm (300);
+
+ struct thread_ctrl *info = malloc (sizeof (struct thread_ctrl) * max);
+ if (info == NULL)
+ abort ();
+
+ /* Put the pid somewhere easy for GDB to read, also print it. */
+ global_pid = getpid ();
+ printf ("pid = %lld\n", ((long long) global_pid));
+
+ /* Block SIGUSR1, all threads will inherit this sigmask. */
+ sigemptyset (&set);
+ sigaddset (&set, SIGUSR1);
+ if (pthread_sigmask (SIG_BLOCK, &set, NULL))
+ abort ();
+
+ /* Create each thread. */
+ for (i = 0; i < max; ++i)
+ {
+ struct thread_ctrl *thr = &info[i];
+ thread_ctrl_init (thr, i + 1);
+
+ if (pthread_create (&thr->thread, NULL, thread_worker, &thr->info) != 0)
+ abort ();
+
+ /* Wait for an indication that the thread has started, and is ready
+ for action. */
+ wait_on_byte (&thr->fds);
+ }
+
+ printf ("All threads created.\n");
+
+ /* Give thread thread #1 a little nudge. */
+ if (max >= 2)
+ {
+ send_byte (&info[1].fds);
+ wait_on_byte (&info[1].fds);
+ }
+
+ breakpt_1 ();
+
+ /* For each thread in turn wait for a SIGUSR1 to arrive, signal the
+ thread so that it will exit (by sending it a byte down its pipe), then
+ join the newly exited thread. */
+ for (i = 0; i < max; ++i)
+ {
+ struct thread_ctrl *thr = &info[i];
+
+ wait_for_sigusr1 ();
+
+ printf ("Telling thread %d to exit\n", thr->info.id);
+ send_byte (&thr->fds);
+
+ if (pthread_join (thr->thread, NULL) != 0)
+ abort ();
+
+ printf ("Thread %d exited\n", thr->info.id);
+ }
+
+ free (info);
+
+ /* Final wait before exiting. */
+ wait_for_sigusr1 ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/restore-thread.exp b/gdb/testsuite/gdb.threads/restore-thread.exp
new file mode 100644
index 00000000000..f768b123c74
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/restore-thread.exp
@@ -0,0 +1,219 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# 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 <http://www.gnu.org/licenses/>.
+
+# Test GDB's ability to restore the selected thread when switching
+# between inferiors, and check what happens when the selected thread
+# of one inferior exits while we have a different inferior selected.
+
+standard_testfile
+
+if [prepare_for_testing "failed to prepare" $binfile $srcfile \
+ {debug pthreads}] {
+ return -1
+}
+
+# Check that the current thread is THR in inferior INF.
+proc check_current_thread { inf thr {testname ""} } {
+ if {${testname} == ""} {
+ set testname "check_current_thread ${inf} ${thr}"
+ }
+
+ # As a final check, lets check the output for the 'thread'
+ # command.
+ gdb_test "thread" "Current thread is ${inf}.${thr} .*" \
+ "current thread is ${inf}.${thr}: $testname"
+}
+
+# Switch to inferior number INF, we expect that thread number THR
+# within the inferior will be selected.
+proc switch_to_inferior { inf thr {testname ""} } {
+ if {${testname} == ""} {
+ set testname "switch_to_inferior $inf $thr"
+ }
+
+ gdb_test "inferior $inf" \
+ "Switching to inferior ${inf} .*Switching to thread ${inf}.${thr} .*" \
+ "$testname: select inferior ${inf}"
+
+ check_current_thread $inf $thr "$testname: check current thread"
+}
+
+# Switch to thread number THR. INF should be the number of the
+# currently selected inferior and is used when checking the currently
+# selected thread.
+proc switch_to_thread { inf thr {testname ""} } {
+ if {${testname} == ""} {
+ set testname "switch_to_thread $inf $thr"
+ }
+
+ gdb_test "thread ${thr}" \
+ "Switching to thread ${inf}.${thr} .*" \
+ "${testname}: select thread ${thr}"
+ check_current_thread $inf $thr \
+ "${testname}: check current thread"
+}
+
+# Continue the program in the background.
+proc continue_in_bg { testname } {
+ global gdb_prompt
+
+ gdb_test_multiple "continue&" $testname {
+ -re "Continuing\\.\r\n$gdb_prompt " {
+ pass $gdb_test_name
+ }
+ }
+}
+
+# Send SIGUSR1 to PID, this will cause one of that processes threads
+# to exit (assuming the process is currently running).
+proc send_thread_exit_signal { pid } {
+ global decimal
+
+ remote_exec target "kill -USR1 ${pid}"
+ gdb_test_multiple "" "wait for thread to exit" {
+ -re "Thread $decimal exited.*exited\\\].*" {
+ }
+ }
+}
+
+# Start of test script.
+
+set pid_1 0
+set pid_2 0
+
+if ![runto_main] {
+ return -1
+}
+
+# Restoring the selected thread is off by default. Switch it on now.
+gdb_test_no_output "set restore-selected-thread on"
+
+gdb_breakpoint "breakpt_0"
+gdb_breakpoint "breakpt_1"
+
+with_test_prefix "start inferior 1" {
+ gdb_continue_to_breakpoint "created thread 1.2" ".* breakpt_0 .*"
+ gdb_continue_to_breakpoint "created thread 1.3" ".* breakpt_0 .*"
+ gdb_continue_to_breakpoint "created thread 1.4" ".* breakpt_0 .*"
+ gdb_continue_to_breakpoint "all inferior 1 threads created" \
+ ".* breakpt_1 .*"
+ gdb_test "info threads" ".*"
+ set pid_1 [get_valueof "/d" "global_pid" 0]
+}
+
+# Start another inferior.
+gdb_test "add-inferior" [multi_line \
+ "\\\[New inferior 2\\\]" \
+ "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" {
+ gdb_breakpoint "breakpt_2"
+ gdb_run_cmd
+ gdb_test "" "hit Breakpoint .*" \
+ "runto breakpoint in main"
+ gdb_continue_to_breakpoint "created thread 2.2" ".* breakpt_0 .*"
+ gdb_continue_to_breakpoint "created thread 2.3" ".* breakpt_0 .*"
+ gdb_continue_to_breakpoint "created thread 2.4" ".* breakpt_0 .*"
+ gdb_continue_to_breakpoint "all inferior 2 threads created" \
+ ".* breakpt_2 .*"
+ gdb_test "info threads" ".*"
+ set pid_2 [get_valueof "/d" "global_pid" 0]
+}
+
+gdb_assert {${pid_1} != 0} "read the pid for inferior 1"
+gdb_assert {${pid_2} != 0} "read the pid for inferior 2"
+
+check_current_thread 2 3 "check initial thread is 2.3"
+switch_to_inferior 1 1 "first switch to thread 1.1"
+switch_to_inferior 2 3
+switch_to_thread 2 2
+
+switch_to_inferior 1 1 "second switch to thread 1.1"
+switch_to_thread 1 3
+switch_to_inferior 2 2
+
+# Inferior 2 is special; it will have stopped at breakpt_2, in thread
+# 2.3. To set this inferior up so that threads can exit we need to
+# continue to breakpt_1.
+gdb_continue_to_breakpoint "all inferior 2 threads created" \
+ ".* breakpt_1 .*"
+
+with_test_prefix "inferior 2 ready" {
+ check_current_thread 2 1
+
+ switch_to_inferior 1 3
+ switch_to_thread 1 2
+
+ continue_in_bg "continue inferior 1"
+ switch_to_inferior 2 1
+ switch_to_thread 2 2
+ continue_in_bg "continue inferior 2"
+}
+
+# Cause thread 1.2 to exit.
+send_thread_exit_signal ${pid_1}
+
+with_test_prefix "after 1.2 exited" {
+ # We should go back to 1.1 now as 1.2 has exited.
+ switch_to_inferior 1 1
+ switch_to_thread 1 4
+
+ # Cause thread 2.2 to exit.
+ send_thread_exit_signal ${pid_2}
+}
+
+with_test_prefix "after 2.2 exited" {
+ # We should go back to 2.1 now as 2.2 has exited.
+ switch_to_inferior 2 1
+
+ # Cause thread 1.3 to exit.
+ send_thread_exit_signal ${pid_1}
+}
+
+with_test_prefix "after 1.3 exited" {
+ # We should still switch back to 1.4 as only 1.3 exited.
+ switch_to_inferior 1 4
+
+ # Cause thread 2.3 to exit.
+ send_thread_exit_signal ${pid_2}
+}
+
+with_test_prefix "after 2.3 exited" {
+ # Switch back to 2.1, which should still be selected.
+ switch_to_inferior 2 1
+
+ # Cause thread 1.4 to exit.
+ send_thread_exit_signal ${pid_1}
+}
+
+with_test_prefix "after 1.4 exited" {
+ # We should now switch back to 1.1 as 1.4 exited, and 1.1 is the
+ # only thread left now.
+ switch_to_inferior 1 1
+
+ # Cause thread 2.4 to exit.
+ send_thread_exit_signal ${pid_2}
+}
+
+with_test_prefix "after 2.4 exited" {
+ # Switch back to 2.1, which should still be selected.
+ switch_to_inferior 2 1
+}
--
2.25.4