[PATCH 0/2] Adding a Python event for when GDB exits

[PATCH 0/2] Adding a Python event for when GDB exits

[Andrew Burgess]

Adding a new event for when GDB exits (the second patch).

I ran into some issues with the unrelated inferior exiting event
gdb.ExitedEvent while writing the test for the new GDB exiting event,
which explains the first patch.

Thanks,
Andrew


---

Andrew Burgess (2):
  gdb/python: update events test to handle missing exit_code
  gdb/python: add a new gdb_exiting event

 gdb/NEWS                               |  6 +++
 gdb/doc/python.texi                    | 15 +++++-
 gdb/observable.c                       |  1 +
 gdb/observable.h                       |  4 +-
 gdb/python/py-all-events.def           |  1 +
 gdb/python/py-event-types.def          |  5 ++
 gdb/python/python.c                    | 35 +++++++++++++
 gdb/testsuite/gdb.python/py-events.exp | 69 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-events.py  | 30 ++++++++++-
 gdb/top.c                              |  6 ++-
 10 files changed, 167 insertions(+), 5 deletions(-)

-- 
2.25.4

[PATCH 1/2] gdb/python: update events test to handle missing exit_code

[Andrew Burgess]

The test gdb.python/py-events.exp sets up a handler for the gdb.exited
event.  Unfortunately the handler is slightly broken, it assumes that
the exit_code attribute will always be present.  This is not always
the case.

In a later commit I am going to add more tests to py-events.exp test
script, and in so doing I expose the bug in our handling of gdb.exited
events.

Just to be clear, GDB itself is fine, it is the test that is not
written correctly according to the Python Events API.

So, in this commit I fix the Python code in the test, and extend the
test case to exercise more paths through the Python code.

Additionally, I noticed that the gdb.exited event is used as an
example in the documentation for how to write an event handler.
Unfortunately the same bug that we had in our test was also present in
the example code in the manual.

So I've fixed that too.

After this commit there is no functional change to GDB.
---
 gdb/doc/python.texi                    |  5 ++++-
 gdb/testsuite/gdb.python/py-events.exp | 20 ++++++++++++++++++++
 gdb/testsuite/gdb.python/py-events.py  |  5 ++++-
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d8f682a091c..b123b240980 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3202,7 +3202,10 @@
 @smallexample
 def exit_handler (event):
     print ("event type: exit")
-    print ("exit code: %d" % (event.exit_code))
+    if hasattr (event, 'exit_code'):
+        print ("exit code: %d" % (event.exit_code))
+    else:
+        print ("exit code not available")
 
 gdb.events.exited.connect (exit_handler)
 @end smallexample
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index 753709361f5..310258618c3 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -276,3 +276,23 @@ with_test_prefix "inferior continue exit" {
     gdb_test "continue" "exited with code.*" "continue to exit"
     gdb_test "print \$_foo" "= 2" "check foo after start continue"
 }
