Re: [PATCH] gdb/python: fix 'exited' event when GDB exits from core file debugging

[Lancelot SIX <Lancelot.Six@amd.com>]

Hi Andrew,

Thanks for the quick turnaround!

FYI, when applying the patch, git reports:

```
Applying: gdb/python: fix 'exited' event when GDB exits from core file 
debugging
.git/rebase-apply/patch:127: indent with spaces.
             "python print(str(gdb.selected_inferior()))" ""]
.git/rebase-apply/patch:133: indent with spaces.
            set inferior_string $expect_out(1,string)
.git/rebase-apply/patch:134: indent with spaces.
            incr event_count
.git/rebase-apply/patch:135: indent with spaces.
            exp_continue
.git/rebase-apply/patch:139: indent with spaces.
            verbose -log "GDB has now exited"
warning: squelched 7 whitespace errors
warning: 12 lines add whitespace errors.
```

I have tested the patch on top of our downstream ROCgdb, and this fixes 
the issue.

I have one minor comment below, but otherwise the patch itself looks 
good to me.

On 04/06/2026 17:47, Andrew Burgess wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> This fixes an issue that was reported here:
> 
>    https://inbox.sourceware.org/gdb-patches/v3x4md2dg6rflq35ymzwrmmqf5uaem5exrnlbsp5dmhph2vihy@lq22ncu774yu
> 
> After commit:
> 
>    commit 3780b9993c973a2b68b496b80eddb820c0932cc0
>    Date:   Fri Mar 27 11:29:07 2026 +0000
> 
>      gdb: refactor core_target ::close and ::detach functions
> 
> it was observed that the Python 'exited' event was no longer being
> emitted when debugging a core file, and then exiting GDB.
> 
> The problem is that, when GDB is exiting we eventually end up in
> quit_force (in top.c), which calls kill_or_detach for every inferior.
> 
> In kill_or_detach we call either target_detach or target_kill, but
> only for non-core file targets.  For core file targets, neither of
> these is called and kill_or_detach does nothing of interest.
> 
> After the call to kill_or_detach, we call inferior::pop_all_targets,
> which calls inferior::pop_all_targets_above the dummy_stratum target,
> which means popping all targets.
> 
> In inferior::pop_all_targets_above (in inferior.c), we call
> switch_to_inferior_no_thread, which ensures the correct inferior is
> selected, but makes it so that no thread is selected.  Switching to no
> thread sets inferior_ptid to null_ptid.
> 
> Now popping the core_target calls core_target::close, and within
> core_target::close we currently check inferior_ptid in order to
> determine if exit_core_file_inferior has already been called or not.
> We only call exit_core_file_inferior if inferior_ptid is not
> null_ptid, so in this case we will not call exit_core_file_inferior.
> 
> The only other place that exit_core_file_inferior can be called from
> is core_target::detach, but remember we specifically avoided calling
> target_detach earlier in kill_or_detach.  This means that
> exit_core_file_inferior ends up never being called.
> 
> It is exit_core_file_inferior that calls exit_inferior, and it is from
> here that the Python 'exited' event is emitted.
> 
> I don't see any reason why kill_or_detach couldn't call target_detach
> for a core file target, but I don't propose making that change in this
> commit.
> 
> The check against inferior_ptid in core_target::close is clearly
> incorrect, checking this requires that a suitable thread within the
> inferior be selected, and that is not really a requirement for closing
> a core_target.  Instead, we can just check the inferior::pid field.
> When we open a core_target we always set inferior::pid, even if we
> just assign a fake CORELOW_PID value, so checking inferior::pid
> against zero will tell us if the inferior has already been exited.
> Fixing this check is enough to resolve the reported bug and ensure
> that the 'exited' event is always emitted, which is why I don't
> propose changing kill_or_detach in this commit.
> 
> An assert in core_target::exit_core_file_inferior has to go too for
> the same reason, the assert is checking that a thread is currently
> selected, and as discussed above, this is not always the case.
> 
> There's a new test which checks that the 'exited' event is emitted for
> both a core file debug session, and a live inferior debug session.
> Only the core file case was broken before this commit, but more
> testing is always a good thing.
> ---
>   gdb/corelow.c                                 |  13 +--
>   .../gdb.python/py-inf-exited-at-exit.c        |  32 +++++
>   .../gdb.python/py-inf-exited-at-exit.exp      | 110 ++++++++++++++++++
>   .../gdb.python/py-inf-exited-at-exit.py       |  20 ++++
>   4 files changed, 167 insertions(+), 8 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.python/py-inf-exited-at-exit.c
>   create mode 100644 gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp
>   create mode 100644 gdb/testsuite/gdb.python/py-inf-exited-at-exit.py
> 
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 819e7cae6f9..185b8da90de 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -629,10 +629,6 @@ core_target::build_file_mappings ()
>   void
>   core_target::exit_core_file_inferior ()
>   {
> -  /* Opening a core file ensures that some thread, even if it's just a
> -     "fake" thread, will have been selected.  */
> -  gdb_assert (inferior_ptid != null_ptid);
> -
>     /* Avoid confusion from thread stuff.  */
>     switch_to_no_thread ();
> 
> @@ -665,10 +661,11 @@ core_target::close ()
>        mostly harmless except it causes two 'exited' events to be emitted in
>        the Python API, which isn't ideal.
> 
> -     As opening a core_target always ensures that some thread is selected,
> -     then we can tell if exit_core_file_inferior has already been called by
> -     checking if no thread is now selected.  */
> -  if (inferior_ptid != null_ptid)
> +     As opening a core_target always ensures that a pid is assigned to the
> +     core file inferior, even if it is the fake CORELOW_PID, then we can
> +     tell if exit_core_file_inferior has already been called by checking if
> +     the inferior has a non-zero pid or not.  */
> +  if (current_inferior ()->pid != 0)
>       exit_core_file_inferior ();
> 
>     /* Core targets are heap-allocated (see core_target_open), so here
> diff --git a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c
> new file mode 100644
> index 00000000000..708e3eb98ea
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c
> @@ -0,0 +1,32 @@
> +/* 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
> +foo (void)
> +{
> +  /* With correct ulimit, etc. this should cause a core dump.  */
> +  abort ();
> +}
> +
> +int
> +main (void)
> +{
> +  foo ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp
> new file mode 100644
> index 00000000000..ab415b2c496
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp
> @@ -0,0 +1,110 @@
> +# 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/>.
> +
> +# Check that the 'exited' event triggers when GDB exits.  Test for
> +# both live inferiors, and for core files.
> +
> +require allow_python_tests
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile
> +
> +if {[build_executable "build executable" $testfile $srcfile] == -1} {
> +    return
> +}
> +
> +set remote_python_file \
> +    [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
> +
> +# Load the Python script for this test.  Record the string
> +# representation of the current inferior.  Then exit GDB.  Ensure that
> +# during the exit we see a single Python 'exited' event associated
> +# with the expected inferior.
> +proc source_py_script_and_exit_checking_event {} {
> +    gdb_test_no_output "source $::remote_python_file" \
> +       "load python script"
> +
> +    set expected_inferior_string \
> +       [capture_command_output \
> +            "python print(str(gdb.selected_inferior()))" ""]
> +
> +    set inferior_string ""
> +    set event_count 0
> +    gdb_test_multiple "with confirm off -- exit" "exit gdb" {
> +       -re "^EVENT: inferior exited event\\.  Inferior is (\[^\r\n\]+)\r\n" {
> +           set inferior_string $expect_out(1,string)
> +           incr event_count
> +           exp_continue
> +       }
> +
> +       eof {
> +           verbose -log "GDB has now exited"
> +           gdb_assert { $expected_inferior_string eq $inferior_string \
> +                            && $event_count == 1 } $gdb_test_name
> +       }
> +
> +       -re "^\[^\r\n\]*\r\n" {
> +           exp_continue
> +       }
> +    }
> +}
> +
> +# Create a core file.  Start GDB and load the core file.  Exit GDB.
> +# Check that we see an 'exited' event, and that it is associated with
> +# the correct gdb.Inferior.
> +proc_with_prefix check_with_corefile {} {
> +    set corefile [core_find $::binfile]
> +    if {$corefile eq ""} {
> +       unsupported "couldn't create or find corefile"
> +       return
> +    }

I expect you could have GDB create the coredump instead of relying on 
the kernel for this.  This allows this test to run on systems where 
kernel.core_pattern is not set in a way supported by the testsuite:


----
diff --git a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c 
b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c
index 708e3eb98ea..f999fdbbf00 100644
--- a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c
+++ b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.c
@@ -20,8 +20,6 @@
  void
  foo (void)
  {
-  /* With correct ulimit, etc. this should cause a core dump.  */
-  abort ();
  }

  int
diff --git a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp 
b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp
index f90974dc894..80543bc6f7c 100644
--- a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp
+++ b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.exp
@@ -66,20 +66,27 @@ proc source_py_script_and_exit_checking_event {} {
  # Check that we see an 'exited' event, and that it is associated with
  # the correct gdb.Inferior.
  proc_with_prefix check_with_corefile {} {
-    set corefile [core_find $::binfile]
-    if {$corefile eq ""} {
-       unsupported "couldn't create or find corefile"
+    clean_restart $::testfile
+
+    if {![runto_main]} {
         return
      }

+    gdb_breakpoint "foo"
+    gdb_continue_to_breakpoint "stop in foo"
+
+    set corefile "$::binfile.core"
+    gdb_test "generate-core $corefile" "Saved corefile $corefile" \
+           "generate-core"
+
      clean_restart $::testfile

      gdb_core_cmd $corefile "load corefile"

      gdb_test "bt" \
         [multi_line \
-            "#$::decimal  (?:$::hex in )?foo \\(\\) at \[^\r\n\]+" \
-            "#$::decimal  (?:$::hex in )?main \\(\\) at \[^\r\n\]+"] \
+           "#0  (?:$::hex in )?foo \\(\\) at \[^\r\n\]+" \
+           "#1  (?:$::hex in )?main \\(\\) at \[^\r\n\]+"] \
         "backtrace after loading corefile"

      source_py_script_and_exit_checking_event

----

Best,
Lancelot.

> +
> +    clean_restart $::testfile
> +
> +    gdb_core_cmd $corefile "load corefile"
> +
> +    gdb_test "bt" \
> +       [multi_line \
> +            "#$::decimal  (?:$::hex in )?foo \\(\\) at \[^\r\n\]+" \
> +            "#$::decimal  (?:$::hex in )?main \\(\\) at \[^\r\n\]+"] \
> +       "backtrace after loading corefile"
> +
> +    source_py_script_and_exit_checking_event
> +}
> +
> +# Start a running inferior.  Exit GDB.  Check that we see an 'exited'
> +# event, and that it is associated with the correct gdb.Inferior.
> +proc_with_prefix check_with_live {} {
> +    clean_restart $::testfile
> +
> +    if {![runto_main]} {
> +       return
> +    }
> +
> +    gdb_breakpoint "foo"
> +    gdb_continue_to_breakpoint "stop in foo"
> +
> +    gdb_test "bt" \
> +       [multi_line \
> +            "#0  (?:$::hex in )?foo \\(\\) at \[^\r\n\]+" \
> +            "#1  (?:$::hex in )?main \\(\\) at \[^\r\n\]+"] \
> +       "backtrace at breakpoint"
> +
> +    source_py_script_and_exit_checking_event
> +}
> +
> +check_with_live
> +check_with_corefile
> diff --git a/gdb/testsuite/gdb.python/py-inf-exited-at-exit.py b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.py
> new file mode 100644
> index 00000000000..b6fe39e4061
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-inf-exited-at-exit.py
> @@ -0,0 +1,20 @@
> +# 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/>.
> +
> +def exit_event_handler(event):
> +    inf = event.inferior
> +    print("EVENT: inferior exited event.  Inferior is " + str(inf))
> +
> +gdb.events.exited.connect(exit_event_handler)
> 
> base-commit: bd64797371d27c766d551d0bf115d9090f1d0594
> --
> 2.25.4
>