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

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves <pedro@palves.net> writes:

> Hi!
>
> On 2025-10-08 09:48, Andrew Burgess wrote:
>
>> 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.
>
> AMD and Intel have been having discussions about settling on how to 
> expose GPU lane debugging in GDB for the past year, and this very detail
> came up there.
>
> (There was a presentation/BoF about the topic at the Cauldron and videos/slides
> are already up:
> https://conf.gnu-tools-cauldron.org/opo25/talk/FN7FKL/ )
>
> For lane debugging support, we're going to propose a new "lane" command (along with
> 'info lanes', 'lane find', 'break foo lane N' etc.), where "lane" is new level of
> execution context.  Currently an inferior has threads.  And with lane debugging, 
> a thread has lanes.  So inferior has threads, thread has lanes.  A lane has
> a fully-qualified ID like "I.T.L" (inferior.thread.lane), just like threads have a
> fully qualified ID like "I.T".
>
> For the issue at hand, it works like this:
>
>  (gdb) inferior 1
>  [Switching to inferior 1 [process 3286570] (/home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.rocm/simple/simple)]
>  [Switching to thread 1.7 (AMDGPU Wave 1:2:1:1 (0,0,0)/0)]
>  [Switching to lane 1.7.0 (AMDGPU Lane 1:2:1:1/0 (0,0,0)[0,0,0])]
>  #0  do_an_addition (a=1, b=2, out=0x7ffee3e00000) at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.cpp:24
>  24        *out = a + b;
>  (gdb) thread 7
>  [Switching to thread 1.7 (AMDGPU Wave 1:2:1:1 (0,0,0)/0)]
>  [Switching to lane 1.7.0 (AMDGPU Lane 1:2:1:1/0 (0,0,0)[0,0,0])]
>  #0  do_an_addition (a=1, b=2, out=0x7ffee3e00000) at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.cpp:24
>  24        *out = a + b;
>  (gdb) lane 0
>  [Switching to lane 1.7.0 (AMDGPU Lane 1:2:1:1/0 (0,0,0)[0,0,0])]
>  #0  do_an_addition (a=1, b=2, out=0x7ffee3e00000) at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.cpp:24
>  24        *out = a + b;
>
>
> Note this is just reusing the logic of the current behavior you observed
> with "inferior" and "thread".
>
> There is a logic/pattern to it that I think is the right balance.
>
> Let me explain.
>
> When you do "inferior 1", GDB also switches to some thread of inferior 1.
> Reusing your example, if GDB didn't print about what thread it switched
> to, and just said:
>
>  (gdb) inferior 1
>  [Switching to inferior 1 [process 818430] (/home/andrew/tmp/thr)]
>  #0  0xf7eb2888 in clone () from /lib/libc.so.6
>
> then the user would have no way to know what thread is now current,
> and would be forced to issue the "thread" command to get it.  So
> that's why current GDB prints the "Switch to thread" part too:
>
>  (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
>
> This however, does not apply when you use the "thread" command instead
> of the "inferior" command, as in, you are not _forced_ to use the
> "inferior" command.  That is because in that case, the
> "Switching to thread 2.2" part, as in your example:
>
>  (gdb) thread 2.2
>  [Switching to thread 2.2 (Thread 0xf7dbbb40 (LWP 818457))]
>   #0  breakpt () at thr.c:19
>   19	  while (stop)
>
> ... that "thread 2.2" part already tells you which inferior it is
> GDB switched to.  It's inferior 2, from "2.2", which is "I.T".
>
> That is why in my example above, when I switch to lane 0 with the
> "lane" command, GDB only needs to tell me about the lane switch and
> not the "inferior" and "thread" switching:
>
>  (gdb) lane 0
>  [Switching to lane 1.7.0 (AMDGPU Lane 1:2:1:1/0 (0,0,0)[0,0,0])]
>  #0  do_an_addition (a=1, b=2, out=0x7ffee3e00000) at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.cpp:24
>  24        *out = a + b;
>
> From "lane 1.7.0", I already know the inferior (1) and the thread (7), so
> GDB doesn't need to say it.  I _can_ use the "inferior" and "thread"
> commands to get more information if I want, but I'm not _forced_ to.
>
> So the current logic, extended to lanes, is I think simple -- when you
> switch to an execution object using a "FOO" command, then GDB informs
> you about the current FOO it switched to, as well as any finer
> execution object context below FOO.  I.e., 
>
>  - if you use the "inferior" command, GDB tells you about
>    "inferior", "thread" and "lane" (if lanes exist).
>
>  - if you use the "thread" command, GDB tells you about
>    "thread" and "lane" (if lanes exist).
>
>  - if you use the "lane" command, GDB tells you about
>    "lane".

I like this list, because I think it perfectly explains why I disagree
with your position.  But lets first take a quick look at your next
example... 

> This reduces noise for a set of core commands that are used all the time.
> When you're focused on switching threads around to debug something, you don't
> need to be told what is the current program the inferior is running.
> IMHO, it ends up being noise after a short while, like:
>
>  (gdb) lane 0
>  [Switching to inferior 1 [process 3286570] (/home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.rocm/simple/simple)]
>  [Switching to thread 1.7 (AMDGPU Wave 1:2:1:1 (0,0,0)/0)]
>  [Switching to lane 1.7.0 (AMDGPU Lane 1:2:1:1/0 (0,0,0)[0,0,0])]
>  #0  do_an_addition (a=1, b=2, out=0x7ffee3e00000) at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.cpp:24
>  24        *out = a + b;
>
>  (gdb) lane 1
>  [Switching to inferior 1 [process 3286570] (/home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.rocm/simple/simple)]
>  [Switching to thread 1.7 (AMDGPU Wave 1:2:1:1 (0,0,0)/0)]
>  [Switching to lane 1.7.1 (AMDGPU Lane 1:2:1:1/1 (0,0,0)[0,0,1])]
>  #0  do_an_addition (a=<lane inactive>, b=<lane inactive>, out=<lane inactive>)
>      at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.cpp:24
>  24        *out = a + b;

I agree with your conclusion that the "Switching to inferior..." output
is unnecessary in the example you just gave.  But given you mentioned it
in response to my patch, but didn't really touch on how my patch is
different, I think it's worth mentioning, just in case this was missed.

The above does not represent what I'm proposing, and is not what the
patch I posted implements; obviously I'm talking inferiors and threads
only here, but I would be against implementing the above behaviour for
lanes too, which I think is your position.

Notice that in the above case the inferior and thread didn't change.
That's the important distinction, which is maybe missing from this
discussion.  I'm not suggesting that we should display the inferior
during a thread change if the inferior doesn't actually change.  Nor
would I suggest that you display the inferior and thread when switching
lane, if the inferior and/or thread didn't change.

I'll try rewriting one of your examples by handle to more closely match
what I'm actually proposing (or would propose in the future when lane's
exist):

  (gdb) lane 2.3.1
  [Switching to inferior 2 [process 3286570] (/home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.rocm/blah/blah)]
  [Switching to thread 2.3 (AMDGPU Wave ... snip ...]
  [Switching to lane 2.3.1 (AMDGPU Lane ... snip ...]
  #0  some_function (...snip...) file.c:23
  24        // ... some code ...

Here the user used the lane command to change inferior, thread, and
lane.  Now I think GDB should display all the things that changed.  Why
I hear you ask, lets revisit your list from above:

>  - if you use the "inferior" command, GDB tells you about
>    "inferior", "thread" and "lane" (if lanes exist).
>
>  - if you use the "thread" command, GDB tells you about
>    "thread" and "lane" (if lanes exist).
>
>  - if you use the "lane" command, GDB tells you about
>    "lane".

You argument seems to be that if I'm in thread 1.1 and use the command
'thread 2.2' to switch both inferior and thread, then I don't need to be
told about the inferior change, that information is given in the command
already.

But by this argument, why does the "inferior" command show the inferior?
Surely that information is present in the command, and is just "noise".
So the inferior command should only show the thread and lane.

Similarly, the thread command should only show the lane being switched
too, not which thread was selected.

And the lane command should only show the ... well, nothing (actually
all of the above show the frame too, but lets ignore that for now).  As
the lane being selected is right there in the command.

I'm not seriously suggesting we change GDB inline with what I just
wrote.  My position is that when something changes we should print what
it is we just switched too.  Just like you say in your list above.
Switching inferior?  Print the new inferior details, along with the
thread, lane, and frame.  Switching thread _only_, print thread, lane,
and frame. Etc.

I actually feel that 'thread 2.2' really should be thought of as short
hand for "inferior 2; thread 2", and as such should act just as an
inferior command followed by a thread command would.

Is this noise?  Sure.  But is this overwhelming?  I don't feel so.  I
think letting the user know when something changes is OK.

>
> etc., etc.
>
>
> IOW, even though you could argue that the "Switching to inferior ..."
> information could be useful when you use the "thread" command, that is
> information that the user can easily retrieve with the "inferior"
> command.  I think reducing noise is more important here, especially when
> we start adding other levels of execution objects, like lanes.

I was actually doubting myself so much by this point that I went and
retested my patch.  Here's it in action:

  (gdb) info threads 
    Id   Target Id                               Frame 
    1.1  Thread 0xf7dbc700 (LWP 1750862) "thr_a" breakpt () at thr.c:19
    1.2  Thread 0xf7dbbb40 (LWP 1750865) "thr_a" breakpt () at thr.c:19
    1.3  Thread 0xf75bab40 (LWP 1750866) "thr_a" breakpt () at thr.c:19
    1.4  Thread 0xf6bffb40 (LWP 1750867) "thr_a" breakpt () at thr.c:19
  * 2.1  Thread 0xf7dbc700 (LWP 1750910) "thr_b" breakpt () at thr.c:19
    2.2  Thread 0xf7dbbb40 (LWP 1750911) "thr_b" 0xf7fd0579 in __kernel_vsyscall ()
    2.3  Thread 0xf73ffb40 (LWP 1750912) "thr_b" breakpt () at thr.c:19
    2.4  Thread 0xf6bfeb40 (LWP 1750913) "thr_b" breakpt () at thr.c:19
  (gdb) thread 2
  [Switching to thread 2.2 (Thread 0xf7dbbb40 (LWP 1750911))]
  #0  0xf7fd0579 in __kernel_vsyscall ()
  (gdb) thread 1.3
  [Switching to inferior 1 [process 1750862] (/tmp/thr_a)]
  [Switching to thread 1.3 (Thread 0xf75bab40 (LWP 1750866))]
  #0  breakpt () at thr.c:19
  19	  while (stop)
  (gdb) thread 1.2
  [Switching to thread 1.2 (Thread 0xf7dbbb40 (LWP 1750865))]
  #0  breakpt () at thr.c:19
  19	  while (stop)
  (gdb) 

GDB only printed the "Switching to inferior ..." line when we actually
switched inferiors.

>
> (If you happen to have an AMD GPU, you can play with my examples above
> using this branch:
>   https://github.com/ROCm/ROCgdb/tree/users/palves/lane-debugging)

Sadly not.

I'd also like to draw you attention to the gdb/mi/* changes in this
patch as you didn't give your thoughts on that aspect of the change.

That part of the patch handles the case where a user as an MI and CLI
interpreter running, and the inferior/thread changes as a result of an
MI action.

In this case, the MI action might be a click, or miss-click in a UI, or
might even be some automated action by the MI frontend that the user
didn't (knowingly) perform.  In this case, currently, the CLI will
receive a thread/frame change notification (i.e. the CLI interpreter
prints the new thread and frame), but the inferior notification is
missing.  My patch changed GDB so the CLI would (when appropriate) also
receive an inferior changed notification, and so also print a "Switching
to inferior..." line.

Surely in that case, your "the inferior change is known from the
command" argument doesn't apply, right?

But then there's a whole argument about consistency.  If I change
inferior and thread via the MI the CLI gets a "Switching to inferior"
line, but if I do the same switch via the CLI, I wouldn't.  Which I
dislike.

Anyway, I still feel like this change is the right thing to do.  I'd
love to hear your thoughts on the points I've raised.

Clearly though, my original patch did a poor job of explaining that the
extra "Switching to inferior" line would only appear when necessary, so
if I can get you on board with this change, I'll rewrite the commit
message to make that _much_ clearer.

Thanks,
Andrew