+
+# Check that when GDB exits, we see gdb.ExitedEvent objects with no
+# 'exit_code' attribute.
+with_test_prefix "gdb exiting" {
+    set saw_inferior_exit 0
+    gdb_test_multiple "quit" "" {
+	-re "Quit anyway\\? \\(y or n\\) $" {
+	    send_gdb "y\n"
+	    exp_continue
+	}
+	-re "event type: exit\r\nexit code: not-present\r\nexit inf: $decimal\r\nexit pid: $decimal\r\ndir ok: False\r\n" {
+	    incr saw_inferior_exit
+	    exp_continue
+	}
+	eof {
+	    gdb_assert { $saw_inferior_exit == 2 }
+	    pass $gdb_test_name
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 1524267117d..b21f562bfd3 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -45,7 +45,10 @@ def breakpoint_stop_handler(event):
 def exit_handler(event):
     assert isinstance(event, gdb.ExitedEvent)
     print("event type: exit")
-    print("exit code: %d" % (event.exit_code))
+    if hasattr(event, 'exit_code'):
+        print("exit code: %d" % (event.exit_code))
+    else:
+        print("exit code: not-present")
     print("exit inf: %d" % (event.inferior.num))
     print("exit pid: %d" % (event.inferior.pid))
     print("dir ok: %s" % str("exit_code" in dir(event)))
-- 
2.25.4

[PATCH 2/2] gdb/python: add a new gdb_exiting event

[Andrew Burgess]

Add a new event, gdb.events.gdb_exiting, which is called once GDB
decides it is going to exit.

This event is not triggered in the case that GDB performs a hard
abort, for example, when handling an internal error and the user
decides to quit the debug session, or if GDB hits an unexpected,
fatal, signal.

This event is triggered if the user just types 'quit' at the command
prompt, or if GDB is run with '-batch' and has processed all of the
required commands.

The new event type is gdb.GdbExitingEvent, and it has a single
attribute exit_code, which is the value that GDB is about to exit
with.

The event is triggered before GDB starts dismantling any of its own
internal state, so, my expectation is that most Python calls should
work just fine at this point.

When considering this functionality I wondered about using the
'atexit' Python module.  However, this is triggered when the Python
environment is shut down, which is done from a final cleanup.  At
this point we don't know for sure what other GDB state has already
been cleaned up.
---
 gdb/NEWS                               |  6 +++
 gdb/doc/python.texi                    | 10 +++++
 gdb/observable.c                       |  1 +
 gdb/observable.h                       |  4 +-
 gdb/python/py-all-events.def           |  1 +
 gdb/python/py-event-types.def          |  5 +++
 gdb/python/python.c                    | 35 +++++++++++++++++
 gdb/testsuite/gdb.python/py-events.exp | 53 +++++++++++++++++++++++++-
 gdb/testsuite/gdb.python/py-events.py  | 25 ++++++++++++
 gdb/top.c                              |  6 ++-
 10 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index f9485520438..b978b8a7039 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -17,6 +17,12 @@ maint show backtrace-on-fatal-signal
      integer, the index of the new item in the history list, is
      returned.
 
+  ** New gdb.events.gdb_exiting event.  This event is called with a
+     gdb.GdbExitingEvent object which has the read-only attribute
+     'exit_code', which contains the value GDB is about to exit with.
+     This event is triggered once GDB decides it is going to exit, but
+     before GDB starts to clean up its internal state.
+
 *** Changes in GDB 11
 
 * The 'set disassembler-options' command now supports specifying options
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index b123b240980..e65a9a61888 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3418,6 +3418,16 @@
 The new thread.
 @end defvar
 
+@item events.gdb_exiting
+This is emitted when @value{GDBN} exits.  This event is not emitted if
+@value{GDBN} exits as a result of an internal error, or after an
+unexpected signal.  The event is of type @code{gdb.GdbExitingEvent},
+which has a single attribute:
+
+@defvar GdbExitingEvent.exit_code
+An integer, the value with which GDB will exit.
+@end defvar
+
 @end table
 
 @node Threads In Python
diff --git a/gdb/observable.c b/gdb/observable.c
index 51f5edb0a1f..b020076cf26 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -77,6 +77,7 @@ DEFINE_OBSERVABLE (register_changed);
 DEFINE_OBSERVABLE (user_selected_context_changed);
 DEFINE_OBSERVABLE (source_styling_changed);
 DEFINE_OBSERVABLE (current_source_symtab_and_line_changed);
+DEFINE_OBSERVABLE (gdb_exiting);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 915770ff363..f20f532870f 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -248,9 +248,11 @@ extern observable<> source_styling_changed;
 /* The CLI's notion of the current source has changed.  This differs
    from user_selected_context_changed in that it is also set by the
    "list" command.  */
-
 extern observable<> current_source_symtab_and_line_changed;
 
+/* Called when GDB is about to exit.  */
+extern observable<int> gdb_exiting;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/python/py-all-events.def b/gdb/python/py-all-events.def
index d12a2103e80..83f10989e4a 100644
--- a/gdb/python/py-all-events.def
+++ b/gdb/python/py-all-events.def
@@ -38,3 +38,4 @@ GDB_PY_DEFINE_EVENT(breakpoint_created)
 GDB_PY_DEFINE_EVENT(breakpoint_deleted)
 GDB_PY_DEFINE_EVENT(breakpoint_modified)
 GDB_PY_DEFINE_EVENT(before_prompt)
+GDB_PY_DEFINE_EVENT(gdb_exiting)
diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
index 70df4804fc4..aeaee02e8bb 100644
--- a/gdb/python/py-event-types.def
+++ b/gdb/python/py-event-types.def
@@ -105,3 +105,8 @@ GDB_PY_DEFINE_EVENT_TYPE (thread,
 			  "ThreadEvent",
 			  "GDB thread event object",
 			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (gdb_exiting,
+			  "GdbExitingEvent",
+			  "GDB is about to exit",
+			  event_object_type);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 6af9c722e7b..b9dbdb5e26d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -35,6 +35,7 @@
 #include <ctype.h>
 #include "location.h"
 #include "run-on-main-thread.h"
+#include "observable.h"
 
 /* Declared constants and enum for python stack printing.  */
 static const char python_excp_none[] = "none";
@@ -1716,6 +1717,38 @@ init__gdb_module (void)
 }
 #endif
 
+/* Emit a gdb.GdbExitingEvent, return a negative value if there are any
+   errors, otherwise, return 0.  */
+
+static int
+emit_exiting_event (int exit_code)
+{
+  gdbpy_ref<> event_obj = create_event_object (&gdb_exiting_event_object_type);
+  if (event_obj == nullptr)
+    return -1;
+
+  gdbpy_ref<> code = gdb_py_object_from_longest (exit_code);
+  if (evpy_add_attribute (event_obj.get (), "exit_code", code.get ()) < 0)
+    return -1;
+
+  return evpy_emit_event (event_obj.get (), gdb_py_events.gdb_exiting);
+}
+
+/* Callback for the gdb_exiting observable.  EXIT_CODE is the value GDB
+   will exit with.  */
+
+static void
+gdbpy_gdb_exiting (int exit_code)
+{
+  if (!gdb_python_initialized)
+    return;
+
+  gdbpy_enter enter_py (python_gdbarch, python_language);
+
+  if (emit_exiting_event (exit_code) < 0)
+    gdbpy_print_stack ();
+}
+
 static bool
 do_start_initialization ()
 {
@@ -1867,6 +1900,8 @@ do_start_initialization ()
   if (gdbpy_value_cst == NULL)
     return false;
 
+  gdb::observers::gdb_exiting.attach (gdbpy_gdb_exiting, "python");
+
   /* Release the GIL while gdb runs.  */
   PyEval_SaveThread ();
 
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index 310258618c3..fc2383d45bb 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -278,21 +278,70 @@ with_test_prefix "inferior continue exit" {
 }
 
 # Check that when GDB exits, we see gdb.ExitedEvent objects with no
-# 'exit_code' attribute.
-with_test_prefix "gdb exiting" {
+# 'exit_code' attribute, and that a gdb.GdbExitingEvent is emitted.
+with_test_prefix "gdb exiting: normal" {
+    gdb_test "test-exiting-event normal" "GDB exiting event registered\\."
+
+    set saw_exiting_event 0
     set saw_inferior_exit 0
     gdb_test_multiple "quit" "" {
 	-re "Quit anyway\\? \\(y or n\\) $" {
 	    send_gdb "y\n"
 	    exp_continue
 	}
+	-re "event type: gdb-exiting\r\nexit code: $decimal" {
+	    incr saw_exiting_event
+	    exp_continue
+	}
 	-re "event type: exit\r\nexit code: not-present\r\nexit inf: $decimal\r\nexit pid: $decimal\r\ndir ok: False\r\n" {
 	    incr saw_inferior_exit
 	    exp_continue
 	}
 	eof {
+	    gdb_assert { $saw_exiting_event == 1 }
 	    gdb_assert { $saw_inferior_exit == 2 }
 	    pass $gdb_test_name
 	}
     }
 }
+
+# Check that if the GdbExitingEvent raises an exception then this
+# doesn't impact GDB's exit process.
+with_test_prefix "gdb exiting: error" {
+    clean_restart ${testfile}
+
+    if ![runto_main] then {
+	fail "cannot run to main."
+	return 0
+    }
+
+    gdb_test_no_output "source ${pyfile}" "load python file"
+    gdb_test "test-exiting-event error" "GDB exiting event registered\\."
+    gdb_test "test-events" "Event testers registered\\."
+
+    set saw_exiting_error 0
+    set saw_inferior_exit 0
+    gdb_test_multiple "quit" "" {
+	-re "Quit anyway\\? \\(y or n\\) $" {
+	    send_gdb "y\n"
+	    exp_continue
+	}
+	-re "event type: gdb-exiting\r\nexit code: $decimal" {
+	    fail "$gdb_test_name XXXX"
+	    exp_continue
+	}
+	-re "Python Exception <class 'gdb.GdbError'>: error from gdb_exiting_handler\r\n" {
+	    incr saw_exiting_error
+	    exp_continue
+	}
+	-re "event type: exit\r\nexit code: not-present\r\nexit inf: $decimal\r\nexit pid: $decimal\r\ndir ok: False\r\n" {
+	    incr saw_inferior_exit
+	    exp_continue
+	}
+	eof {
+	    gdb_assert { $saw_inferior_exit == 1 }
+	    gdb_assert { $saw_exiting_error == 1 }
+	    pass $gdb_test_name
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index b21f562bfd3..57b0324598f 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -132,3 +132,28 @@ class test_newobj_events(gdb.Command):
 
 
 test_newobj_events()
+
+def gdb_exiting_handler(event, throw_error):
+    assert isinstance(event, gdb.GdbExitingEvent)
+    if throw_error:
+        raise gdb.GdbError("error from gdb_exiting_handler")
+    else:
+        print("event type: gdb-exiting")
+        print("exit code: %d" % (event.exit_code))
+
+class test_exiting_event(gdb.Command):
+    """GDB Exiting event."""
+
+    def __init__(self):
+        gdb.Command.__init__(self, "test-exiting-event", gdb.COMMAND_STACK)
+
+    def invoke(self, arg, from_tty):
+        if arg == "normal":
+            gdb.events.gdb_exiting.connect(lambda e: gdb_exiting_handler(e,False))
+        elif arg == "error":
+            gdb.events.gdb_exiting.connect(lambda e: gdb_exiting_handler(e,True))
+        else:
+            raise gdb.GdbError("invalid or missing argument")
+        print("GDB exiting event registered.")
+
+test_exiting_event()
diff --git a/gdb/top.c b/gdb/top.c
index 0c49f4f9eb4..a8c627df4f9 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1760,8 +1760,6 @@ quit_force (int *exit_arg, int from_tty)
 {
   int exit_code = 0;
 
-  undo_terminal_modifications_before_exit ();
-
   /* An optional expression may be used to cause gdb to terminate with the
      value of that expression.  */
   if (exit_arg)
@@ -1769,6 +1767,10 @@ quit_force (int *exit_arg, int from_tty)
   else if (return_child_result)
     exit_code = return_child_result_value;
 
+  gdb::observers::gdb_exiting.notify (exit_code);
+
+  undo_terminal_modifications_before_exit ();
+
   /* We want to handle any quit errors and exit regardless.  */
 
   /* Get out of tfind mode, and kill or detach all inferiors.  */
-- 
2.25.4

Re: [PATCH 1/2] gdb/python: update events test to handle missing exit_code

[Eli Zaretskii]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Wed,  8 Sep 2021 14:56:39 +0100
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index d8f682a091c..b123b240980 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3202,7 +3202,10 @@
>  @smallexample
>  def exit_handler (event):
>      print ("event type: exit")
> -    print ("exit code: %d" % (event.exit_code))
> +    if hasattr (event, 'exit_code'):
> +        print ("exit code: %d" % (event.exit_code))
> +    else:
> +        print ("exit code not available")
>  
>  gdb.events.exited.connect (exit_handler)
>  @end smallexample

This part is OK, thanks.

Re: [PATCH 2/2] gdb/python: add a new gdb_exiting event

[Eli Zaretskii]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Wed,  8 Sep 2021 14:56:40 +0100
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -17,6 +17,12 @@ maint show backtrace-on-fatal-signal
>       integer, the index of the new item in the history list, is
>       returned.
>  
> +  ** New gdb.events.gdb_exiting event.  This event is called with a
> +     gdb.GdbExitingEvent object which has the read-only attribute
> +     'exit_code', which contains the value GDB is about to exit with.

"the value GDB is about to exit with" sounds too complicated.  How
about "the value of the GDB exit code" instead?

> +@item events.gdb_exiting
> +This is emitted when @value{GDBN} exits.  This event is not emitted if
> +@value{GDBN} exits as a result of an internal error, or after an
> +unexpected signal.

What is the rationale not to emit this event when exiting abnormally?

>                      The event is of type @code{gdb.GdbExitingEvent},
> +which has a single attribute:
> +
> +@defvar GdbExitingEvent.exit_code
> +An integer, the value with which GDB will exit.

I suggest "the value of the exit code @value{GDBN} will return."

Thanks.

Re: [PATCH 1/2] gdb/python: update events test to handle missing exit_code

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The test gdb.python/py-events.exp sets up a handler for the gdb.exited
Andrew> event.  Unfortunately the handler is slightly broken, it assumes that
Andrew> the exit_code attribute will always be present.  This is not always
Andrew> the case.

This patch looks good to me.  Thanks.

Tom

Re: [PATCH 2/2] gdb/python: add a new gdb_exiting event

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Add a new event, gdb.events.gdb_exiting, which is called once GDB
Andrew> decides it is going to exit.
[...]

This patch looks good to me.  Thanks for doing this.

Tom

Re: [PATCH 2/2] gdb/python: add a new gdb_exiting event

[Andrew Burgess]

* Eli Zaretskii <eliz@gnu.org> [2021-09-08 17:10:28 +0300]:

> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Date: Wed,  8 Sep 2021 14:56:40 +0100
> > 
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -17,6 +17,12 @@ maint show backtrace-on-fatal-signal
> >       integer, the index of the new item in the history list, is
> >       returned.
> >  
> > +  ** New gdb.events.gdb_exiting event.  This event is called with a
> > +     gdb.GdbExitingEvent object which has the read-only attribute
> > +     'exit_code', which contains the value GDB is about to exit with.
> 
> "the value GDB is about to exit with" sounds too complicated.  How
> about "the value of the GDB exit code" instead?

Thanks, I'll make that change.

> 
> > +@item events.gdb_exiting
> > +This is emitted when @value{GDBN} exits.  This event is not emitted if
> > +@value{GDBN} exits as a result of an internal error, or after an
> > +unexpected signal.
> 
> What is the rationale not to emit this event when exiting
> abnormally?

Mainly because we don't know why GDB is in the process of crashing,
and it felt like re-entering Python would present too much risk of
triggering recursive errors.

Even if GDB does manage to enter Python OK, we're then going to call
into user code, and the user might start poking at various bits of GDB
state, which may, or may not, then trigger a recursive error.

We also don't do any other GDB cleanup in these cases, for example,
writing out the command line history doesn't happen if GDB crashes.

Given that the conditions I listed should be considered exceptional,
and ideally would never happen, I felt like excluding them was far
simpler, both for me, and for a user making use of the exit event.

> 
> >                      The event is of type @code{gdb.GdbExitingEvent},
> > +which has a single attribute:
> > +
> > +@defvar GdbExitingEvent.exit_code
> > +An integer, the value with which GDB will exit.
> 
> I suggest "the value of the exit code @value{GDBN} will return."

I'll change that too.

Thanks,
Andrew

Re: [PATCH 0/2] Adding a Python event for when GDB exits

[Andrew Burgess]

I've now pushed this series.

Thanks,
Andrew

* Andrew Burgess <andrew.burgess@embecosm.com> [2021-09-08 14:56:38 +0100]:

> Adding a new event for when GDB exits (the second patch).
> 
> I ran into some issues with the unrelated inferior exiting event
> gdb.ExitedEvent while writing the test for the new GDB exiting event,
> which explains the first patch.
> 
> Thanks,
> Andrew
> 
> 
> ---
> 
> Andrew Burgess (2):
>   gdb/python: update events test to handle missing exit_code
>   gdb/python: add a new gdb_exiting event
> 
>  gdb/NEWS                               |  6 +++
>  gdb/doc/python.texi                    | 15 +++++-
>  gdb/observable.c                       |  1 +
>  gdb/observable.h                       |  4 +-
>  gdb/python/py-all-events.def           |  1 +
>  gdb/python/py-event-types.def          |  5 ++
>  gdb/python/python.c                    | 35 +++++++++++++
>  gdb/testsuite/gdb.python/py-events.exp | 69 ++++++++++++++++++++++++++
>  gdb/testsuite/gdb.python/py-events.py  | 30 ++++++++++-
>  gdb/top.c                              |  6 ++-
>  10 files changed, 167 insertions(+), 5 deletions(-)
> 
> -- 
> 2.25.4
>