Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: Klaus Gerlicher <klaus.gerlicher@intel.com>
Cc: gdb-patches@sourceware.org, aburgess@redhat.com
Subject: Re: [PATCH v3 1/1] gdb: remember previously selected thread per inferior
Date: Tue, 8 Sep 2026 13:01:02 -0700	[thread overview]
Message-ID: <20260908130102.7ccb3698@f44-mesa-1> (raw)
In-Reply-To: <20260908133426.444197-2-klaus.gerlicher@intel.com>

Hi Klaus,

I had my AI reviewer look this over.  It found some problems.  In
addition to needing a rebase w/ resultant build problems fixed, the
test case needs some rework.

I'll point out what it found below...

On Tue,  8 Sep 2026 13:34:26 +0000
Klaus Gerlicher <klaus.gerlicher@intel.com> wrote:

[...]
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 09080ccdad9..d4a59ff6445 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -3518,6 +3518,26 @@ To switch focus between inferiors, use the @code{inferior} command:
>  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.
> +
> +By default, when switching to an inferior, @value{GDBN} selects the first
> +available thread.  Optionally, @value{GDBN} can remember and restore the
> +thread that was previously selected in that inferior (if it still exists).
> +This behavior can be controlled with the @code{set remember-threads-per-inferior}
> +command, described below.
> +@end table
> +
> +@cindex remember threads per inferior
> +@kindex set remember-threads-per-inferior
> +@kindex show remember-threads-per-inferior
> +@table @code
> +@item set remember-threads-per-inferior on|off
> +Control whether @value{GDBN} remembers the selected thread for each inferior.
> +When enabled, switching to a previously used inferior will restore the thread
> +that was active when you switched away.  When disabled (the default),

I believe that Eli asked for the word "selected" to be used instead
of "active" here.

[...]

> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 8c619ffec5a..9d8ea91f29e 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -46,6 +46,9 @@ static int highest_inferior_num;
>  /* See inferior.h.  */
>  bool print_inferior_events = true;
>  
> +/* Control whether to remember selected threads per inferior.  */
> +bool remember_inferior_threads = false;
> +
>  /* The Current Inferior.  This is a strong reference.  I.e., whenever
>     an inferior is the current inferior, its refcount is
>     incremented.  */
> @@ -508,6 +511,32 @@ inferior_pid_to_str (int pid)
>  
>  /* See inferior.h.  */
>  
> +void
> +save_inferior_last_thread ()
> +{
> +  if (has_current_thread ())
> +    current_inferior ()->last_user_thread
> +      = thread_info_ref::new_reference (inferior_thread ());
> +  else
> +    current_inferior ()->last_user_thread = nullptr;
> +}
> +
> +/* Observer callback for explicit user-initiated thread switches.
> +   This captures switches from commands like "thread X" or "inferior Y".
> +   Note: This does NOT fire for automatic event-driven switches (e.g.,
> +   breakpoint hits in schedule-multiple mode).  Those are handled by a
> +   direct call in fetch_inferior_event.  Both mechanisms are necessary
> +   to capture all thread switch scenarios.  */
> +
> +static void
> +inferiors_on_user_selected_context_changed (user_selected_what selection)
> +{
> +  if (remember_inferior_threads && (selection & USER_SELECTED_THREAD))
> +    save_inferior_last_thread ();
> +}
> +
> +/* See inferior.h.  */
> +
>  void
>  print_selected_inferior (struct ui_out *uiout)
>  {
> @@ -771,8 +800,23 @@ inferior_command (const char *args, int from_tty)
>  	{
>  	  if (inf != current_inferior ())
>  	    {
> -	      thread_info *tp = any_non_exited_thread_of_inferior (inf);
> -	      if (tp == NULL)
> +	      thread_info *tp = nullptr;
> +
> +	      if (remember_inferior_threads)
> +		{
> +		  thread_info *stored_tp = inf->last_user_thread.get ();
> +
> +		  /* Fallback to selecting any non-exited thread of
> +		     inferior.  */
> +		  tp = (stored_tp == nullptr
> +			|| stored_tp->state == THREAD_EXITED)

thread_info::state is now a member function.  That line should
now be:

   			|| stored_tp->state () == THREAD_EXITED)

> +			? any_non_exited_thread_of_inferior (inf)
> +			: stored_tp;
> +		}
> +	      else
> +		tp = any_non_exited_thread_of_inferior (inf);
> +
> +	      if (tp == nullptr)
>  		error (_("Inferior has no threads."));
>  
>  	      switch_to_thread (tp);
[...]

> diff --git a/gdb/testsuite/gdb.base/inferior-switch.c b/gdb/testsuite/gdb.base/inferior-switch.c
> new file mode 100644
> index 00000000000..3b94713ff43
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/inferior-switch.c
> @@ -0,0 +1,42 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +   Copyright 2024 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 <unistd.h>
> +#include <pthread.h>
> +
> +static void *
> +task (void *arg)
> +{
> +  volatile unsigned int duration = 0;
> +
> +  int a = 1; /* worker thread break 1.  */
> +
> +  sleep (duration);
> +  int b = 2; /* worker thread break 2.  */
> +
> +  sleep (duration);
> +  return NULL;
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t th;
> +
> +  alarm (30);
> +
> +  pthread_create (&th, NULL, task, NULL);
> +  pthread_join (&th, NULL);

I think this should be:

     pthread_join (th, NULL);

[...]

> diff --git a/gdb/testsuite/gdb.base/inferior-switch.exp b/gdb/testsuite/gdb.base/inferior-switch.exp
> new file mode 100644
> index 00000000000..5cf7ee98fb1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/inferior-switch.exp
[...]

FYI, for this test, I'm seeing the failures below.  I think this is
likely due to the change in default behavior.

FAIL: gdb.base/inferior-switch.exp: test_disabled_behavior: feature on again, restores previously selected thread 1.2
FAIL: gdb.base/inferior-switch.exp: test_explicit: again back to inferior 2 thread 2.2
FAIL: gdb.base/inferior-switch.exp: test_explicit: back to inferior 1 thread 1.2
FAIL: gdb.base/inferior-switch.exp: test_explicit: switch to inferior 2 thread 2.2
FAIL: gdb.base/inferior-switch.exp: test_implicit: switch inferior 1 stopped at 2nd BP
FAIL: gdb.base/inferior-switch.exp: test_implicit: switch inferior 2 stopped at 2nd BP

Kevin


      reply	other threads:[~2026-09-08 20:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 13:34 [PATCH v3 0/1] " Klaus Gerlicher
2026-09-08 13:34 ` [PATCH v3 1/1] " Klaus Gerlicher
2026-09-08 20:01   ` Kevin Buettner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908130102.7ccb3698@f44-mesa-1 \
    --to=kevinb@redhat.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=klaus.gerlicher@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox