Re: [PATCHv2] gdb: notify of inferior switch when needed from 'thread' command

[Guinevere Larsen <guinevere@redhat.com>]

On 1/26/26 10:58 AM, Andrew Burgess wrote:
> In v2:
>
>    - Rebased and retested.
>
>    - Reworked commit message to try and address Pedro's concerns.
>
> Thanks,
> Andrew
>
> ---

Hi Andrew!

I've taken a look at this patch and it looks pretty good, and I think it 
is a good change.

I know you're waiting for Pedro's feedback, but hopefully this message 
acts as a ping for him as well as a review for you

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>

>
> While working on the commit:
>
>    commit 9959019545d8d6d71d927f20f088efba944b1e9c
>    Date:   Sun Sep 28 16:16:53 2025 +0100
>
>        gdb: fix for 'set suppress-cli-notifications on' missed case
>
> I spotted this message in the gdb.mi/user-selected-context-sync.exp
> test script:
>
>    # Idea for the future: selecting a thread in a different inferior.  For now,
>    # GDB doesn't show an inferior switch, but if it did, it would be a nice
>    # place to test it.
>
> What this message is talking about is this behaviour:
>
>    (gdb) info threads
>      Id   Target Id                              Frame
>      1.1  Thread 0xf7dbc700 (LWP 818430) "thr" 0xf7eb2888 in clone () from /lib/libc.so.6
>      1.2  Thread 0xf7dbbb40 (LWP 818433) "thr" 0xf7fd0579 in __kernel_vsyscall ()
>      1.3  Thread 0xf73ffb40 (LWP 818434) "thr" breakpt () at thr.c:19
>      2.1  Thread 0xf7dbc700 (LWP 818456) "thr" 0xf7eb2888 in clone () from /lib/libc.so.6
>      2.2  Thread 0xf7dbbb40 (LWP 818457) "thr" breakpt () at thr.c:19
>    * 2.3  Thread 0xf73ffb40 (LWP 818458) "thr" breakpt () at thr.c:19
>    (gdb) inferior 1
>    [Switching to inferior 1 [process 818430] (/home/andrew/tmp/thr)]
>    [Switching to thread 1.1 (Thread 0xf7dbc700 (LWP 818430))]
>    #0  0xf7eb2888 in clone () from /lib/libc.so.6
>    (gdb) thread 2.2
>    [Switching to thread 2.2 (Thread 0xf7dbbb40 (LWP 818457))]
>    #0  breakpt () at thr.c:19
>    19	  while (stop)
>    (gdb)
>
> Notice that when we switch from thread 2.3 to 1.1 using the 'inferior
> 1' command, GDB tells us that the inferior has changed, and that the
> thread has changed (and also that the frame has changed).
>
> But, when we switch from 1.1 to 2.2 using the 'thread 2.2' command, we
> are only told about the thread change.
>
> The 'Switching to inferior ...' line includes some useful information,
> the process PID and the executable name, and I think it is a shame
> that these are not presented when using the 'thread' command to switch
> inferior.
>
> So, this commit addresses this issue.
>
> A question that came up during review, and which I'm clarifying here:
> this change only effects the output of GDB when the thread command is
> also used to switch inferiors.  I am (in effect) arguing that the
> command 'thread 2.2' should be treated as a shorthand for 'inferior 2;
> thread 2', and should display all of the associated output.  If the
> user is only switching threads within a single inferior then it is not
> necessary to re-display the inferior information.
>
> I acknowledge that this does mean the output of the 'thread' command
> will now be different depending on whether the user changes inferior
> or not.  However, I think this is better than the alternative, having
> the 'thread' command always re-print the inferior information.  I
> think this would introduce excess noise that is not useful.
>
> There are changes in basically two areas.  The easy part is in
> thread_command (thread.c).  Here we spot when the inferior has changed
> as a result of the 'thread' command, and included
> USER_SELECTED_INFERIOR in the set of state passed to the
> notify_user_selected_context_changed function.
>
> The change in mi/mi-main.c is a little more involved.  In the
> mi_cmd_execute function we use an instance of user_selected_context to
> spot if any inferior state (frame, thread, or inferior) changes after
> an MI command, this is then used to decide if there should be a call
> to notify_user_selected_context_changed.
>
> Currently, in mi_cmd_execute, notify_user_selected_context_changed is
> always passed 'USER_SELECTED_THREAD | USER_SELECTED_FRAME'.  This
> makes sense, the MI doesn't allow "switching inferiors" as a command,
> instead, an MI frontend must switch threads, and the inferior is
> switched as a consequence.  But this does mean that if a user has a
> CLI and MI interpreter running, and the MI switches threads, the CLI
> will only receive the thread switch style notifications, that is,
> there will be no "Switching to inferior ..." line.
>
> What I've done is rename user_selected_context::has_changed to
> user_selected_context::what_changed, this function is now responsible
> for returning the set of USER_SELECTED_* flags that indicate what
> changed.
>
> If anything has changed then we always return USER_SELECTED_THREAD |
> USER_SELECTED_FRAME as a minimum.  This retains the existing
> behaviour, but is possibly more aggressive that we need to be; the
> -stack-select-frame command can only change the frame, so maybe in
> this case we should only return USER_SELECTED_FRAME?  I've left that
> for the future though.
>
> However, the important change is that in ::what_changed, I now spot
> when the inferior has changed and include USER_SELECTED_INFERIOR in
> the set of flags that are returned.
>
> In mi_cmd_execute we now call the new what_changed function, and use
> the set of flags returned when calling
> notify_user_selected_context_changed.  This means that the CLI will
> now receive inferior changed notifications when appropriate.
>
> The gdb.mi/user-selected-context-sync.exp script has been updated,
> replacing the comment I quoted above with an actual test that the
> inferior change is announced correctly.
> ---
>   gdb/mi/mi-main.c                              | 44 ++++++++++++---
>   .../gdb.mi/user-selected-context-sync.exp     | 56 ++++++++++++++++---
>   gdb/thread.c                                  | 13 ++++-
>   3 files changed, 93 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 77e56bc9fc8..a18209366ff 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -538,6 +538,11 @@ mi_cmd_thread_select (const char *command, const char *const *argv, int argc)
>   
>     thread_select (argv[0], thr);
>   
> +  /* We don't call print_selected_inferior here as this never prints
> +     anything when the output is MI like (as it is now).  MI consumers are
> +     expected to derive the inferior change from the global thread-id
> +     included in the print_selected_thread_frame output.  */
> +
>     print_selected_thread_frame (current_uiout,
>   			       USER_SELECTED_THREAD | USER_SELECTED_FRAME);
>   }
> @@ -1968,12 +1973,30 @@ struct user_selected_context
>   
>     /* Return true if the user selected context has changed since this object
>        was created.  */
> -  bool has_changed () const
> +  user_selected_what what_changed () const
>     {
> +    /* If anything changed then we report both the thread and frame at a
> +       minimum.  We optionally add the inferior if we know that it
> +       changed.
> +
> +       This means that for pure frame changes, e.g. -stack-select-frame, we
> +       still report both a thread and a frame, which isn't ideal, but
> +       there's also the cases where -thread-select is used to re-select the
> +       current thread, in this case we'd still like to see the thread
> +       reported, at least, that's what we have historically done.  */
> +    user_selected_what state
> +      = USER_SELECTED_THREAD | USER_SELECTED_FRAME;
> +
>       /* Did the selected thread change?  */
>       if (m_previous_ptid != null_ptid && inferior_ptid != null_ptid
>   	&& m_previous_ptid != inferior_ptid)
> -      return true;
> +      {
> +	/* Did the inferior change too?  */
> +	if (m_previous_ptid.pid () != inferior_ptid.pid ())
> +	  state |= USER_SELECTED_INFERIOR;
> +
> +	return state;
> +      }
>   
>       /* Grab details of the currently selected frame, for comparison.  */
>       frame_id current_frame_id;
> @@ -1982,7 +2005,7 @@ struct user_selected_context
>   
>       /* Did the selected frame level change?  */
>       if (current_frame_level != m_previous_frame_level)
> -      return true;
> +      return state;
>   
>       /* Did the selected frame id change?  If the innermost frame is
>          selected then the level will be -1, and the frame-id will be
> @@ -1991,10 +2014,10 @@ struct user_selected_context
>          other than the innermost frame selected.  */
>       if (current_frame_level != -1
>   	&& current_frame_id != m_previous_frame_id)
> -      return true;
> +      return state;
>   
>       /* Nothing changed!  */
> -    return false;
> +    return 0;
>     }
>   private:
>     /* The previously selected thread.  This might be null_ptid if there was
> @@ -2098,10 +2121,13 @@ mi_cmd_execute (struct mi_parse *parse)
>   
>     parse->cmd->invoke (parse);
>   
> -  if (!parse->cmd->preserve_user_selected_context ()
> -      && current_user_selected_context.has_changed ())
> -    interps_notify_user_selected_context_changed
> -      (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> +  if (!parse->cmd->preserve_user_selected_context ())
> +    {
> +      user_selected_what what
> +	= current_user_selected_context.what_changed ();
> +      if (what != 0)
> +	notify_user_selected_context_changed (what);
> +    }
>   }
>   
>   /* See mi-main.h.  */
> diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> index 3434ffa2a46..4f02b950f6e 100644
> --- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> +++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> @@ -698,9 +698,21 @@ proc_with_prefix test_cli_thread { mode } {
>   	}
>       }
>   
> -    # Idea for the future: selecting a thread in a different inferior.  For now,
> -    # GDB doesn't show an inferior switch, but if it did, it would be a nice
> -    # place to test it.
> +    with_test_prefix "thread 2.2" {
> +	# Select a thread in a different inferior.  This should trigger an
> +	# 'inferior changed' and 'thread changed' notification on the CLI,
> +	# and a single MI async notification.
> +	set mi_re [make_mi_re $mode 5 0 event]
> +	set cli_re [make_cli_re $mode 2 2.2 0]
> +
> +	with_spawn_id $gdb_main_spawn_id {
> +	    gdb_test "thread 2.2" $cli_re "select thread"
> +	}
> +
> +	with_spawn_id $mi_spawn_id {
> +	    match_re_or_ensure_no_output $mi_re "select thread, event on MI"
> +	}
> +    }
>   }
>   
>   # Test frame selection from CLI.
> @@ -995,9 +1007,22 @@ proc_with_prefix test_mi_thread_select { mode } {
>   	}
>       }
>   
> -    # Idea for the future: selecting a thread in a different inferior.  For now,
> -    # GDB doesn't show an inferior switch, but if it did, it would be a nice
> -    # place to test it.
> +    with_test_prefix "thread 2.2" {
> +	# Select a thread in a different inferior.  This should trigger an
> +	# 'inferior changed' and 'thread changed' notification on the CLI,
> +	# and a single MI async notification.
> +	set mi_re [make_mi_re $mode 5 0 response]
> +	set cli_re [make_cli_re $mode 2 2.2 0]
> +
> +	with_spawn_id $mi_spawn_id {
> +	    mi_gdb_test "-thread-select 5" $mi_re "-thread-select"
> +	}
> +
> +	with_spawn_id $gdb_main_spawn_id {
> +	    match_re_or_ensure_no_output "$cli_re\r\n" "-thread-select, event on CLI"
> +	}
> +
> +    }
>   }
>   
>   proc_with_prefix test_mi_stack_select_frame { mode } {
> @@ -1290,9 +1315,22 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } {
>   	}
>       }
>   
> -    # Idea for the future: selecting a thread in a different inferior.  For now,
> -    # GDB doesn't show an inferior switch, but if it did, it would be a nice
> -    # place to test it.
> +    with_test_prefix "thread 2.2" {
> +	# Select a thread in a different inferior.  This should trigger an
> +	# 'inferior changed' and 'thread changed' notification on the CLI,
> +	# and a single MI async notification.
> +	set command [make_cli_in_mi_command $cli_in_mi_mode "thread 2.2"]
> +	set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 1 2 2.2 5 0]
> +	set cli_re [make_cli_re $mode 2 2.2 0]
> +
> +	with_spawn_id $mi_spawn_id {
> +	    mi_gdb_test $command $mi_re "select thread"
> +	}
> +
> +	with_spawn_id $gdb_main_spawn_id {
> +	    match_re_or_ensure_no_output "$cli_re\r\n" "select thread, event on CLI"
> +	}
> +    }
>   }
>   
>   # Test selecting the frame using a CLI command in the MI channel.
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 0788bea235a..9ca1a8d973a 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1983,10 +1983,19 @@ thread_command (const char *tidstr, int from_tty)
>       }
>     else
>       {
> +      inferior *previous_inferior = current_inferior ();
> +
>         thread_select (tidstr, parse_thread_id (tidstr, NULL));
>   
> -      notify_user_selected_context_changed
> -	(USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> +      user_selected_what selection = (USER_SELECTED_THREAD
> +				      | USER_SELECTED_FRAME);
> +
> +      /* If the inferior changed as a consequence of the thread change,
> +	 then let the user know.  */
> +      if (previous_inferior != current_inferior ())
> +	selection |= USER_SELECTED_INFERIOR;
> +
> +      notify_user_selected_context_changed (selection);
>       }
>   }
>   
>
> base-commit: 449035c35f2169e0c690d83f28306275ab7f7463


-- 
Cheers,
Guinevere Larsen
It/she