Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Markus Metzger <markus.t.metzger@intel.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simark@simark.ca>
Subject: [PATCH v2] gdb: fix an issue with thread list corruption
Date: Tue,  5 May 2026 05:06:18 +0000	[thread overview]
Message-ID: <20260505050618.1366426-1-markus.t.metzger@intel.com> (raw)

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


             reply	other threads:[~2026-05-05  5:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05  5:06 Markus Metzger [this message]
2026-07-17 17:18 ` Tom Tromey
2026-07-20  6:03   ` Metzger, Markus T

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260505050618.1366426-1-markus.t.metzger@intel.com \
    --to=markus.t.metzger@intel.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox