[PATCHv4 1/2] gdb: Restore previously selected thread when switching inferior
Aktemur, Tankut Baris
tankut.baris.aktemur@intel.com
Mon Nov 9 15:02:56 GMT 2020
On Saturday, November 7, 2020 12:02 AM, Andrew Burgess wrote:
> 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
"inferiors" -> "inferior's".
I also think that a comma is need after "true".
> + 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\
A comma after "on" could improve the sentence, IMHO.
> +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,
It feels like there should be a period after "inferior", instead of a comma, to
end the sentence.
Also, the comment "a reference to a thread_info is stored in here..." is possibly
redundant, because the type "thread_info_ref" implies this.
> + 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;
>
Thanks.
-Baris
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
More information about the Gdb-patches
mailing list