gdb: disable commit resumed in wait_for_inferior

This patch proposes a fix for PR gdb/33147.  The bug can be reproduced
like this:

  gdb -q -ex 'file /bin/ls' \
         -ex 'run &' \
	 -ex 'add-inferior' \
	 -ex 'infer 2' \
	 -ex 'set sysroot' \
	 -ex 'target remote | gdbserver - ls'

Which will trigger an assertion failure:

  target.c:3760: internal-error: target_stop: Assertion `!proc_target->commit_resumed_state' failed.

The problem is that target_stop is being called for a target when
commit_resumed_state is true, the comment on
process_stratum_target::commit_resumed_state is pretty clear:

  To simplify the implementation of targets, the following methods
  are guaranteed to be called with COMMIT_RESUMED_STATE set to
  false:

     - resume
     - stop
     - wait

So clearly we're breaking a precondition of target_stop.  In this
example there are two target, the native target (inferior 1), and the
remote target (inferior 2).  It is the first, the native target, for
which commit_resumed_state is set incorrectly.

At the point target_stop is called looks like this:

  bminor#11 0x00000000009a3c19 in target_stop (ptid=...) at ../../src/gdb/target.c:3760
  bminor#12 target_stop (ptid=...) at ../../src/gdb/target.c:3756
  bminor#13 0x00000000007042f2 in stop_all_threads (reason=<optimized out>, inf=<optimized out>) at ../../src/gdb/infrun.c:5739
  bminor#14 0x0000000000711d3a in wait_for_inferior (inf=0x2b90fd0) at ../../src/gdb/infrun.c:4412
  bminor#15 start_remote (from_tty=from_tty@entry=1) at ../../src/gdb/infrun.c:3829
  bminor#16 0x0000000000897014 in remote_target::start_remote_1 (this=this@entry=0x2c4a520, from_tty=from_tty@entry=1, extended_p=extended_p@entry=0) at ../../src/gdb/remote.c:5350
  bminor#17 0x00000000008976e7 in remote_target::start_remote (extended_p=0, from_tty=1, this=0x2c4a520) at ../../src/gdb/remote.c:5441
  #18 remote_target::open_1 (name=<optimized out>, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:6312
  #19 0x00000000009a815f in open_target (args=0x7fffffffa93c "| gdbserver - ls", from_tty=1, command=<optimized out>) at ../../src/gdb/target.c:838

For new inferiors commit_resumed_state starts set to false, for this
reason, if we only start a remote inferior, then when
wait_for_inferior is called commit_resumed_state will be false, and
everything will work.

Further, as target_stop is only called for running threads, if, when
the remote inferior is started, all other threads (in other targets)
are already stopped, then GDB will never need to call target_stop for
the other targets, and so GDB will not notice that
commit_resumed_state for those target is set to true.

In this case though, as the first (native) inferior is left running in
the background while the remote inferior is created, and because GDB
is running in all-stop mode (so needs to stop all threads in all
targets), then GDB does call target_stop for the other targets, and so
spots that commit_resumed_state is not set correctly and asserts.

The fix is to add scoped_disable_commit_resumed somewhere in the call
stack.  Initially I planned to add the scoped_disable_commit_resumed
in `wait_for_inferior`, however, this isn't good enough.  This
location would solve the problem as described in the bug, but when
writing the test I extended the problem to also cover non-stop mode,
and this runs into a second problem, the same assertion, but triggered
from a different call path.  For this new case the stack looks like
this:

  #1  0x0000000000fb0e50 in target_stop (ptid=...) at ../../src/gdb/target.c:3771
  #2  0x0000000000a7f0ae in stop_all_threads (reason=0x1d0ff74 "remote connect in all-stop", inf=0x0) at ../../src/gdb/infrun.c:5756
  #3  0x0000000000d9c028 in remote_target::process_initial_stop_replies (this=0x3e10670, from_tty=1) at ../../src/gdb/remote.c:5017
  #4  0x0000000000d9cdf0 in remote_target::start_remote_1 (this=0x3e10670, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:5405
  bminor#5  0x0000000000d9d0d4 in remote_target::start_remote (this=0x3e10670, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:5457
  bminor#6  0x0000000000d9e8ac in remote_target::open_1 (name=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1, extended_p=0) at ../../src/gdb/remote.c:6329
  bminor#7  0x0000000000d9d167 in remote_target::open (name=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1) at ../../src/gdb/remote.c:5479
  bminor#8  0x0000000000f9914d in open_target (args=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1, command=0x35d1a40) at ../../src/gdb/target.c:838

So I'm now thinking that stop_all_threads would be the best place for
the scoped_disable_commit_resumed.  I did leave an assert in
wait_for_inferior as, having thought about the assert some, I do still
think the logic of it is true, and it doesn't hurt to leave it in
place I think.

However, it's not quite that simple, the test throws up yet another
bug when we 'maint set target-non-stop on', but then 'set non-stop
off'.  This bug leaves a stopped thread marked as "(running)" in the
'info threads' output.  I have a fix for this issue, but I'm leaving
that for the next commit.  For now I've just disabled part of the test
in the problem case.

I've also tagged this patch with PR gdb/27322.  That bug was created
before the above assert was added, but if you follow the steps to
reproduce for that bug today you will hit the above assert.  The
actual issue described in PR gdb/27322 is fixed in the next patch.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27322
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33147

Author: T-J-Teru

gdb/infrun.c

@@ -4393,6 +4393,11 @@ wait_for_inferior (inferior *inf)
   scoped_finish_thread_state finish_state
     (inf->process_target (), minus_one_ptid);
 
+  /* The commit_resumed_state of INF should already be false at this point
+     as INF will be a newly started remote target.  This might not be true
+     for other targets but this will be handled in stop_all_threads.  */
+  gdb_assert (!inf->process_target ()->commit_resumed_state);
+
   while (1)
     {
       execution_control_state ecs;
@@ -5699,6 +5704,8 @@ stop_all_threads (const char *reason, inferior *inf)
 	debug_prefixed_printf ("infrun", "stop_all_threads", "done");
     };
 
+  scoped_disable_commit_resumed disable_commit_resumed ("stop all threads");
+
   /* Request threads to stop, and then wait for the stops.  Because
      threads we already know about can spawn more threads while we're
      trying to stop them, and we only learn about new threads when we

gdb/testsuite/gdb.multi/remote-with-running-inferior.c

@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+#include <unistd.h>
+
+int global_var = 123;
+
+void
+breakpt (void)
+{
+  /* Nothing.  */
+}
+
+int
+main (void)
+{
+  for (int i = 0; i < 30; ++i)
+    {
+      sleep (1);
+      breakpt ();
+    }
+
+  return 0;
+}

gdb/testsuite/gdb.multi/remote-with-running-inferior.exp

@@ -0,0 +1,180 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2025 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/>.
+
+# Set an inferior running in the background (using "run&"), then
+# connect to gdbserver in a second inferior.  When this test was added
+# there were two bugs in GDB.  First, just connecting to gdbserver
+# while an inferior was running on a different connection type
+# (e.g. inferior 1 was native, and inferior 2 was gdbserver) would
+# trigger an assertion.
+#
+# Then, once the assertion was fixed, GDB would stop the thread from
+# the first inferior, but would fail to update it's state in GDB core,
+# this would leave the thread stopped, but GDB core thinking the
+# thread was running.  Check this is fixed by looking at the 'info
+# threads' output after connecting to the remote target.
+
+# This tests the use of native and remote targets, and also depends on use
+# of the 'run' command.  If we try to run it with a board that forces native
+# targets to become remote, then this test isn't going to work, especially
+# for 'remote' targets where 'run' is not supported.
+require {string equal [target_info gdb_protocol] ""}
+
+load_lib gdbserver-support.exp
+
+require allow_gdbserver_tests
+
+standard_testfile
+
+if { [build_executable "failed to build" $testfile $srcfile] } {
+    return
+}
+
+# Set non-stop mode based on NON_STOP.  Start a native inferior running in
+# the background, then start a second, remote inferior.  Based on the value
+# of NON_STOP we might expect the inferior thread to have been stopped.
+# Confirm inferior one is in the correct state, and that it can be
+# interrupted and/or resumed.
+proc run_test { target_non_stop non_stop } {
+    clean_restart $::testfile
+
+    # Setup non-stop settings.
+    gdb_test_no_output "maint set target-non-stop $target_non_stop"
+    gdb_test_no_output "set non-stop $non_stop"
+
+    # Start the first inferior running in the background.
+    gdb_test -no-prompt-anchor "run&" "Starting program: .*" "start background inferior"
+
+    # Add a second inferior.
+    gdb_test "add-inferior" "Added inferior 2.*"
+    gdb_test "inferior 2" "Switching to inferior 2.*"
+
+    # Setup the sysroot if possible.  This will make connecting to
+    # gdbserver quicker.
+    if { ![is_remote host] && ![is_remote target] } {
+	gdb_test "set sysroot"
+    }
+
+    # Setup, and connect to, a remote target.
+    set target_exec [gdbserver_download_current_prog]
+    set res [gdbserver_start "" $target_exec]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+    set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
+    gdb_assert {$res == 0} "connect to remote target"
+
+    # FIXME: A bug in GDB means that the state of thread 1.1 will be wrong,
+    # GDB's frontend thinks that the thread is running when really the
+    # thread is stopped.  This will be fixed in the next commit, at which
+    # point this whole 'if' will be removed.
+    if { $target_non_stop == "on" && $non_stop == "off" } {
+	gdb_test "p 1 + 2" " = 3" "check GDB is still alive"
+	return
+    }
+
+    # Check the info threads output.  We're checking that we see the two
+    # threads we expect, that the correct thread (inferior two's thread)
+    # is current, and that none of the threads are running.
+    set state_inferior_1 ""
+    set state_inferior_2 ""
+    gdb_test_multiple "info threads" "" {
+	-re "^info threads\r\n" {
+	    exp_continue
+	}
+
+	-re "^\\s+Id\\s+Target Id\\s+Frame\\s*\r\n" {
+	    exp_continue
+	}
+
+	-re "^\\s+1\\.1\\s+\[^\r\n\]+\\(running\\)\r\n" {
+	    set state_inferior_1 "running"
+	    exp_continue
+	}
+
+	-re "^\\*\\s+2\\.1\\s+\[^\r\n\]+\\(running\\)\r\n" {
+	    set state_inferior_2 "running"
+	    exp_continue
+	}
+
+	-re "^\\s+1\\.1\\s+\[^\r\n\]+\r\n" {
+	    set state_inferior_1 "stopped"
+	    exp_continue
+	}
+
+	-re "^\\*\\s+2\\.1\\s+\[^\r\n\]+\r\n" {
+	    set state_inferior_2 "stopped"
+	    exp_continue
+	}
+
+	-re "^$::gdb_prompt $" {
+	    if { $non_stop } {
+		gdb_assert { $state_inferior_1 == "running" \
+				 && $state_inferior_2 == "stopped" } \
+		    $gdb_test_name
+	    } else {
+		gdb_assert { $state_inferior_1 == "stopped" \
+				 && $state_inferior_2 == "stopped" } \
+		    $gdb_test_name
+	    }
+	}
+    }
+
+    # Allow inferior 2 to reach main.  The confirms that inferior 2 can be
+    # set running again.
+    gdb_breakpoint main
+    gdb_continue_to_breakpoint "breakpoint in main"
+    gdb_test "bt 1" \
+	"#0\\s+main \\(\\) at\[^\r\n\]+" \
+	"check inferior 2 is in main"
+
+    # Switch to inferior 1 and allow it to continue.  This is a
+    # critical part of the test.  When the test was added a bug (in
+    # all-stop mode) would leave inferior 1 stopped, but GDB code
+    # would think the thread was running.  As such. the thread
+    # couldn't be resumed again.
+    gdb_test "inferior 1" "Switching to inferior 1.*"
+
+    # In non-stop mode, thread 1.1 is correctly left running, so we
+    # need to stop it now.
+    if { $non_stop } {
+	gdb_test -no-prompt-anchor "interrupt"
+	gdb_test "p 1 + 1" " = 2" \
+	    "simple print to resync output"
+    }
+
+    gdb_breakpoint breakpt
+    gdb_continue_to_breakpoint "continue to breakpoint in breakpt"
+    gdb_test "bt 1" \
+	[multi_line \
+	     "#0\\s+breakpt \\(\\) at\[^\r\n\]+" \
+	     "\\(More stack frames follow\\.\\.\\.\\)"] \
+	"check inferior 1 is in breakpt"
+
+    # Switch back to inferior 2.  The testing infrastructure will try to
+    # use 'monitor exit' to close gdbserver.  It helps if we are in the
+    # gdbserver inferior when the script finishes.
+    gdb_test "inferior 2" "Switching to inferior 2.*" \
+	"switch back to inferior 2"
+}
+
+# Multi-inferior support requires non-stop targets.
+foreach_with_prefix target_non_stop { auto on } {
+    # But it's OK if we're emulating all-stop mode on top of non-stop.
+    foreach_with_prefix non_stop { on off } {
+	run_test $target_non_stop $non_stop
+    }
+}