Re: [PATCH] gdb: fix an issue with thread list corruption

[Simon Marchi <simark@simark.ca>]

On 2026-05-06 01:43, Metzger, Markus T wrote:
> Hello Simon,
> 
>>>> On 5/4/26 3:16 AM, Markus Metzger wrote:
>>>>> When resuming a target in non-stop mode with 'c -a', the continue
>>>> command
>>>>> uses for_each_thread() to proceed each stopped thread individually.  This
>>>>> uses an all_threads_safe() iteration.
>>>>>
>>>>> If one of the stopped threads does an inline step-over, since the target
>>>>> is non-stop, we stop_all_threads(), which involves update_thread_list(),
>>>>> which, in turn, may delete_thread().
>>>>>
>>>>> If this deleted the thread pointed to by the m_next safe iterator member,
>>>>> the above all_threads_safe() iteration will be corrupted.
>>>>>
>>>>> The thread we're proceeding is stopped and there is no reason to delete
>>>>> it.  Consequently, there is no reason for all_threads_safe(), which isn't
>>>>> that safe in this scenario.
>>>>>
>>>>> Iterate using all_threads() and inline proceed_thread_callback().
>>>>
>>>> Just to try to make sure I understand the circumstances that lead the
>>>> the failure correcty (and make sure this doesn't just cover up other
>>>> problems):
>>>>
>>>> - thread 1 is stopped on a breakpoint
>>>> - thread 2 is executing
>>>> - displaced stepping is disabled
>>>> - the target doesn't report thread events to the core
>>>> - you "continue -a"
>>>> - thread 1 starts an inline step-over, which calls stop_all_threads,
>>>>   which calls update_thread_list
>>>> - meanwhile, thread 2 exits
>>>> - update_thread_list causes thread 2 to be deleted
>>>> - the safe iterator's m_next field now points to a deleted thread info
>>
>> Can you comment on whether the sequence of events above sounds correct?
>> If it is, I think it would be possible to write a CPU test to reproduce
>> it, that would be helpful along with the fix.
> 
> The sequence sounds good, but I wonder how we would ensure that thread 2
> exits just when we need it to.

Usually, by hammering.  But I think it's not as difficult as it sounds.
The state before the "continue -a" on thread N is that GDB knows about
thread N+1.  Assuming that thread N+1 is a short-lived thread (one that
does nothing but exit), it exits even before the "continue -a" is
issued, but GDB doesn't know that until the next thread list update.
When you "continue -a", GDB iterates using all_threads_safe, without
doing any thread list update.  When iteration reaches thread N, the next
thread is N+1, exited on the target but it still exists in GDB's mind.
Thread N starts its inline step-over, and etc etc until the crash.

> This is non-stop mode, so we should learn about the exit immediately.
> The test would also need to rely on the ordering of threads in the
> thread list.  And detecting the issue requires the next pointer of the
> next element to be overwritten in a way that causes a deterministic
> effect - like crashing GDB.  We wouldn't want to add another test that
> fails sporadically.

I wrote a test that fails consistently (at least with ASan) with
the board native-extended-gdbserver:

https://review.lttng.org/c/binutils-gdb/+/17704

    (gdb) PASS: gdb.threads/continue-a-step-over-other-thread-exit.exp: step_over_thread_slot=1: continue to break_here
    continue -a
    Continuing.
    =================================================================
    ==1250831==ERROR: AddressSanitizer: heap-use-after-free on address 0x7d0984eb4480 at pc 0x564f8b3da4dd bp 0x7ffdad6a63d0 sp 0x7ffdad6a63c0

With your fixes, it passes.

Feel free to check it out and add it to your patch.  It will need a bit
of cleanup, and there is a FIXME saying that it won't work with
native-gdbserver because it requires inferior args.  I have another
patch series that I need to send that add the ability to pass inferior
args when using native-gdbserver, I'll try to get that done.

Simon