[PATCH 2/2] gdb: share some thread proceed related code between CLI and MI

Andrew Burgess <[email protected]>
Newsgroups gmane.comp.gdb.patches
Message-ID <55e17e264f17f47680100684c9aa7bd9b88e377c.1786049312.git.aburgess@redhat.com>
I noticed that some code related to proceeding threads could be shared
between CLI and MI.  This fixes a bug as the CLI code contains a fix
that the MI code is missing.

In continue_1 (in infcmd.c) we have a loop that iterates over all
threads looking for threads that are THREAD_STOPPED and are in an
inferior that has_execution.  For each thread found we then call:

  switch_to_thread (&thread);
  clear_proceed_status (0);
  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);

In exec_continue (in mi/mi-main.c) we also have a loop over all
threads that calls the proceed_thread helper function which skips
threads that are not THREAD_STOPPED, does some PID related
filtering (more on this later) and then calls the same three
functions: switch_to_thread, clear_proceed_status, proceed.

The PID filtering mentioned above allows proceed_thread to do two
jobs, if the PID is zero then we proceed all threads.  If PID is
non-zero then we proceed only the threads in the inferior with that
PID.

You might also have spotted that in continue_1 we checked if the
inferior has execution or not.  This check was added in commit:

  commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2
  Date:   Fri Jan 10 20:06:08 2020 +0000

      Multi-target support

A matching check was not added into the MI at this point, nor did the
commit message mention why such a check was not added.  I'm choosing
to believe that this was an oversight in the 5b6d1e4fa4fc6827 commit.
And this is the bug I mentioned above.

If we call `proceed` with a thread that is part of an inferior that
does not "has_execution" then the thread will be marked running even
though it will never actually be set running.  See the early return at
the top of `proceed_resume_thread_checked` and the call to set_state
in `proceed`.

I do worry that there might be a bigger set of bugs here if proceed
can set a thread's state to THREAD_RUNNING, but then never actually
sets the underlying thread running.  But in this case, just having the
MI share code with the CLI means that we pick up the fix for this case
basically for free.

I propose adding a new global helper function `proceed_one_thread`,
this will check if the thread is THREAD_STOPPED and is in an inferior
which has_execution.  If these conditions are met then the three
functions mentioned above will be called to proceed the thread.

I will then add a second new function `proceed_all_threads`, this will
iterate over all threads and call proceed_one_thread.

We can then use proceed_all_threads from continue_1, replacing the
existing loop.

In exec_continue we can move the PID (or rather inferior) check
earlier, outside the loop.  If we want to resume all threads (the old
PID is zero path) then we call proceed_all_threads.  If we only want
to proceed threads with one PID then we loop over threads in the
matching inferior and call proceed_one_thread on each.

As the code I am factoring out is all within non_stop only paths I
have added `gdb_assert (non_stop);` to each of the new helper
functions, this will prevent these functions accidentally being called
in the all_stop code path.

While moving the two `for (...)` loops I have replaced 'auto' with
'thread_info' for additional type clarity.

With the exception of the new has_execution check in the MI path there
should be no other user visible changes with this commit.  I've added
a new test which exposes the missing has_execution check issue.
---
 gdb/infcmd.c                                  | 62 ++++++++----
 gdb/inferior.h                                | 11 +++
 gdb/mi/mi-main.c                              | 31 ++----
 gdb/testsuite/gdb.mi/mi-corefile-and-live.c   | 37 +++++++
 gdb/testsuite/gdb.mi/mi-corefile-and-live.exp | 97 +++++++++++++++++++
 5 files changed, 195 insertions(+), 43 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-corefile-and-live.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-corefile-and-live.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3e943123519..23e93587874 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -722,6 +722,46 @@ ensure_not_running (void)
     error_is_running ();
 }
 
+/* See inferior.h.  */
+
+void
+proceed_one_thread (thread_info &thread)
+{
+  gdb_assert (non_stop);
+
+  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);
+}
+
+/* See inferior.h.  */
+
+void
+proceed_all_threads ()
+{
+  gdb_assert (non_stop);
+
+  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.  */
+      proceed_one_thread (thread);
+    }
+}
+
 void
 continue_1 (bool all_threads_p)
 {
@@ -739,27 +779,7 @@ continue_1 (bool all_threads_p)
       scoped_disable_commit_resumed disable_commit_resumed
 	("continue all threads in non-stop");
 
-      for (auto &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);
-	}
+      proceed_all_threads ();
 
       if (current_ui->prompt_state == PROMPT_BLOCKED)
 	{
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 217741adc07..8255654d9b8 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -227,6 +227,17 @@ extern void registers_info (const char *, bool);
 
 extern void continue_1 (bool all_threads_p);
 
+/* For use only when non_stop is true.  Proceed all threads in every
+   inferior.  */
+
+extern void proceed_all_threads ();
+
+/* For use only when non_stop is true.  If THREAD is stopped, and is in an
+   inferior that has_execution then switch to THREAD, clear its proceed
+   status, and proceed the thread.  */
+
+extern void proceed_one_thread (thread_info &thread);
+
 extern void interrupt_target_1 (bool all_threads);
 
 using delete_longjmp_breakpoint_cleanup
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 903c6a5f411..e110b029e1e 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -235,20 +235,6 @@ mi_cmd_exec_jump (const char *args, const char *const *argv, int argc)
   mi_execute_async_cli_command ("jump", argv, argc);
 }
 
