[PATCH] gdb: pass inferior argument to inf_child_target::maybe_unpush_target

[Andrew Burgess <aburgess@redhat.com>]

Bug PR gdb/29944 highlights an issue where this assertion can trigger:

  thread-iter.c:109: internal-error: all_matching_threads_iterator: Assertion `filter_target != nullptr' failed.

The problem occurs when GDB is configured with these settings:

  set detach-on-fork off
  set schedule-multiple on
  set non-stop on

There is a Python exit event listener registered like this:

  def exit_handler(_):
    gdb.execute('inferior 1')

  gdb.events.exited.connect(exit_handler)

Then the user runs an inferior which forks a child process, the
child (at some point) exits while the parent process continues
running.  Then, at some future time, the parent process also exits.

Initially we only have inferior 1, but when this inferior forks we now
have inferior's 1 and 2.  Due to the settings GDB follows both, and
resumes both inferiors.

On GNU/Linux, when the child, inferior 2, exits, we eventually end up
in inf_child_target::mourn_inferior.  This then calls
generic_mourn_inferior which operates on the current inferior.  This
includes calling exit_inferior, which is where the inferior_exit
observer is notified, and it is this that runs the Python 'exit' event
handlers.

In our case the Python exit handler runs 'inferior 1', which changes
the current inferior.

Back in inf_child_target::mourn_inferior we now call
maybe_unpush_target which potentially unpushes the inf_child_target
from the target stack of the current inferior.

But notice, we already switched the current inferior to inferior 1.
This means that we just unpushed the inf_child_target from inferior 1,
which is still running, not inferior 2, which has exited.

Later, when inferior 1 exits we end up in normal_stop and, because we
are in non-stop mode, we try to find the finish_ptid and finish_target
based on the inferior which just exited.  In this case inferior 1.
But remember, inferior 1 no longer has a process stratum target, we
incorrectly unpushed it earlier when inferior 2 exited.

We now initialise maybe_finish_thread_state, a
scoped_finish_thread_state object, using the finish_ptid, which is the
ptid of inferior 1, and finish_target, which is NULL.

Eventually, later in normal_stop, the scoped_finish_thread_state runs,
which calls finish_thread_state, which calls all_non_exited_threads.

This eventually calls
all_matching_threads_iterator::all_matching_threads_iterator to
iterate over the applicable threads, and, as the filter_ptid is that
of inferior 1 (i.e. not minus_one_ptid), and filter_target is NULL,
the assert triggers.

The solution is simple enough, have
inf_child_target::maybe_unpush_target take the inferior to operate on,
rather than relying on the current_inferior being correct.  It looks
like we might have run into something like this before as in
inf_child_target::follow_exec we temporarily change the current
inferior back prior to calling maybe_unpush_target.  In that case it
is GDB's follow-exec-mode that was causing the problem. See:

  commit 737358ba1ed8b28820cc965f62027bb7417b132b
  Date:   Thu May 13 15:28:42 2021 -0400

      gdb: maybe unpush target from old inferior in inf_child_target::follow_exec

I did consider having inf_child_target::maybe_unpush_target perform
the inferior switch for us, but after looking at what
maybe_unpush_target actually does, I don't think it is really
necessary for the current inferior to be set "correctly", given an
inferior pointer, we can just operate on that.

So that's what I've done.  maybe_unpush_target takes an 'inferior *'
argument, and it is that inferior from which we unpush the target.  It
is no longer necessary to switch inferiors in follow_exec.

There's a test that exposes the original assertion failure, which
passes with this patch.  The gdb.base/foll-exec-mode.exp test, which
was added in commit 737358ba1ed8b288 also still passes after this
change.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29944
---
 gdb/gnu-nat.c                                 |   2 +-
 gdb/go32-nat.c                                |   3 +-
 gdb/inf-child.c                               |  15 +-
 gdb/inf-child.h                               |  13 +-
 gdb/inf-ptrace.c                              |   2 +-
 gdb/procfs.c                                  |   6 +-
 .../py-switch-inferior-in-exit-event.c        |  62 ++++++
 .../py-switch-inferior-in-exit-event.exp      | 195 ++++++++++++++++++
 gdb/windows-nat.c                             |   2 +-
 9 files changed, 277 insertions(+), 23 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.c
 create mode 100644 gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.exp

diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 985aa14debc..39e02f42f40 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2226,7 +2226,7 @@ gnu_nat_target::detach (inferior *inf, int from_tty)
   switch_to_no_thread ();
   detach_inferior (inf);
 
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 \f
 
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index b3ea5c1fd5c..bc377c97dc0 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -773,8 +773,9 @@ go32_nat_target::mourn_inferior ()
 
   prog_has_started = 0;
 
+  inferior *inf = current_inferior ();
   generic_mourn_inferior ();
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 
 /* Hardware watchpoint support.  */
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index a87aa925133..6f2a2020f2a 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -191,17 +191,18 @@ inf_child_target::close ()
 void
 inf_child_target::mourn_inferior ()
 {
+  inferior *inf = current_inferior ();
   generic_mourn_inferior ();
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 
 /* See inf-child.h.  */
 
 void
-inf_child_target::maybe_unpush_target ()
+inf_child_target::maybe_unpush_target (inferior *inf)
 {
   if (!inf_child_explicitly_opened)
-    current_inferior ()->unpush_target (this);
+    inf->unpush_target (this);
 }
 
 bool
@@ -420,13 +421,7 @@ inf_child_target::follow_exec (inferior *follow_inf, ptid_t ptid,
   process_stratum_target::follow_exec (follow_inf, ptid, execd_pathname);
 
   if (orig_inf != follow_inf)
-    {
-      /* If the target was implicitly push in the original inferior, unpush
-	 it.  */
-      scoped_restore_current_thread restore_thread;
-      switch_to_inferior_no_thread (orig_inf);
-      maybe_unpush_target ();
-    }
+    maybe_unpush_target (orig_inf);
 }
 
 /* See inf-child.h.  */
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index 9fa62664b36..4392b119b40 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -95,13 +95,14 @@ class inf_child_target
   bool can_use_agent () override;
 
 protected:
-  /* Unpush the target if it wasn't explicitly open with "target native"
-     and there are no live inferiors left.  Note: if calling this as a
-     result of a mourn or detach, the current inferior shall already
-     have its PID cleared, so it isn't counted as live.  That's usually
-     done by calling either generic_mourn_inferior or
+  /* Unpush the target from inferior INF if it wasn't explicitly open with
+     "target native" and there are no live inferiors left.
+
+     Note: if calling this as a result of a mourn or detach, INF shall
+     already have its PID cleared, so it isn't counted as live.
+     That's usually done by calling either generic_mourn_inferior or
      detach_inferior.  */
-  void maybe_unpush_target ();
+  void maybe_unpush_target (inferior *inf);
 };
 
 /* Convert the host wait(2) status to a target_waitstatus.  */
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 5363717208e..aed0759dddf 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -213,7 +213,7 @@ inf_ptrace_target::detach_success (inferior *inf)
   switch_to_no_thread ();
   detach_inferior (inf);
 
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 
 /* Kill the inferior.  */
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 7472c10616e..718fb295513 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1784,7 +1784,7 @@ procfs_target::detach (inferior *inf, int from_tty)
 
   switch_to_no_thread ();
   detach_inferior (inf);
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 
 static void
@@ -2594,9 +2594,9 @@ procfs_target::mourn_inferior ()
 	destroy_procinfo (pi);
     }
 
+  inferior *inf = current_inferior ();
   generic_mourn_inferior ();
-
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 
 /* When GDB forks to create a runnable inferior process, this function
diff --git a/gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.c b/gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.c
new file mode 100644
index 00000000000..f329e17f8b8
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.c
@@ -0,0 +1,62 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <assert.h>
+
+volatile int global_var = 0;
+
+void
+breakpt ()
+{
+  global_var = 42;	/* Break here.  */
+}
+
+int
+main ()
+{
+  pid_t pid;
+
+  alarm (300);
+
+  /* Create the child process.  */
+  pid = fork ();
+  assert (pid >= 0);
+
+  if (pid == 0)
+    {
+      /* Child process.  */
+      exit (0);
+    }
+  else
+    {
+      /* Parent process.  */
+
+      /* Wait for the child process to finish.  */
+      wait (NULL);
+
+      while (global_var == 0)
+	sleep (1);
+
+      breakpt ();
+    }
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.exp b/gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.exp
new file mode 100644
index 00000000000..2f23a5529c4
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-switch-inferior-in-exit-event.exp
@@ -0,0 +1,195 @@
+# Copyright (C) 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/>.
+
+# Test for PR gdb/29944.  In GDB set 'non-stop on', set
+# 'detach-on-fork off', and set 'schedule-multiple on'.
+#
+# Register a Python event handler for the exit event, when an exit
+# event arrives, switch to inferior 1.
+#
+# Run an inferior which forks, the child process exits, and then
+# parent process exits some time later.
+#
+# The bug was that GDB would unpush the target from inferior 1 instead
+# of inferior 2 (the one that actually exited).  When inferior 1 exits
+# later it has no process stratum target, and this triggers an
+# assertion.
+
+require allow_fork_tests
+require allow_python_tests
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if { [build_executable "build executable" ${testfile} ${srcfile}] } {
+    return
+}
+
+foreach_with_prefix non_stop { on off } {
+    save_vars { GDBFLAGS } {
+	append GDBFLAGS " -ex \"set detach-on-fork off\""
+	append GDBFLAGS " -ex \"set schedule-multiple on\""
+	append GDBFLAGS " -ex \"set non-stop $non_stop\""
+	clean_restart $testfile
+    }
+
+    if {![runto_main]} {
+	return
+    }
+
+    set before_output [capture_command_output "info inferiors" ""]
+
+    gdb_test_multiline "add exited listener" \
+	"python" "" \
+	"handler_ran = False" "" \
+	"def exit_handler(_):" "" \
+	"  global handler_ran" "" \
+	"  handler_ran = True" "" \
+	"  gdb.execute('inferior 1')" "" \
+	"gdb.events.exited.connect(exit_handler)" "" \
+	"end" ""
+
+    set lineno [gdb_get_line_number "/* Break here.  */"]
+    gdb_breakpoint $lineno
+
+    # Initial part of the test.  Resume inferior 1, this will fork a child
+    # process, which immediately exits.  The Python exit event will trigger,
+    # and GDB will switch back to inferior 1.  At this point we land back at
+    # a GDB prompt.
+    set saw_inferior_created false
+    set saw_inferior_exited false
+    set saw_inferior_selected false
+    set saw_thread_selected false
+    gdb_test_multiple "continue" "continue to inferior exit event" {
+	-re "^\\\[New inferior 2\[^\r\n\]*\\\]\r\n" {
+	    set saw_inferior_created true
+	    exp_continue
+	}
+
+	-re "^\\\[Inferior 2 \[^\r\n\]+exited normally\\\]\r\n" {
+	    set saw_inferior_exited true
+	    exp_continue
+	}
+
+	-re "^\\\[Switching to inferior 1\[^\r\n\]*\\\]\r\n" {
+	    set saw_inferior_selected true
+	    exp_continue
+	}
+
+	-re "^\\\[Switching to thread 1\\.1\[^\r\n\]*\\\]\\(running\\)\r\n" {
+	    # In all-stop (non-stop off) mode, this line is possibly a
+	    # little confusing; the inferior is marked as '(running)', but
+	    # by the time the prompt is displayed, the inferior will be
+	    # stopped.  This is a consequence of when the exit observer for
+	    # inferior 2 triggers.  If this is ever changed / fixed then the
+	    # above regexp will need changing, at least for all-stop mode.
+	    set saw_thread_selected true
+	    exp_continue
+	}
+
+	-re "^$gdb_prompt $" {
+	    gdb_assert { $saw_inferior_created && $saw_inferior_exited \
+			     && $saw_inferior_selected && $saw_thread_selected } \
+		$gdb_test_name
+	}
+
+	-re "^\[^\r\n\]*\r\n" {
+	    exp_continue
+	}
+    }
+
+    # Confirm the event handler ran.
+    gdb_test "python print(handler_ran)" "^True"
+
+    if { $non_stop eq "on" } {
+	set resume_cmd "set global_var = 1"
+	set re "\r\n\\*\\s+1\\s+\[^\r\n\]+\\(running\\)"
+    } else {
+	gdb_test_no_output "set global_var = 1"
+	set resume_cmd "continue"
+	set re "\r\n\\*\\s+1\\s+\[^\r\n\]+$hex in \[^\r\n\]+"
+    }
+
+    # This validates that the expected thread is selected.  In non-stop mode
+    # this also checks that the thread is still running.
+    gdb_test "info threads" $re
+
+    # Check that 'info inferiors' output is unchanged.  We are mostly
+    # checking that the 'connection' is unchanged.
+    set midway_output [capture_command_output "info inferiors" ""]
+    gdb_assert {$before_output eq $midway_output} \
+	"info inferiors output is unchanged at midway point"
+
+    # The second part of the test.  Send RESUME_CMD which will allow
+    # inferior 1 to progress to the 'breakpt' function, where a breakpoint
+    # will be hit and the inferior will stop again.
+    set saw_breakpoint_hit false
+    set saw_source_line false
+    set saw_prompt false
+    gdb_test_multiple $resume_cmd "continue to breakpoint" {
+	-re "^Thread $::decimal \[^\r\n\]*hit Breakpoint $::decimal, breakpt \\(\\) at \[^\r\n\]+\r\n" {
+	    set saw_breakpoint_hit true
+	    exp_continue
+	}
+
+	-re "^$lineno\\s+\[^\r\n\]+Break here\\.  \\*/\r\n" {
+	    set saw_source_line true
+
+	    if { $non_stop eq "on" } {
+		gdb_assert { $saw_inferior_created && $saw_inferior_exited \
+				 && $saw_inferior_selected && $saw_thread_selected \
+				 && $saw_breakpoint_hit && $saw_source_line } \
+		    $gdb_test_name
+	    } else {
+		exp_continue
+	    }
+	}
+
+	-re "^$gdb_prompt $" {
+	    set saw_prompt true
+	    # In all-stop (non-stop off) mode we expect to see a prompt
+	    # after hitting the breakpoint.
+	    #
+	    # In non-stop mode, the prompt is displayed immediately after
+	    # sending the RESUME_CMD (above).  Hitting the breakpoint is an
+	    # async background event, which is announced to the console.
+	    # The prompt is not redisplayed afterwards, that's why for
+	    # non-stop mode the test's exit point is after seeing the source
+	    # line.
+	    if { $non_stop eq "off" } {
+		gdb_assert { $saw_breakpoint_hit && $saw_source_line \
+				 && $saw_prompt } \
+		    $gdb_test_name
+	    } else {
+		exp_continue
+	    }
+	}
+
+	-re "^\[^\r\n\]*\r\n" {
+	    exp_continue
+	}
+    }
+
+    # Check GDB is still alive, plus in non-stop mode, get us back to a GDB
+    # prompt.
+    gdb_test "print 1" " = 1"
+
+    # Check that 'info inferiors' output is unchanged.  We are mostly
+    # checking that the 'connection' is unchanged.
+    set after_output [capture_command_output "info inferiors" ""]
+    gdb_assert {$before_output eq $after_output} \
+	"info inferiors output is unchanged at end of test"
+}
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index d506b42fbda..bff670a3652 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2427,7 +2427,7 @@ windows_nat_target::detach (inferior *inf, int from_tty)
 
   windows_process->process_id = 0;
 
-  maybe_unpush_target ();
+  maybe_unpush_target (inf);
 }
 
 /* The pid_to_exec_file target_ops method for this platform.  */

base-commit: 2d7f2dbbd4adadf7c388fa0d8b9ce95d9dfde641
-- 
2.25.4