gdb: use correct target in notify_thread_exited()

  clean_up_just_stopped_threads_fsms() may call notify_thread_exited() in
the context of a thread from a different target.  In my case:

    at .../gdb/thread.c:404
    exit_code=std::optional [no contained value], silent=false)
    at .../gdb/thread.c:436
    exit_code=std::optional [no contained value], silent=false)
    at .../gdb/thread.c:733
    at .../gdb/thread.c:763
    at .../gdb/infrun.c:4548
    at .../gdb/infrun.c:4786

Instead of relying on global state, use the exited thread's inferior's
target, which is exactly the target we want.
---
 gdb/thread.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

On 5/4/26 3:16 AM, Markus Metzger wrote:
> clean_up_just_stopped_threads_fsms() may call notify_thread_exited() in
> the context of a thread from a different target.  In my case:
> 
>     at .../gdb/thread.c:404
>     exit_code=std::optional [no contained value], silent=false)
>     at .../gdb/thread.c:436
>     exit_code=std::optional [no contained value], silent=false)
>     at .../gdb/thread.c:733
>     at .../gdb/thread.c:763
>     at .../gdb/infrun.c:4548
>     at .../gdb/infrun.c:4786

This is missing the function names, so not very useful.

> 
> Instead of relying on global state, use the exited thread's inferior's
> target, which is exactly the target we want.
> ---
>  gdb/thread.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 74124596b28..5c3b60b2aa7 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -203,14 +203,16 @@ notify_thread_exited (thread_info *t, std::optional<ULONGEST> exit_code,
>  {
>    if (!silent && print_thread_events)
>      {
> +      struct target_ops *target = t->inf->top_target ();
> +
>        if (exit_code.has_value ())
>  	gdb_printf (_("[%s (id %s) exited with code %s]\n"),
> -		    target_pid_to_str (t->ptid).c_str (),
> +		    target->pid_to_str (t->ptid).c_str (),
>  		    print_thread_id (t),
>  		    pulongest (*exit_code));
>        else
>  	gdb_printf (_("[%s (id %s) exited]\n"),
> -		    target_pid_to_str (t->ptid).c_str (),
> +		    target->pid_to_str (t->ptid).c_str (),
>  		    print_thread_id (t));

This is unfortunately not correct, because target calls (annoyingly IMO)
use the target stack of the current inferior to find the target beneath.
If you are having to do this change, it means that this function is
called for a thread of inferior A, while inferior B is the current one.
What will happen is that you'll start with the top target of inferior A,
but if that target calls "target_ops::beneath()" to delegate to the
target beneath, that will jump to a target of inferior B.  If the
pid_to_str operation is handled by the top target of inferior A, it will
work, but not in the general case.

As of today, the only fix for that would be to temporarily switch
inferior.

I once made a prototype [1] of passing down the target stack explicitly to
all target functions.  It would be quite invasive, but I don't see any
other solution if we want to lift that "target calls care about current
inferior" limitation.

Simon

[1] https://review.lttng.org/c/binutils-gdb/+/5627
  

Hello Simon,

>On 5/4/26 3:16 AM, Markus Metzger wrote:
>> clean_up_just_stopped_threads_fsms() may call notify_thread_exited() in
>> the context of a thread from a different target.  In my case:
>>
>>     at .../gdb/thread.c:404
>>     exit_code=std::optional [no contained value], silent=false)
>>     at .../gdb/thread.c:436
>>     exit_code=std::optional [no contained value], silent=false)
>>     at .../gdb/thread.c:733
>>     at .../gdb/thread.c:763
>>     at .../gdb/infrun.c:4548
>>     at .../gdb/infrun.c:4786
>
>This is missing the function names, so not very useful.

I removed it in v2.

>> Instead of relying on global state, use the exited thread's inferior's
>> target, which is exactly the target we want.
>> ---
>>  gdb/thread.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/thread.c b/gdb/thread.c
>> index 74124596b28..5c3b60b2aa7 100644
>> --- a/gdb/thread.c
>> +++ b/gdb/thread.c
>> @@ -203,14 +203,16 @@ notify_thread_exited (thread_info *t,
>std::optional<ULONGEST> exit_code,
>>  {
>>    if (!silent && print_thread_events)
>>      {
>> +      struct target_ops *target = t->inf->top_target ();
>> +
>>        if (exit_code.has_value ())
>>  	gdb_printf (_("[%s (id %s) exited with code %s]\n"),
>> -		    target_pid_to_str (t->ptid).c_str (),
>> +		    target->pid_to_str (t->ptid).c_str (),
>>  		    print_thread_id (t),
>>  		    pulongest (*exit_code));
>>        else
>>  	gdb_printf (_("[%s (id %s) exited]\n"),
>> -		    target_pid_to_str (t->ptid).c_str (),
>> +		    target->pid_to_str (t->ptid).c_str (),
>>  		    print_thread_id (t));
>
>This is unfortunately not correct, because target calls (annoyingly IMO)
>use the target stack of the current inferior to find the target beneath.
>If you are having to do this change, it means that this function is
>called for a thread of inferior A, while inferior B is the current one.
>What will happen is that you'll start with the top target of inferior A,
>but if that target calls "target_ops::beneath()" to delegate to the
>target beneath, that will jump to a target of inferior B.  If the
>pid_to_str operation is handled by the top target of inferior A, it will
>work, but not in the general case.
>
>As of today, the only fix for that would be to temporarily switch
>inferior.

Those scoped_restore_foo are everywhere.  I tried to not rely on
global state and instead use the arguments passed to the function,
but I see now how this cannot work.  Makes you wonder why we
even bother passing thread_info * and inferior *.

I switch to the exited thread in v2.

>I once made a prototype [1] of passing down the target stack explicitly to
>all target functions.  It would be quite invasive, but I don't see any
>other solution if we want to lift that "target calls care about current
>inferior" limitation.
>
>[1] https://review.lttng.org/c/binutils-gdb/+/5627

It's every call to inferior_thread() and current_inferior(), too.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

On 2026-05-05 03:56, Metzger, Markus T wrote:
>> This is unfortunately not correct, because target calls (annoyingly IMO)
>> use the target stack of the current inferior to find the target beneath.
>> If you are having to do this change, it means that this function is
>> called for a thread of inferior A, while inferior B is the current one.
>> What will happen is that you'll start with the top target of inferior A,
>> but if that target calls "target_ops::beneath()" to delegate to the
>> target beneath, that will jump to a target of inferior B.  If the
>> pid_to_str operation is handled by the top target of inferior A, it will
>> work, but not in the general case.
>>
>> As of today, the only fix for that would be to temporarily switch
>> inferior.
> 
> Those scoped_restore_foo are everywhere.  I tried to not rely on
> global state and instead use the arguments passed to the function,
> but I see now how this cannot work.  Makes you wonder why we
> even bother passing thread_info * and inferior *.

Indeed, if a random function takes an inferior parameter but also
requires that this inferior is the current one, it can be misleading.
It makes you think that it is global-context-agnostic, while it is not.
I try not to do this.

I would say that this "target calls rely on current inferior's target
stack" is the major blocker for continuing doing cleanups to pass
context through parameters, rather than global variables.

> I switch to the exited thread in v2.
> 
>> I once made a prototype [1] of passing down the target stack explicitly to
>> all target functions.  It would be quite invasive, but I don't see any
>> other solution if we want to lift that "target calls care about current
>> inferior" limitation.
>>
>> [1] https://review.lttng.org/c/binutils-gdb/+/5627
> 
> It's every call to inferior_thread() and current_inferior(), too.

I am not sure what you mean here, those are just simple getters.

Simon
  

Hello Simon,

>> It's every call to inferior_thread() and current_inferior(), too.
>
>I am not sure what you mean here, those are just simple getters.

I meant that they get global state while it would be cleaner to pass
the inferior or thread as argument.  If some function you end up
calling uses inferior_thread(), you're still required to switch to that
thread and back.

It would be nice if the current thread and inferior were UI local state.

Markus.
Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928