-static void
-proceed_thread (struct thread_info *thread, int pid)
-{
-  if (thread->state () != THREAD_STOPPED)
-    return;
-
-  if (pid != 0 && thread->ptid.pid () != pid)
-    return;
-
-  switch_to_thread (thread);
-  clear_proceed_status (0);
-  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-}
-
 static void
 exec_continue (const char *const *argv, int argc)
 {
@@ -262,24 +248,25 @@ exec_continue (const char *const *argv, int argc)
 	 all threads in all inferiors, we need to iterate over
 	 threads.
 
-	 See comment on infcmd.c:proceed_thread_callback for rationale.  */
+	 See comment in infcmd.c:proceed_all_threads for rationale.  */
       if (current_context->all || current_context->thread_group != -1)
 	{
 	  scoped_restore_current_thread restore_thread;
 	  scoped_disable_commit_resumed disable_commit_resumed
 	    ("MI continue all threads in non-stop");
-	  int pid = 0;
 
+	  inferior *inf = nullptr;
 	  if (!current_context->all)
-	    {
-	      struct inferior *inf
-		= find_inferior_id (current_context->thread_group);
+	    inf = find_inferior_id (current_context->thread_group);
 
-	      pid = inf->pid;
+	  if (inf == nullptr)
+	    proceed_all_threads ();
+	  else
+	    {
+	      for (thread_info &thread : inf->threads ())
+		proceed_one_thread (thread);
 	    }
 
-	  for (auto &thread : all_threads ())
-	    proceed_thread (&thread, pid);
 	  disable_commit_resumed.reset_and_commit ();
 	}
       else
diff --git a/gdb/testsuite/gdb.mi/mi-corefile-and-live.c b/gdb/testsuite/gdb.mi/mi-corefile-and-live.c
new file mode 100644
index 00000000000..831c7f0de7f
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-corefile-and-live.c
@@ -0,0 +1,37 @@
+/* Copyright 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+void
+bar (void)
+{
+  abort ();
+}
+
+void
+foo (void)
+{
+  bar ();
+}
+
+int
+main (int argc, char **argv)
+{
+  foo ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp
new file mode 100644
index 00000000000..a3a57d3c81a
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp
@@ -0,0 +1,97 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# In non-stop mode with schedule-multiple turned on, create two
+# inferiors, a core target and one other.  Then use the command
+# '-exec-continue --all'.
+#
+# The core target threads should not be marked as running by this.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+require isnative
+require allow_multi_inferior_tests
+
+standard_testfile
+
+if {[build_executable "build executable" $testfile $srcfile] == -1} {
+    return
+}
+
+set corefile [core_find $binfile]
+if {$corefile == ""} {
+    untested "unable to create or find corefile"
+    return
+}
+
+# Start GDB in non-stop and schedule-multiple mode.
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -ex \"set non-stop on\""
+    append GDBFLAGS " -ex \"set schedule-multiple on\""
+    mi_clean_restart $::testfile
+}
+
+if {[mi_runto_main] == -1} {
+    return
+}
+
+# Arrange for inferior 1, our non core file inferior, to stop before
+# it hits the abort call.
+mi_create_breakpoint "-g i1 foo" \
+    "set breakpoint on foo" \
+    -inferior 1
+
+# Setup inferior 2, this will load the core file.
+mi_gdb_test "-add-inferior" \
+    [multi_line "=thread-group-added,id=\"i2\"" \
+	 "~\"\\\[New inferior 2\\\]\\\\n\"" \
+	 "\~\"Added inferior 2\[^\r\n\]*\\\\n\"" \
+	 "\\^done,inferior=\"\[^\"\]+\"(?:,connection={.*})?" ] \
+    "add inferior 2"
+
+# Set the executable for inferior 2.
+mi_gdb_test "-file-exec-and-symbols --thread-group i2 $::binfile" \
+    "\\^done" \
+    "set executable of inferior 2"
+
+# Load the core file into inferior 2.
+mi_gdb_test \
+    "-target-select --thread-group i2 core $::corefile" \
+    [multi_line \
+	 "=thread-group-started,id=\"i2\",.*" \
+	 "=thread-created,id=\"2\",group-id=\"i2\"" \
+	 ".*\\^connected,frame=.*"] \
+    "load core file in inferior 2"
+
+# Check the core file thread is initially shown as stopped.
+mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \
+    "core file thread is initially stopped"
+
+# Resume "all" threads.  As the core target doesn't support execution
+# this should not try to set the core target threads running.
+mi_gdb_test "-exec-continue --all" \
+    [multi_line \
+	 "\\^running" \
+	 "\\*running,thread-id=\"1\""] \
+    "resume all"
+
+# Wait for the non-core target thread to stop.
+mi_expect_stop "breakpoint-hit" \
+    "foo" ".*" ".*" ".*" {"" "disp=\"keep\""} "w1,i2 stop"
+
+# Check that the core target thread is still showing as stopped.
+mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \
+    "core file thread is still stopped"
-- 
2.25.4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.