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

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

[Markus Metzger]

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().

CC: Simon Marchi <simark@simark.ca>
---
 gdb/infcmd.c | 53 ++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f3f15c9ae1d..af61c468f7a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -685,29 +685,6 @@ starti_command (const char *args, int from_tty)
   run_command_1 (args, from_tty, RUN_STOP_AT_FIRST_INSN);
 }
 
-static void
-proceed_thread_callback (struct thread_info *thread)
-{
-  /* We go through all threads individually instead of compressing
-     into a single target `resume_all' request, because some threads
-     may be stopped in internal breakpoints/events, or stopped waiting
-     for its turn in the displaced stepping queue (that is, they are
-     running from the user's perspective but internally stopped).  The
-     target side has no idea about why the thread is stopped, so a
-     `resume_all' command would resume too much.  If/when GDB gains a
-     way to tell the target `hold this thread stopped until I say
-     otherwise', then we can optimize this.  */
-  if (thread->state () != THREAD_STOPPED)
-    return;
-
-  if (!thread->inf->has_execution ())
-    return;
-
-  switch_to_thread (thread);
-  clear_proceed_status (0);
-  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-}
-
 static void
 ensure_valid_thread (void)
 {
@@ -746,12 +723,12 @@ ensure_not_running (void)
 }
 
 void
-continue_1 (int all_threads)
+continue_1 (int all)
 {
   ERROR_NO_INFERIOR;
   ensure_not_tfind_mode ();
 
-  if (non_stop && all_threads)
+  if (non_stop && all)
     {
       /* Don't error out if the current thread is running, because
 	 there may be other stopped threads.  */
@@ -762,7 +739,31 @@ continue_1 (int all_threads)
       scoped_disable_commit_resumed disable_commit_resumed
 	("continue all threads in non-stop");
 
-      for_each_thread (proceed_thread_callback);
+      /* Do not use all_threads_safe, because it's possible for the next
+	 thread to get removed while resuming THREAD.  We know that thread
+	 is stopped and should not disappear under our feet.  */
+      for (thread_info &thread : all_threads ())
+	{
+	  /* We go through all threads individually instead of compressing
+	     into a single target `resume_all' request, because some
+	     threads may be stopped in internal breakpoints/events, or
+	     stopped waiting for its turn in the displaced stepping queue
+	     (that is, they are running from the user's perspective but
+	     internally stopped).  The target side has no idea about why
+	     the thread is stopped, so a `resume_all' command would resume
+	     too much.  If/when GDB gains a way to tell the target `hold
+	     this thread stopped until I say otherwise', then we can
+	     optimize this.  */
+	  if (thread.state () != THREAD_STOPPED)
+	    continue;
+
+	  if (!thread.inf->has_execution ())
+	    continue;
+
+	  switch_to_thread (&thread);
+	  clear_proceed_status (0);
+	  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
+	}
 
       if (current_ui->prompt_state == PROMPT_BLOCKED)
 	{
-- 
2.34.1

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

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

[Tom Tromey]

>>>>> "Markus" == Markus Metzger <markus.t.metzger@intel.com> writes:

Markus> The thread we're proceeding is stopped and there is no reason to delete
Markus> it.  Consequently, there is no reason for all_threads_safe(), which isn't
Markus> that safe in this scenario.

Markus> Iterate using all_threads() and inline proceed_thread_callback().

Thanks, this is ok.  Somehow I missed this earlier.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

[Metzger, Markus T]

>Markus> The thread we're proceeding is stopped and there is no reason to
>delete
>Markus> it.  Consequently, there is no reason for all_threads_safe(), which
>isn't
>Markus> that safe in this scenario.
>
>Markus> Iterate using all_threads() and inline proceed_thread_callback().
>
>Thanks, this is ok.  Somehow I missed this earlier.
>
>Approved-By: Tom Tromey <tom@tromey.com>

The patch was replaced by

    https://sourceware.org/pipermail/gdb-patches/2026-May/227320.html

which you already approved.

Thanks,
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