RE: [PATCH] gdb: use correct target in notify_thread_exited()

["Metzger, Markus T" <markus.t.metzger@intel.com>]

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