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

Message ID 20260505050618.1366426-1-markus.t.metzger@intel.com
State New
Headers
Series [v2] gdb: fix an issue with thread list corruption |

Commit Message

Metzger, Markus T May 5, 2026, 5:06 a.m. UTC
  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(-)
  

Patch

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)
 	{