[PATCH] gdb: add Python events for program space addition and removal

[PATCH] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Initially I just wanted a Python event for when GDB removes a program
space, I'm writing a Python extension that caches information for each
program space, and need to know when I should discard entries for a
particular program space.

But, it seemed easy enough to also add an event for when GDB adds a
new program space, so I went ahead and added both new events.

Of course, we don't currently have an observable for program space
addition or removal, so I first needed to add these.  After that it's
pretty simple to add two new Python events and have these trigger.

The two new event registries are:

  events.new_progspace
  events.free_progspace

These emit NewProgspaceEvent and FreeProgspaceEvent objects
respectively, each of these new event types has a 'progspace'
attribute that contains the relevant gdb.Progspace object.

There's a couple of things to be mindful of.

First, it is not possible to catch the NewProgspaceEvent for the very
first program space, the one that is created when GDB first starts, as
this program space is created before any Python scripts are sourced.

In order to allow this event to be caught we would need to defer
creating the first program space, and as a consequence the first
inferior, until some later time.  But, existing scripts could easily
depend on there being an initial inferior, so I really don't think we
should change that -- and so, we end up with the consequence that we
can't catch the event for the first program space.

The second, I think minor, issue, is that GDB doesn't clean up its
program spaces upon exit -- or at least, they are not cleaned up
before Python is shut down.  As a result, any program spaces in use at
the time GDB exits don't generate a FreeProgspaceEvent.  I'm not
particularly worried about this for my use case, I'm using the event
to ensure that a cache doesn't hold stale entries within a single GDB
session.  It's also easy enough to add a Python at-exit callback which
can do any final cleanup if needed.

Finally, when testing, I did hit a slightly weird issue with some of
the remote boards (e.g. remote-stdio-gdbserver).  As a consequence of
this issue I see some output like this in the gdb.log:

  (gdb) PASS: gdb.python/py-progspace-events.exp: inferior 1
  step
  FreeProgspaceEvent: <gdb.Progspace object at 0x7fb7e1d19c10>
  warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  warning: cannot close "target:/lib64/libc.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  do_parent_stuff () at py-progspace-events.c:41
  41        ++global_var;
  (gdb) PASS: gdb.python/py-progspace-events.exp: step

The 'FreeProgspaceEvent ...' line is expected, that's my test Python
extension logging the event.  What isn't expected are all the blocks
like:

  warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.

It turns out that this has nothing to do with my changes, this is just
a consequence of reading files over the remote protocol.  The test
forks a child process which GDB stays attached too.  When the child
exits, GDB cleans up by calling prune_inferiors, which in turn can
result in GDB trying to close some files that are open because of the
inferior being deleted.

If the prune_inferiors call occurs when the remote target is
running (and in non-async mode) then GDB will try to send a fileio
packet while the remote target is waiting for a stop reply, and the
remote target will throw an error, see remote_target::putpkt_binary in
remote.c for details.

I'm going to look at fixing this, but, as I said, this is nothing to
do with this change, I just mention it because I ended up needing to
account for these warning messages in one of my tests, and it all
looks a bit weird.
---
 gdb/NEWS                                      |   7 ++
 gdb/doc/python.texi                           |  28 +++++
 gdb/observable.c                              |   2 +
 gdb/observable.h                              |   7 ++
 gdb/progspace.c                               |   3 +
 gdb/python/py-all-events.def                  |   2 +
 gdb/python/py-event-types.def                 |  10 ++
 gdb/python/py-progspace.c                     |  67 +++++++++++
 .../gdb.python/py-progspace-events.c          |  73 ++++++++++++
 .../gdb.python/py-progspace-events.exp        | 107 ++++++++++++++++++
 .../gdb.python/py-progspace-events.py         |  29 +++++
 11 files changed, 335 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.c
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.exp
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.py

diff --git a/gdb/NEWS b/gdb/NEWS
index 98ff00d5efc..0c64e9fea6f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -286,6 +286,13 @@ info main
      might be array- or string-like, even if they do not have the
      corresponding type code.
 
+  ** New event registries gdb.events.new_progspace and
+     gdb.events.free_progspace, these emit NewProgspaceEvent and
+     FreeProgspaceEvent event types respectively.  Both of these event
+     types have a single 'progspace' attribute, which is the
+     gdb.Progspace that is either being added to GDB, or removed from
+     GDB.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5b13958aeaf..6d91969e0e0 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3893,6 +3893,34 @@
 The @code{gdb.TargetConnection} that is being removed.
 @end defvar
 
+@item events.new_progspace
+This is emitted when @value{GDBN} adds a new program space
+(@pxref{Progspaces In Python,,Program Spaces In Python}).  The event
+is of type @code{gdb.NewProgspaceEvent}, and has a single read-only
+attribute:
+
+@defvar NewProgspaceEvent.progspace
+The @code{gdb.Progspace} that was added to @value{GDBN}.
+@end defvar
+
+No @code{NewProgspaceEvent} is emitted for the very first program
+space, which is assigned to the first inferior.  This first program
+space is created within @value{GDBN} before and Python scripts are
+sourced.
+
+@item events.free_progspace
+This is emitted when @value{GDBN} removes a program space
+(@pxref{Progspaces In Python,,Program Spaces In Python}), for example
+as a result of the @kbd{remove-inferiors} command
+(@pxref{remove_inferiors_cli,,@kbd{remove-inferiors}}).  The event is
+of type @code{gdb.FreeProgspaceEvent}, and has a single read-only
+attribute:
+
+@defvar FreeProgspaceEvent.progspace
+The @code{gdb.Progspace} that is about to be removed from
+@value{GDBN}.
+@end defvar
+
 @end table
 
 @node Threads In Python
diff --git a/gdb/observable.c b/gdb/observable.c
index 33e51223cf4..93a842e911f 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -72,6 +72,8 @@ DEFINE_OBSERVABLE (gdb_exiting);
 DEFINE_OBSERVABLE (connection_removed);
 DEFINE_OBSERVABLE (target_pre_wait);
 DEFINE_OBSERVABLE (target_post_wait);
+DEFINE_OBSERVABLE (new_program_space);
+DEFINE_OBSERVABLE (free_program_space);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 4ea203c6fc3..84e9b401e95 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -31,6 +31,7 @@ struct inferior;
 struct process_stratum_target;
 struct target_ops;
 struct trace_state_variable;
+struct program_space;
 
 namespace gdb
 {
@@ -233,6 +234,12 @@ extern observable <ptid_t /* ptid */> target_pre_wait;
 /* About to leave target_wait (). */
 extern observable <ptid_t /* event_ptid */> target_post_wait;
 
+/* New program space PSPACE was created.  */
+extern observable <program_space */* pspace */> new_program_space;
+
+/* The program space PSPACE is about to be deleted.  */
+extern observable <program_space */* pspace */> free_program_space;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 5cf8334ee67..1dbcd5875dd 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -28,6 +28,7 @@
 #include "inferior.h"
 #include <algorithm>
 #include "cli/cli-style.h"
+#include "observable.h"
 
 /* The last program space number assigned.  */
 static int last_program_space_num = 0;
@@ -98,6 +99,7 @@ program_space::program_space (address_space *aspace_)
     aspace (aspace_)
 {
   program_spaces.push_back (this);
+  gdb::observers::new_program_space.notify (this);
 }
 
 /* See progspace.h.  */
@@ -106,6 +108,7 @@ program_space::~program_space ()
 {
   gdb_assert (this != current_program_space);
 
+  gdb::observers::free_program_space.notify (this);
   remove_program_space (this);
 
   scoped_restore_current_program_space restore_pspace;
diff --git a/gdb/python/py-all-events.def b/gdb/python/py-all-events.def
index aa28f2c6f8b..ce0249f8fed 100644
--- a/gdb/python/py-all-events.def
+++ b/gdb/python/py-all-events.def
@@ -42,3 +42,5 @@ GDB_PY_DEFINE_EVENT(breakpoint_modified)
 GDB_PY_DEFINE_EVENT(before_prompt)
 GDB_PY_DEFINE_EVENT(gdb_exiting)
 GDB_PY_DEFINE_EVENT(connection_removed)
+GDB_PY_DEFINE_EVENT(new_progspace)
+GDB_PY_DEFINE_EVENT(free_progspace)
diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
index 395d6c07447..450132e2a2e 100644
--- a/gdb/python/py-event-types.def
+++ b/gdb/python/py-event-types.def
@@ -125,3 +125,13 @@ GDB_PY_DEFINE_EVENT_TYPE (connection,
 			  "ConnectionEvent",
 			  "GDB connection added or removed object",
 			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (new_progspace,
+			  "NewProgspaceEvent",
+			  "GDB new Progspace event object",
+			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (free_progspace,
+			  "FreeProgspaceEvent",
+			  "GDB free Progspace event object",
+			  event_object_type);
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index b98ac8dde61..18328e33fb6 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -26,6 +26,8 @@
 #include "arch-utils.h"
 #include "solib.h"
 #include "block.h"
+#include "observable.h"
+#include "py-event.h"
 
 struct pspace_object
 {
@@ -553,9 +555,74 @@ gdbpy_is_progspace (PyObject *obj)
   return PyObject_TypeCheck (obj, &pspace_object_type);
 }
 
+/* Helper function to emit NewProgspaceEvent (when ADDING_P is true) or
+   FreeProgspaceEvent events (when ADDING_P is false).  */
+
+static void
+gdbpy_program_space_event (program_space *pspace, bool adding_p)
+{
+  if (!gdb_python_initialized)
+    return;
+
+  gdbpy_enter enter_py;
+
+  eventregistry_object *registry;
+  PyTypeObject *event_type;
+  if (adding_p)
+    {
+      registry = gdb_py_events.new_progspace;
+      event_type = &new_progspace_event_object_type;
+    }
+  else
+    {
+      registry = gdb_py_events.free_progspace;
+      event_type = &free_progspace_event_object_type;
+    }
+
+  if (evregpy_no_listeners_p (registry))
+    return;
+
+  gdbpy_ref<> pspace_obj = pspace_to_pspace_object (pspace);
+  if (pspace_obj == nullptr)
+    {
+      gdbpy_print_stack ();
+      return;
+    }
+
+  gdbpy_ref<> event = create_event_object (event_type);
+  if (event == nullptr
+      || evpy_add_attribute (event.get (), "progspace",
+			     pspace_obj.get ()) < 0
+      || evpy_emit_event (event.get (), registry) < 0)
+    gdbpy_print_stack ();
+}
+
+
+/* Emit a NewProgspaceEvent to indicate PSPACE has been created.  */
+
+static void
+gdbpy_new_program_space_event (program_space *pspace)
+{
+  gdbpy_program_space_event (pspace, true);
+}
+
+/* Emit a FreeProgspaceEvent to indicate PSPACE is just about to be removed
+   from GDB.  */
+
+static void
+gdbpy_free_program_space_event (program_space *pspace)
+{
+  gdbpy_program_space_event (pspace, false);
+}
+
 static int CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
 gdbpy_initialize_pspace (void)
 {
+  gdb::observers::new_program_space.attach (gdbpy_new_program_space_event,
+					    "py-progspace");
+  gdb::observers::free_program_space.attach (gdbpy_free_program_space_event,
+					     "py-progspace");
+
   if (PyType_Ready (&pspace_object_type) < 0)
     return -1;
 
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.c b/gdb/testsuite/gdb.python/py-progspace-events.c
new file mode 100644
index 00000000000..6684cf8ac0a
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.c
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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>
+#include <sys/types.h>
+#include <assert.h>
+#include <sys/wait.h>
+
+/* This gives the inferior something to do.  */
+volatile int global_var = 0;
+
+void
+breakpt ()
+{ /* Nothing.  */ }
+
+void
+do_child_stuff ()
+{
+  breakpt ();
+  ++global_var;
+}
+
+void
+do_parent_stuff ()
+{
+  breakpt ();
+  ++global_var;
+}
+
+void
+create_child ()
+{
+  int stat;
+  pid_t wpid;
+  breakpt ();
+  pid_t pid = fork ();
+  assert (pid != -1);
+
+  if (pid == 0)
+    {
+      /* Child.  */
+      do_child_stuff ();
+      return;
+    }
+
+  /* Parent.  */
+  do_parent_stuff ();
+  wpid = waitpid (pid, &stat, 0);
+  assert (wpid == pid);
+}
+
+
+
+int
+main ()
+{
+  create_child ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.exp b/gdb/testsuite/gdb.python/py-progspace-events.exp
new file mode 100644
index 00000000000..9ff48d0561a
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.exp
@@ -0,0 +1,107 @@
+# Copyright (C) 2023 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/>.
+
+# This file is part of the GDB testsuite.  It tests the program space
+# related events in the Python API.
+
+load_lib gdb-python.exp
+
+require allow_python_tests
+
+standard_testfile
+
+if {[prepare_for_testing "preparing" $testfile $srcfile] == -1} {
+    return -1
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/py-progspace-events.py]
+gdb_test_no_output "source ${pyfile}" "load python file"
+
+if {![runto_main]} {
+    return
+}
+
+gdb_breakpoint breakpt
+
+gdb_continue_to_breakpoint "run to breakpt function"
+
+gdb_test_no_output "set detach-on-fork off"
+
+gdb_test "continue" \
+    [multi_line \
+	 "^Continuing\\." \
+	 "\\\[New inferior $decimal \[^\r\n\]+\\\]" \
+	 "NewProgspaceEvent: <gdb.Progspace object at $hex>(?:\r\nReading \[^\r\n\]+ from remote target\\.\\.\\.)*" \
+	 "" \
+	 "Thread \[^\r\n\]+, breakpt \\(\\) at \[^\r\n\]+" \
+	 "$decimal\\s+\[^\r\n\]+"] \
+    "continue until child process appears"
+
+# Switch to inferior 2 and continue until we hit breakpt.
+gdb_test "inferior 2" "\\\[Switching to inferior 2 .*"
+gdb_continue_to_breakpoint "run to breakpt in inferior 2"
+
+# Let inferior 2 exit.  The new program space is not removed at this
+# point.
+gdb_test "continue" \
+    [multi_line \
+	 "^Continuing\\." \
+	 "\\\[Inferior $decimal \[^\r\n\]+ exited normally\\\]"] \
+    "continue until inferior 2 exits"
+
+gdb_test "inferior 1" "\\\[Switching to inferior 1 .*"
+
+# Step the inferior.  During this process GDB will prune the now
+# defunct inferior, which deletes its program space, which should
+# trigger the FreeProgspaceEvent.
+#
+# However, there is a slight problem.  When the target is remote, and
+# GDB is accessing files using remote fileio, then GDB will attempt to
+# prune the inferior at a point in time when the remote target is
+# waiting for a stop reply.  Pruning an inferior causes GDB to close
+# files associated with that inferior.
+#
+# In non-async mode we can't send fileio packets while waiting for a
+# stop reply, so the attempts to close files fails, and this shows up
+# as an error.
+#
+# As this error has nothing to do with the feature being tested here,
+# we just accept the error message, the important part is the
+# 'FreeProgspaceEvent' string, so long as that appears (just once)
+# then the test is a success.
+set warning_msg \
+    [multi_line \
+	 "warning: cannot close \"\[^\r\n\]+\": Cannot execute this command while the target is running\\." \
+	 "Use the \"interrupt\" command to stop the target" \
+	 "and then try again\\."]
+
+gdb_test "step" \
+    [multi_line \
+	 "^FreeProgspaceEvent: <gdb.Progspace object at $hex>(?:\r\n$warning_msg)*" \
+	 "do_parent_stuff \\(\\) at \[^\r\n\]+" \
+	 "$decimal\\s+\[^\r\n\]+"]
+
+# Let this inferior run to completion.
+gdb_continue_to_end
+
+# Check the program space events trigger when a new inferior is
+# manually added and removed.
+gdb_test "add-inferior" \
+    [multi_line \
+	 "^NewProgspaceEvent: <gdb.Progspace object at $hex>" \
+	 "\\\[New inferior 3\\\]" \
+	 "Added inferior 3\[^\r\n\]*"]
+gdb_test "remove-inferior 3" \
+    "^FreeProgspaceEvent: <gdb.Progspace object at $hex>"
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.py b/gdb/testsuite/gdb.python/py-progspace-events.py
new file mode 100644
index 00000000000..3abb421cffa
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.py
@@ -0,0 +1,29 @@
+# Copyright (C) 2023 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/>.
+
+# This file is part of the GDB testsuite, it registers listeners for
+# the Progspace related events.
+
+import gdb
+import gdb.events
+
+def new_progspace(event):
+    print("NewProgspaceEvent: %s" % str(event.progspace))
+
+def free_progspace(event):
+    print("FreeProgspaceEvent: %s" % str(event.progspace))
+
+gdb.events.new_progspace.connect(new_progspace)
+gdb.events.free_progspace.connect(free_progspace)

base-commit: cf2ab5ef0b716dea512d85276c484fce758fa5d4
-- 
2.25.4

Re: [PATCH] gdb: add Python events for program space addition and removal

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Initially I just wanted a Python event for when GDB removes a program
Andrew> space, I'm writing a Python extension that caches information for each
Andrew> program space, and need to know when I should discard entries for a
Andrew> particular program space.

Andrew> But, it seemed easy enough to also add an event for when GDB adds a
Andrew> new program space, so I went ahead and added both new events.

This approach seems totally fine to me, but it's worth noting that you
could attach data directly to a Progspace if the tp_hash field were
filled in.  Some other Python classes in gdb do this.  This makes caches
easier because the cleanup is automatic.

Andrew> +No @code{NewProgspaceEvent} is emitted for the very first program
Andrew> +space, which is assigned to the first inferior.  This first program
Andrew> +space is created within @value{GDBN} before and Python scripts are

Typo, "before any"

Other than that it looks good.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Initially I just wanted a Python event for when GDB removes a program
> Andrew> space, I'm writing a Python extension that caches information for each
> Andrew> program space, and need to know when I should discard entries for a
> Andrew> particular program space.
>
> Andrew> But, it seemed easy enough to also add an event for when GDB adds a
> Andrew> new program space, so I went ahead and added both new events.
>
> This approach seems totally fine to me, but it's worth noting that you
> could attach data directly to a Progspace if the tp_hash field were
> filled in.  Some other Python classes in gdb do this.  This makes caches
> easier because the cleanup is automatic.

I'm not sure I understand how tp_hash helps here?

gdb.Progspace does have a __dict__ attribute, so things like:

  progspace.my_random_var = ....

works just fine, and I did consider this.  But I wasn't sure if this was
"good practice".  My concern was mostly that this might lead to clashes
between different extensions if good names are not chosen for
'my_random_var', or even clashes between extensions and future GDB
enhancements -- my understanding is that if we later added
'my_random_var' as a builtin attribute this would take precedence over
the user defined attribute, breaking their extension.

>
> Andrew> +No @code{NewProgspaceEvent} is emitted for the very first program
> Andrew> +space, which is assigned to the first inferior.  This first program
> Andrew> +space is created within @value{GDBN} before and Python scripts are
>
> Typo, "before any"

Fixed, thanks.

>
> Other than that it looks good.

I'll likely merge this anyway, this doesn't stop folk making use of the
__dict__ approach if they prefer -- as you point out, it does have a
significant benefit in that it automates the cleanup.

>
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks,
Andrew

Re: [PATCH] gdb: add Python events for program space addition and removal

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

>> This approach seems totally fine to me, but it's worth noting that you
>> could attach data directly to a Progspace if the tp_hash field were
>> filled in.  Some other Python classes in gdb do this.  This makes caches
>> easier because the cleanup is automatic.

Andrew> I'm not sure I understand how tp_hash helps here?

Yeah, I meant tp_dictoffset ...

Andrew> gdb.Progspace does have a __dict__ attribute, so things like:

... which is already set.  Sorry about that.

Andrew> works just fine, and I did consider this.  But I wasn't sure if this was
Andrew> "good practice".  My concern was mostly that this might lead to clashes
Andrew> between different extensions if good names are not chosen for
Andrew> 'my_random_var', or even clashes between extensions and future GDB
Andrew> enhancements -- my understanding is that if we later added
Andrew> 'my_random_var' as a builtin attribute this would take precedence over
Andrew> the user defined attribute, breaking their extension.

Yeah.  gdb should promise not to use "_" names here & in other spots
using tp_dictoffset.

Tom

[PATCHv2] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Changes in V2:

  - Fixed a typo in the docs that Tom pointed out,

  - Changed one of the test patterns after Linaro CI testing spotted a
    failure, I'm now using a gdb_test_multiple loop rather than
    gdb_test so that I can more easily ignore lines I'm not interested
    in.

I've been unable to repreduce the exact output that Linaro saw in
their CI testing, but I believe the updated test should now pass.
I'll leave this here for a few days before pushing.

---

Initially I just wanted a Python event for when GDB removes a program
space, I'm writing a Python extension that caches information for each
program space, and need to know when I should discard entries for a
particular program space.

But, it seemed easy enough to also add an event for when GDB adds a
new program space, so I went ahead and added both new events.

Of course, we don't currently have an observable for program space
addition or removal, so I first needed to add these.  After that it's
pretty simple to add two new Python events and have these trigger.

The two new event registries are:

  events.new_progspace
  events.free_progspace

These emit NewProgspaceEvent and FreeProgspaceEvent objects
respectively, each of these new event types has a 'progspace'
attribute that contains the relevant gdb.Progspace object.

There's a couple of things to be mindful of.

First, it is not possible to catch the NewProgspaceEvent for the very
first program space, the one that is created when GDB first starts, as
this program space is created before any Python scripts are sourced.

In order to allow this event to be caught we would need to defer
creating the first program space, and as a consequence the first
inferior, until some later time.  But, existing scripts could easily
depend on there being an initial inferior, so I really don't think we
should change that -- and so, we end up with the consequence that we
can't catch the event for the first program space.

The second, I think minor, issue, is that GDB doesn't clean up its
program spaces upon exit -- or at least, they are not cleaned up
before Python is shut down.  As a result, any program spaces in use at
the time GDB exits don't generate a FreeProgspaceEvent.  I'm not
particularly worried about this for my use case, I'm using the event
to ensure that a cache doesn't hold stale entries within a single GDB
session.  It's also easy enough to add a Python at-exit callback which
can do any final cleanup if needed.

Finally, when testing, I did hit a slightly weird issue with some of
the remote boards (e.g. remote-stdio-gdbserver).  As a consequence of
this issue I see some output like this in the gdb.log:

  (gdb) PASS: gdb.python/py-progspace-events.exp: inferior 1
  step
  FreeProgspaceEvent: <gdb.Progspace object at 0x7fb7e1d19c10>
  warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  warning: cannot close "target:/lib64/libc.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  do_parent_stuff () at py-progspace-events.c:41
  41        ++global_var;
  (gdb) PASS: gdb.python/py-progspace-events.exp: step

The 'FreeProgspaceEvent ...' line is expected, that's my test Python
extension logging the event.  What isn't expected are all the blocks
like:

  warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.

It turns out that this has nothing to do with my changes, this is just
a consequence of reading files over the remote protocol.  The test
forks a child process which GDB stays attached too.  When the child
exits, GDB cleans up by calling prune_inferiors, which in turn can
result in GDB trying to close some files that are open because of the
inferior being deleted.

If the prune_inferiors call occurs when the remote target is
running (and in non-async mode) then GDB will try to send a fileio
packet while the remote target is waiting for a stop reply, and the
remote target will throw an error, see remote_target::putpkt_binary in
remote.c for details.

I'm going to look at fixing this, but, as I said, this is nothing to
do with this change, I just mention it because I ended up needing to
account for these warning messages in one of my tests, and it all
looks a bit weird.

Approved-By: Tom Tromey <tom@tromey.com>
---
 gdb/NEWS                                      |   7 +
 gdb/doc/python.texi                           |  28 ++++
 gdb/observable.c                              |   2 +
 gdb/observable.h                              |   7 +
 gdb/progspace.c                               |   3 +
 gdb/python/py-all-events.def                  |   2 +
 gdb/python/py-event-types.def                 |  10 ++
 gdb/python/py-progspace.c                     |  67 ++++++++++
 .../gdb.python/py-progspace-events.c          |  73 ++++++++++
 .../gdb.python/py-progspace-events.exp        | 125 ++++++++++++++++++
 .../gdb.python/py-progspace-events.py         |  29 ++++
 11 files changed, 353 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.c
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.exp
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.py

diff --git a/gdb/NEWS b/gdb/NEWS
index 5b9ca9be30f..84fc87b07de 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -298,6 +298,13 @@ show tui mouse-events
      class, it signals to gdb that the printer may implement new
      pretty-printer methods.
 
+  ** New event registries gdb.events.new_progspace and
+     gdb.events.free_progspace, these emit NewProgspaceEvent and
+     FreeProgspaceEvent event types respectively.  Both of these event
+     types have a single 'progspace' attribute, which is the
+     gdb.Progspace that is either being added to GDB, or removed from
+     GDB.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5e88fd09e48..55af6e291bc 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3929,6 +3929,34 @@
 The @code{gdb.TargetConnection} that is being removed.
 @end defvar
 
+@item events.new_progspace
+This is emitted when @value{GDBN} adds a new program space
+(@pxref{Progspaces In Python,,Program Spaces In Python}).  The event
+is of type @code{gdb.NewProgspaceEvent}, and has a single read-only
+attribute:
+
+@defvar NewProgspaceEvent.progspace
+The @code{gdb.Progspace} that was added to @value{GDBN}.
+@end defvar
+
+No @code{NewProgspaceEvent} is emitted for the very first program
+space, which is assigned to the first inferior.  This first program
+space is created within @value{GDBN} before any Python scripts are
+sourced.
+
+@item events.free_progspace
+This is emitted when @value{GDBN} removes a program space
+(@pxref{Progspaces In Python,,Program Spaces In Python}), for example
+as a result of the @kbd{remove-inferiors} command
+(@pxref{remove_inferiors_cli,,@kbd{remove-inferiors}}).  The event is
+of type @code{gdb.FreeProgspaceEvent}, and has a single read-only
+attribute:
+
+@defvar FreeProgspaceEvent.progspace
+The @code{gdb.Progspace} that is about to be removed from
+@value{GDBN}.
+@end defvar
+
 @end table
 
 @node Threads In Python
diff --git a/gdb/observable.c b/gdb/observable.c
index 33e51223cf4..93a842e911f 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -72,6 +72,8 @@ DEFINE_OBSERVABLE (gdb_exiting);
 DEFINE_OBSERVABLE (connection_removed);
 DEFINE_OBSERVABLE (target_pre_wait);
 DEFINE_OBSERVABLE (target_post_wait);
+DEFINE_OBSERVABLE (new_program_space);
+DEFINE_OBSERVABLE (free_program_space);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 4ea203c6fc3..84e9b401e95 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -31,6 +31,7 @@ struct inferior;
 struct process_stratum_target;
 struct target_ops;
 struct trace_state_variable;
+struct program_space;
 
 namespace gdb
 {
@@ -233,6 +234,12 @@ extern observable <ptid_t /* ptid */> target_pre_wait;
 /* About to leave target_wait (). */
 extern observable <ptid_t /* event_ptid */> target_post_wait;
 
+/* New program space PSPACE was created.  */
+extern observable <program_space */* pspace */> new_program_space;
+
+/* The program space PSPACE is about to be deleted.  */
+extern observable <program_space */* pspace */> free_program_space;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 5cf8334ee67..1dbcd5875dd 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -28,6 +28,7 @@
 #include "inferior.h"
 #include <algorithm>
 #include "cli/cli-style.h"
+#include "observable.h"
 
 /* The last program space number assigned.  */
 static int last_program_space_num = 0;
@@ -98,6 +99,7 @@ program_space::program_space (address_space *aspace_)
     aspace (aspace_)
 {
   program_spaces.push_back (this);
+  gdb::observers::new_program_space.notify (this);
 }
 
 /* See progspace.h.  */
@@ -106,6 +108,7 @@ program_space::~program_space ()
 {
   gdb_assert (this != current_program_space);
 
+  gdb::observers::free_program_space.notify (this);
   remove_program_space (this);
 
   scoped_restore_current_program_space restore_pspace;
diff --git a/gdb/python/py-all-events.def b/gdb/python/py-all-events.def
index aa28f2c6f8b..ce0249f8fed 100644
--- a/gdb/python/py-all-events.def
+++ b/gdb/python/py-all-events.def
@@ -42,3 +42,5 @@ GDB_PY_DEFINE_EVENT(breakpoint_modified)
 GDB_PY_DEFINE_EVENT(before_prompt)
 GDB_PY_DEFINE_EVENT(gdb_exiting)
 GDB_PY_DEFINE_EVENT(connection_removed)
+GDB_PY_DEFINE_EVENT(new_progspace)
+GDB_PY_DEFINE_EVENT(free_progspace)
diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
index 395d6c07447..450132e2a2e 100644
--- a/gdb/python/py-event-types.def
+++ b/gdb/python/py-event-types.def
@@ -125,3 +125,13 @@ GDB_PY_DEFINE_EVENT_TYPE (connection,
 			  "ConnectionEvent",
 			  "GDB connection added or removed object",
 			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (new_progspace,
+			  "NewProgspaceEvent",
+			  "GDB new Progspace event object",
+			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (free_progspace,
+			  "FreeProgspaceEvent",
+			  "GDB free Progspace event object",
+			  event_object_type);
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index b98ac8dde61..18328e33fb6 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -26,6 +26,8 @@
 #include "arch-utils.h"
 #include "solib.h"
 #include "block.h"
+#include "observable.h"
+#include "py-event.h"
 
 struct pspace_object
 {
@@ -553,9 +555,74 @@ gdbpy_is_progspace (PyObject *obj)
   return PyObject_TypeCheck (obj, &pspace_object_type);
 }
 
+/* Helper function to emit NewProgspaceEvent (when ADDING_P is true) or
+   FreeProgspaceEvent events (when ADDING_P is false).  */
+
+static void
+gdbpy_program_space_event (program_space *pspace, bool adding_p)
+{
+  if (!gdb_python_initialized)
+    return;
+
+  gdbpy_enter enter_py;
+
+  eventregistry_object *registry;
+  PyTypeObject *event_type;
+  if (adding_p)
+    {
+      registry = gdb_py_events.new_progspace;
+      event_type = &new_progspace_event_object_type;
+    }
+  else
+    {
+      registry = gdb_py_events.free_progspace;
+      event_type = &free_progspace_event_object_type;
+    }
+
+  if (evregpy_no_listeners_p (registry))
+    return;
+
+  gdbpy_ref<> pspace_obj = pspace_to_pspace_object (pspace);
+  if (pspace_obj == nullptr)
+    {
+      gdbpy_print_stack ();
+      return;
+    }
+
+  gdbpy_ref<> event = create_event_object (event_type);
+  if (event == nullptr
+      || evpy_add_attribute (event.get (), "progspace",
+			     pspace_obj.get ()) < 0
+      || evpy_emit_event (event.get (), registry) < 0)
+    gdbpy_print_stack ();
+}
+
+
+/* Emit a NewProgspaceEvent to indicate PSPACE has been created.  */
+
+static void
+gdbpy_new_program_space_event (program_space *pspace)
+{
+  gdbpy_program_space_event (pspace, true);
+}
+
+/* Emit a FreeProgspaceEvent to indicate PSPACE is just about to be removed
+   from GDB.  */
+
+static void
+gdbpy_free_program_space_event (program_space *pspace)
+{
+  gdbpy_program_space_event (pspace, false);
+}
+
 static int CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
 gdbpy_initialize_pspace (void)
 {
+  gdb::observers::new_program_space.attach (gdbpy_new_program_space_event,
+					    "py-progspace");
+  gdb::observers::free_program_space.attach (gdbpy_free_program_space_event,
+					     "py-progspace");
+
   if (PyType_Ready (&pspace_object_type) < 0)
     return -1;
 
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.c b/gdb/testsuite/gdb.python/py-progspace-events.c
new file mode 100644
index 00000000000..6684cf8ac0a
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.c
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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>
+#include <sys/types.h>
+#include <assert.h>
+#include <sys/wait.h>
+
+/* This gives the inferior something to do.  */
+volatile int global_var = 0;
+
+void
+breakpt ()
+{ /* Nothing.  */ }
+
+void
+do_child_stuff ()
+{
+  breakpt ();
+  ++global_var;
+}
+
+void
+do_parent_stuff ()
+{
+  breakpt ();
+  ++global_var;
+}
+
+void
+create_child ()
+{
+  int stat;
+  pid_t wpid;
+  breakpt ();
+  pid_t pid = fork ();
+  assert (pid != -1);
+
+  if (pid == 0)
+    {
+      /* Child.  */
+      do_child_stuff ();
+      return;
+    }
+
+  /* Parent.  */
+  do_parent_stuff ();
+  wpid = waitpid (pid, &stat, 0);
+  assert (wpid == pid);
+}
+
+
+
+int
+main ()
+{
+  create_child ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.exp b/gdb/testsuite/gdb.python/py-progspace-events.exp
new file mode 100644
index 00000000000..60ae290c0b2
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.exp
@@ -0,0 +1,125 @@
+# Copyright (C) 2023 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/>.
+
+# This file is part of the GDB testsuite.  It tests the program space
+# related events in the Python API.
+
+load_lib gdb-python.exp
+
+require allow_python_tests
+
+standard_testfile
+
+if {[prepare_for_testing "preparing" $testfile $srcfile] == -1} {
+    return -1
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/py-progspace-events.py]
+gdb_test_no_output "source ${pyfile}" "load python file"
+
+if {![runto_main]} {
+    return
+}
+
+gdb_breakpoint breakpt
+
+gdb_continue_to_breakpoint "run to breakpt function"
+
+gdb_test_no_output "set detach-on-fork off"
+
+# Continue until the parent process forks and a new child is added.
+# Done this way so we can count the new progspace events; we expect to
+# see exactly one.
+set new_progspace_event_count 0
+gdb_test_multiple "continue" "continue until child process appears" {
+    -re "^NewProgspaceEvent: <gdb.Progspace object at $hex>\r\n" {
+	# This is a correctly formed event line.
+	incr new_progspace_event_count
+	exp_continue
+    }
+
+    -re "^NewProgspaceEvent:\[^\r\n\]+\r\n" {
+	# This is an incorrectly formed event line.
+	fail $gdb_test_name
+    }
+
+    -re "^$gdb_prompt $" {
+	pass $gdb_test_name
+    }
+
+    -re "^\[^\r\n\]*\r\n" {
+	exp_continue
+    }
+}
+
+gdb_assert { $new_progspace_event_count == 1 } \
+    "only a single new progspace event seen"
+
+# Switch to inferior 2 and continue until we hit breakpt.
+gdb_test "inferior 2" "\\\[Switching to inferior 2 .*"
+gdb_continue_to_breakpoint "run to breakpt in inferior 2"
+
+# Let inferior 2 exit.  The new program space is not removed at this
+# point.
+gdb_test "continue" \
+    [multi_line \
+	 "^Continuing\\." \
+	 "\\\[Inferior $decimal \[^\r\n\]+ exited normally\\\]"] \
+    "continue until inferior 2 exits"
+
+gdb_test "inferior 1" "\\\[Switching to inferior 1 .*"
+
+# Step the inferior.  During this process GDB will prune the now
+# defunct inferior, which deletes its program space, which should
+# trigger the FreeProgspaceEvent.
+#
+# However, there is a slight problem.  When the target is remote, and
+# GDB is accessing files using remote fileio, then GDB will attempt to
+# prune the inferior at a point in time when the remote target is
+# waiting for a stop reply.  Pruning an inferior causes GDB to close
+# files associated with that inferior.
+#
+# In non-async mode we can't send fileio packets while waiting for a
+# stop reply, so the attempts to close files fails, and this shows up
+# as an error.
+#
+# As this error has nothing to do with the feature being tested here,
+# we just accept the error message, the important part is the
+# 'FreeProgspaceEvent' string, so long as that appears (just once)
+# then the test is a success.
+set warning_msg \
+    [multi_line \
+	 "warning: cannot close \"\[^\r\n\]+\": Cannot execute this command while the target is running\\." \
+	 "Use the \"interrupt\" command to stop the target" \
+	 "and then try again\\."]
+
+gdb_test "step" \
+    [multi_line \
+	 "^FreeProgspaceEvent: <gdb.Progspace object at $hex>(?:\r\n$warning_msg)*" \
+	 "do_parent_stuff \\(\\) at \[^\r\n\]+" \
+	 "$decimal\\s+\[^\r\n\]+"]
+
+# Let this inferior run to completion.
+gdb_continue_to_end
+
+# Check the program space events trigger when a new inferior is
+# manually added and removed.
+gdb_test "add-inferior" \
+    [multi_line \
+	 "^NewProgspaceEvent: <gdb.Progspace object at $hex>" \
+	 "\\\[New inferior 3\\\]" \
+	 "Added inferior 3\[^\r\n\]*"]
+gdb_test "remove-inferior 3" \
+    "^FreeProgspaceEvent: <gdb.Progspace object at $hex>"
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.py b/gdb/testsuite/gdb.python/py-progspace-events.py
new file mode 100644
index 00000000000..3abb421cffa
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.py
@@ -0,0 +1,29 @@
+# Copyright (C) 2023 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/>.
+
+# This file is part of the GDB testsuite, it registers listeners for
+# the Progspace related events.
+
+import gdb
+import gdb.events
+
+def new_progspace(event):
+    print("NewProgspaceEvent: %s" % str(event.progspace))
+
+def free_progspace(event):
+    print("FreeProgspaceEvent: %s" % str(event.progspace))
+
+gdb.events.new_progspace.connect(new_progspace)
+gdb.events.free_progspace.connect(free_progspace)

base-commit: f586e3409b752748bf213520c2dbb0b44e0005d8
-- 
2.25.4

Re: [PATCH] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>>> This approach seems totally fine to me, but it's worth noting that you
>>> could attach data directly to a Progspace if the tp_hash field were
>>> filled in.  Some other Python classes in gdb do this.  This makes caches
>>> easier because the cleanup is automatic.
>
> Andrew> I'm not sure I understand how tp_hash helps here?
>
> Yeah, I meant tp_dictoffset ...
>
> Andrew> gdb.Progspace does have a __dict__ attribute, so things like:
>
> ... which is already set.  Sorry about that.
>
> Andrew> works just fine, and I did consider this.  But I wasn't sure if this was
> Andrew> "good practice".  My concern was mostly that this might lead to clashes
> Andrew> between different extensions if good names are not chosen for
> Andrew> 'my_random_var', or even clashes between extensions and future GDB
> Andrew> enhancements -- my understanding is that if we later added
> Andrew> 'my_random_var' as a builtin attribute this would take precedence over
> Andrew> the user defined attribute, breaking their extension.
>
> Yeah.  gdb should promise not to use "_" names here & in other spots
> using tp_dictoffset.

That's a good idea.  I'll add this to my todo list, as having this
documented would make this more useful I think.

Thanks,
Andrew

Re: [PATCHv2] gdb: add Python events for program space addition and removal

[Eli Zaretskii]

> Date: Wed, 27 Sep 2023 16:16:55 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Tom Tromey <tom@tromey.com>
> 
>  gdb/NEWS                                      |   7 +
>  gdb/doc/python.texi                           |  28 ++++
>  gdb/observable.c                              |   2 +
>  gdb/observable.h                              |   7 +
>  gdb/progspace.c                               |   3 +
>  gdb/python/py-all-events.def                  |   2 +
>  gdb/python/py-event-types.def                 |  10 ++
>  gdb/python/py-progspace.c                     |  67 ++++++++++
>  .../gdb.python/py-progspace-events.c          |  73 ++++++++++
>  .../gdb.python/py-progspace-events.exp        | 125 ++++++++++++++++++
>  .../gdb.python/py-progspace-events.py         |  29 ++++
>  11 files changed, 353 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.c
>  create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.exp
>  create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.py

OK for the documentation parts, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

[PATCHv3] gdb: add Python events for program space addition and removal

[Andrew Burgess]

In v3:

  - Rebased onto current master (there were some conflicts),

  - No other changes.

Code is already approved, just needs a doc review.

Thanks,
Andrew


---

Initially I just wanted a Python event for when GDB removes a program
space, I'm writing a Python extension that caches information for each
program space, and need to know when I should discard entries for a
particular program space.

But, it seemed easy enough to also add an event for when GDB adds a
new program space, so I went ahead and added both new events.

Of course, we don't currently have an observable for program space
addition or removal, so I first needed to add these.  After that it's
pretty simple to add two new Python events and have these trigger.

The two new event registries are:

  events.new_progspace
  events.free_progspace

These emit NewProgspaceEvent and FreeProgspaceEvent objects
respectively, each of these new event types has a 'progspace'
attribute that contains the relevant gdb.Progspace object.

There's a couple of things to be mindful of.

First, it is not possible to catch the NewProgspaceEvent for the very
first program space, the one that is created when GDB first starts, as
this program space is created before any Python scripts are sourced.

In order to allow this event to be caught we would need to defer
creating the first program space, and as a consequence the first
inferior, until some later time.  But, existing scripts could easily
depend on there being an initial inferior, so I really don't think we
should change that -- and so, we end up with the consequence that we
can't catch the event for the first program space.

The second, I think minor, issue, is that GDB doesn't clean up its
program spaces upon exit -- or at least, they are not cleaned up
before Python is shut down.  As a result, any program spaces in use at
the time GDB exits don't generate a FreeProgspaceEvent.  I'm not
particularly worried about this for my use case, I'm using the event
to ensure that a cache doesn't hold stale entries within a single GDB
session.  It's also easy enough to add a Python at-exit callback which
can do any final cleanup if needed.

Finally, when testing, I did hit a slightly weird issue with some of
the remote boards (e.g. remote-stdio-gdbserver).  As a consequence of
this issue I see some output like this in the gdb.log:

  (gdb) PASS: gdb.python/py-progspace-events.exp: inferior 1
  step
  FreeProgspaceEvent: <gdb.Progspace object at 0x7fb7e1d19c10>
  warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  warning: cannot close "target:/lib64/libc.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  do_parent_stuff () at py-progspace-events.c:41
  41        ++global_var;
  (gdb) PASS: gdb.python/py-progspace-events.exp: step

The 'FreeProgspaceEvent ...' line is expected, that's my test Python
extension logging the event.  What isn't expected are all the blocks
like:

  warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.

It turns out that this has nothing to do with my changes, this is just
a consequence of reading files over the remote protocol.  The test
forks a child process which GDB stays attached too.  When the child
exits, GDB cleans up by calling prune_inferiors, which in turn can
result in GDB trying to close some files that are open because of the
inferior being deleted.

If the prune_inferiors call occurs when the remote target is
running (and in non-async mode) then GDB will try to send a fileio
packet while the remote target is waiting for a stop reply, and the
remote target will throw an error, see remote_target::putpkt_binary in
remote.c for details.

I'm going to look at fixing this, but, as I said, this is nothing to
do with this change, I just mention it because I ended up needing to
account for these warning messages in one of my tests, and it all
looks a bit weird.

Approved-By: Tom Tromey <tom@tromey.com>
---
 gdb/NEWS                                      |   7 +
 gdb/doc/python.texi                           |  29 ++++
 gdb/observable.c                              |   2 +
 gdb/observable.h                              |   6 +
 gdb/progspace.c                               |   3 +
 gdb/python/py-all-events.def                  |   2 +
 gdb/python/py-event-types.def                 |  10 ++
 gdb/python/py-progspace.c                     |  63 +++++++++
 .../gdb.python/py-progspace-events.c          |  73 ++++++++++
 .../gdb.python/py-progspace-events.exp        | 125 ++++++++++++++++++
 .../gdb.python/py-progspace-events.py         |  29 ++++
 11 files changed, 349 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.c
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.exp
 create mode 100644 gdb/testsuite/gdb.python/py-progspace-events.py

diff --git a/gdb/NEWS b/gdb/NEWS
index fd7ab282f69..41987c6b9ec 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -315,6 +315,13 @@ show tui mouse-events
      gdb.Progspace) and 'reload' (a Boolean) attributes.  This event
      is emitted when gdb.Progspace.executable_filename changes.
 
+  ** New event registries gdb.events.new_progspace and
+     gdb.events.free_progspace, these emit NewProgspaceEvent and
+     FreeProgspaceEvent event types respectively.  Both of these event
+     types have a single 'progspace' attribute, which is the
+     gdb.Progspace that is either being added to GDB, or removed from
+     GDB.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 0471e78aa19..a97e4451897 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3962,6 +3962,35 @@
 file is updated first, so when this event is emitted, the executable
 filename will have changed, but the symbol filename might still hold
 its previous value.
+
+@item events.new_progspace
+This is emitted when @value{GDBN} adds a new program space
+(@pxref{Progspaces In Python,,Program Spaces In Python}).  The event
+is of type @code{gdb.NewProgspaceEvent}, and has a single read-only
+attribute:
+
+@defvar NewProgspaceEvent.progspace
+The @code{gdb.Progspace} that was added to @value{GDBN}.
+@end defvar
+
+No @code{NewProgspaceEvent} is emitted for the very first program
+space, which is assigned to the first inferior.  This first program
+space is created within @value{GDBN} before any Python scripts are
+sourced.
+
+@item events.free_progspace
+This is emitted when @value{GDBN} removes a program space
+(@pxref{Progspaces In Python,,Program Spaces In Python}), for example
+as a result of the @kbd{remove-inferiors} command
+(@pxref{remove_inferiors_cli,,@kbd{remove-inferiors}}).  The event is
+of type @code{gdb.FreeProgspaceEvent}, and has a single read-only
+attribute:
+
+@defvar FreeProgspaceEvent.progspace
+The @code{gdb.Progspace} that is about to be removed from
+@value{GDBN}.
+@end defvar
+
 @end table
 
 @node Threads In Python
diff --git a/gdb/observable.c b/gdb/observable.c
index 33e51223cf4..93a842e911f 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -72,6 +72,8 @@ DEFINE_OBSERVABLE (gdb_exiting);
 DEFINE_OBSERVABLE (connection_removed);
 DEFINE_OBSERVABLE (target_pre_wait);
 DEFINE_OBSERVABLE (target_post_wait);
+DEFINE_OBSERVABLE (new_program_space);
+DEFINE_OBSERVABLE (free_program_space);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 464d0b70f5b..20535bb655d 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -241,6 +241,12 @@ extern observable <ptid_t /* ptid */> target_pre_wait;
 /* About to leave target_wait (). */
 extern observable <ptid_t /* event_ptid */> target_post_wait;
 
+/* New program space PSPACE was created.  */
+extern observable <program_space */* pspace */> new_program_space;
+
+/* The program space PSPACE is about to be deleted.  */
+extern observable <program_space */* pspace */> free_program_space;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 5cf8334ee67..1dbcd5875dd 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -28,6 +28,7 @@
 #include "inferior.h"
 #include <algorithm>
 #include "cli/cli-style.h"
+#include "observable.h"
 
 /* The last program space number assigned.  */
 static int last_program_space_num = 0;
@@ -98,6 +99,7 @@ program_space::program_space (address_space *aspace_)
     aspace (aspace_)
 {
   program_spaces.push_back (this);
+  gdb::observers::new_program_space.notify (this);
 }
 
 /* See progspace.h.  */
@@ -106,6 +108,7 @@ program_space::~program_space ()
 {
   gdb_assert (this != current_program_space);
 
+  gdb::observers::free_program_space.notify (this);
   remove_program_space (this);
 
   scoped_restore_current_program_space restore_pspace;
diff --git a/gdb/python/py-all-events.def b/gdb/python/py-all-events.def
index 04a12e1bdf0..b1decc77231 100644
--- a/gdb/python/py-all-events.def
+++ b/gdb/python/py-all-events.def
@@ -43,3 +43,5 @@ GDB_PY_DEFINE_EVENT(before_prompt)
 GDB_PY_DEFINE_EVENT(gdb_exiting)
 GDB_PY_DEFINE_EVENT(connection_removed)
 GDB_PY_DEFINE_EVENT(executable_changed)
+GDB_PY_DEFINE_EVENT(new_progspace)
+GDB_PY_DEFINE_EVENT(free_progspace)
diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
index b862094650d..c6225115027 100644
--- a/gdb/python/py-event-types.def
+++ b/gdb/python/py-event-types.def
@@ -130,3 +130,13 @@ GDB_PY_DEFINE_EVENT_TYPE (executable_changed,
 			  "ExecutableChangedEvent",
 			  "GDB executable changed event",
 			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (new_progspace,
+			  "NewProgspaceEvent",
+			  "GDB new Progspace event object",
+			  event_object_type);
+
+GDB_PY_DEFINE_EVENT_TYPE (free_progspace,
+			  "FreeProgspaceEvent",
+			  "GDB free Progspace event object",
+			  event_object_type);
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 082509b2b5b..72a5d4f4c62 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -643,11 +643,74 @@ gdbpy_executable_changed (struct program_space *pspace, bool reload_p)
       gdbpy_print_stack ();
 }
 
+/* Helper function to emit NewProgspaceEvent (when ADDING_P is true) or
+   FreeProgspaceEvent events (when ADDING_P is false).  */
+
+static void
+gdbpy_program_space_event (program_space *pspace, bool adding_p)
+{
+  if (!gdb_python_initialized)
+    return;
+
+  gdbpy_enter enter_py;
+
+  eventregistry_object *registry;
+  PyTypeObject *event_type;
+  if (adding_p)
+    {
+      registry = gdb_py_events.new_progspace;
+      event_type = &new_progspace_event_object_type;
+    }
+  else
+    {
+      registry = gdb_py_events.free_progspace;
+      event_type = &free_progspace_event_object_type;
+    }
+
+  if (evregpy_no_listeners_p (registry))
+    return;
+
+  gdbpy_ref<> pspace_obj = pspace_to_pspace_object (pspace);
+  if (pspace_obj == nullptr)
+    {
+      gdbpy_print_stack ();
+      return;
+    }
+
+  gdbpy_ref<> event = create_event_object (event_type);
+  if (event == nullptr
+      || evpy_add_attribute (event.get (), "progspace",
+			     pspace_obj.get ()) < 0
+      || evpy_emit_event (event.get (), registry) < 0)
+    gdbpy_print_stack ();
+}
+
+/* Emit a NewProgspaceEvent to indicate PSPACE has been created.  */
+
+static void
+gdbpy_new_program_space_event (program_space *pspace)
+{
+  gdbpy_program_space_event (pspace, true);
+}
+
+/* Emit a FreeProgspaceEvent to indicate PSPACE is just about to be removed
+   from GDB.  */
+
+static void
+gdbpy_free_program_space_event (program_space *pspace)
+{
+  gdbpy_program_space_event (pspace, false);
+}
+
 static int CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
 gdbpy_initialize_pspace (void)
 {
   gdb::observers::executable_changed.attach (gdbpy_executable_changed,
 					     "py-progspace");
+  gdb::observers::new_program_space.attach (gdbpy_new_program_space_event,
+					    "py-progspace");
+  gdb::observers::free_program_space.attach (gdbpy_free_program_space_event,
+					     "py-progspace");
 
   if (PyType_Ready (&pspace_object_type) < 0)
     return -1;
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.c b/gdb/testsuite/gdb.python/py-progspace-events.c
new file mode 100644
index 00000000000..6684cf8ac0a
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.c
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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>
+#include <sys/types.h>
+#include <assert.h>
+#include <sys/wait.h>
+
+/* This gives the inferior something to do.  */
+volatile int global_var = 0;
+
+void
+breakpt ()
+{ /* Nothing.  */ }
+
+void
+do_child_stuff ()
+{
+  breakpt ();
+  ++global_var;
+}
+
+void
+do_parent_stuff ()
+{
+  breakpt ();
+  ++global_var;
+}
+
+void
+create_child ()
+{
+  int stat;
+  pid_t wpid;
+  breakpt ();
+  pid_t pid = fork ();
+  assert (pid != -1);
+
+  if (pid == 0)
+    {
+      /* Child.  */
+      do_child_stuff ();
+      return;
+    }
+
+  /* Parent.  */
+  do_parent_stuff ();
+  wpid = waitpid (pid, &stat, 0);
+  assert (wpid == pid);
+}
+
+
+
+int
+main ()
+{
+  create_child ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.exp b/gdb/testsuite/gdb.python/py-progspace-events.exp
new file mode 100644
index 00000000000..60ae290c0b2
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.exp
@@ -0,0 +1,125 @@
+# Copyright (C) 2023 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/>.
+
+# This file is part of the GDB testsuite.  It tests the program space
+# related events in the Python API.
+
+load_lib gdb-python.exp
+
+require allow_python_tests
+
+standard_testfile
+
+if {[prepare_for_testing "preparing" $testfile $srcfile] == -1} {
+    return -1
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/py-progspace-events.py]
+gdb_test_no_output "source ${pyfile}" "load python file"
+
+if {![runto_main]} {
+    return
+}
+
+gdb_breakpoint breakpt
+
+gdb_continue_to_breakpoint "run to breakpt function"
+
+gdb_test_no_output "set detach-on-fork off"
+
+# Continue until the parent process forks and a new child is added.
+# Done this way so we can count the new progspace events; we expect to
+# see exactly one.
+set new_progspace_event_count 0
+gdb_test_multiple "continue" "continue until child process appears" {
+    -re "^NewProgspaceEvent: <gdb.Progspace object at $hex>\r\n" {
+	# This is a correctly formed event line.
+	incr new_progspace_event_count
+	exp_continue
+    }
+
+    -re "^NewProgspaceEvent:\[^\r\n\]+\r\n" {
+	# This is an incorrectly formed event line.
+	fail $gdb_test_name
+    }
+
+    -re "^$gdb_prompt $" {
+	pass $gdb_test_name
+    }
+
+    -re "^\[^\r\n\]*\r\n" {
+	exp_continue
+    }
+}
+
+gdb_assert { $new_progspace_event_count == 1 } \
+    "only a single new progspace event seen"
+
+# Switch to inferior 2 and continue until we hit breakpt.
+gdb_test "inferior 2" "\\\[Switching to inferior 2 .*"
+gdb_continue_to_breakpoint "run to breakpt in inferior 2"
+
+# Let inferior 2 exit.  The new program space is not removed at this
+# point.
+gdb_test "continue" \
+    [multi_line \
+	 "^Continuing\\." \
+	 "\\\[Inferior $decimal \[^\r\n\]+ exited normally\\\]"] \
+    "continue until inferior 2 exits"
+
+gdb_test "inferior 1" "\\\[Switching to inferior 1 .*"
+
+# Step the inferior.  During this process GDB will prune the now
+# defunct inferior, which deletes its program space, which should
+# trigger the FreeProgspaceEvent.
+#
+# However, there is a slight problem.  When the target is remote, and
+# GDB is accessing files using remote fileio, then GDB will attempt to
+# prune the inferior at a point in time when the remote target is
+# waiting for a stop reply.  Pruning an inferior causes GDB to close
+# files associated with that inferior.
+#
+# In non-async mode we can't send fileio packets while waiting for a
+# stop reply, so the attempts to close files fails, and this shows up
+# as an error.
+#
+# As this error has nothing to do with the feature being tested here,
+# we just accept the error message, the important part is the
+# 'FreeProgspaceEvent' string, so long as that appears (just once)
+# then the test is a success.
+set warning_msg \
+    [multi_line \
+	 "warning: cannot close \"\[^\r\n\]+\": Cannot execute this command while the target is running\\." \
+	 "Use the \"interrupt\" command to stop the target" \
+	 "and then try again\\."]
+
+gdb_test "step" \
+    [multi_line \
+	 "^FreeProgspaceEvent: <gdb.Progspace object at $hex>(?:\r\n$warning_msg)*" \
+	 "do_parent_stuff \\(\\) at \[^\r\n\]+" \
+	 "$decimal\\s+\[^\r\n\]+"]
+
+# Let this inferior run to completion.
+gdb_continue_to_end
+
+# Check the program space events trigger when a new inferior is
+# manually added and removed.
+gdb_test "add-inferior" \
+    [multi_line \
+	 "^NewProgspaceEvent: <gdb.Progspace object at $hex>" \
+	 "\\\[New inferior 3\\\]" \
+	 "Added inferior 3\[^\r\n\]*"]
+gdb_test "remove-inferior 3" \
+    "^FreeProgspaceEvent: <gdb.Progspace object at $hex>"
diff --git a/gdb/testsuite/gdb.python/py-progspace-events.py b/gdb/testsuite/gdb.python/py-progspace-events.py
new file mode 100644
index 00000000000..3abb421cffa
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-progspace-events.py
@@ -0,0 +1,29 @@
+# Copyright (C) 2023 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/>.
+
+# This file is part of the GDB testsuite, it registers listeners for
+# the Progspace related events.
+
+import gdb
+import gdb.events
+
+def new_progspace(event):
+    print("NewProgspaceEvent: %s" % str(event.progspace))
+
+def free_progspace(event):
+    print("FreeProgspaceEvent: %s" % str(event.progspace))
+
+gdb.events.new_progspace.connect(new_progspace)
+gdb.events.free_progspace.connect(free_progspace)

base-commit: 68510906a981d6abd31c51f49b2ec7e18db0a338
-- 
2.25.4

Re: [PATCHv3] gdb: add Python events for program space addition and removal

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>,
> 	Eli Zaretskii <eliz@gnu.org>,
> 	Tom Tromey <tom@tromey.com>
> Date: Fri, 29 Sep 2023 14:24:18 +0100
> 
> In v3:
> 
>   - Rebased onto current master (there were some conflicts),
> 
>   - No other changes.
> 
> Code is already approved, just needs a doc review.

I thought I've reviewed this already?

Anyway, the documentation parts are okay, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCHv3] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: Andrew Burgess <aburgess@redhat.com>,
>> 	Eli Zaretskii <eliz@gnu.org>,
>> 	Tom Tromey <tom@tromey.com>
>> Date: Fri, 29 Sep 2023 14:24:18 +0100
>> 
>> In v3:
>> 
>>   - Rebased onto current master (there were some conflicts),
>> 
>>   - No other changes.
>> 
>> Code is already approved, just needs a doc review.
>
> I thought I've reviewed this already?

:) I posted V2 on Wed and V3 on Fri (to resolve a merge-conflict during
a rebase).  You reviewed both versions on Friday, for which I thank you!

>
> Anyway, the documentation parts are okay, thanks.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

I went ahead and pushed this patch.

Thanks,
Andrew

Re: [PATCHv3] gdb: add Python events for program space addition and removal

[Simon Marchi]

On 9/29/23 09:24, Andrew Burgess via Gdb-patches wrote:
> In v3:
> 
>   - Rebased onto current master (there were some conflicts),
> 
>   - No other changes.
> 
> Code is already approved, just needs a doc review.
> 
> Thanks,
> Andrew
> 
> 
> ---
> 
> Initially I just wanted a Python event for when GDB removes a program
> space, I'm writing a Python extension that caches information for each
> program space, and need to know when I should discard entries for a
> particular program space.
> 
> But, it seemed easy enough to also add an event for when GDB adds a
> new program space, so I went ahead and added both new events.
> 
> Of course, we don't currently have an observable for program space
> addition or removal, so I first needed to add these.  After that it's
> pretty simple to add two new Python events and have these trigger.
> 
> The two new event registries are:
> 
>   events.new_progspace
>   events.free_progspace
> 
> These emit NewProgspaceEvent and FreeProgspaceEvent objects
> respectively, each of these new event types has a 'progspace'
> attribute that contains the relevant gdb.Progspace object.
> 
> There's a couple of things to be mindful of.
> 
> First, it is not possible to catch the NewProgspaceEvent for the very
> first program space, the one that is created when GDB first starts, as
> this program space is created before any Python scripts are sourced.
> 
> In order to allow this event to be caught we would need to defer
> creating the first program space, and as a consequence the first
> inferior, until some later time.  But, existing scripts could easily
> depend on there being an initial inferior, so I really don't think we
> should change that -- and so, we end up with the consequence that we
> can't catch the event for the first program space.
> 
> The second, I think minor, issue, is that GDB doesn't clean up its
> program spaces upon exit -- or at least, they are not cleaned up
> before Python is shut down.  As a result, any program spaces in use at
> the time GDB exits don't generate a FreeProgspaceEvent.  I'm not
> particularly worried about this for my use case, I'm using the event
> to ensure that a cache doesn't hold stale entries within a single GDB
> session.  It's also easy enough to add a Python at-exit callback which
> can do any final cleanup if needed.
> 
> Finally, when testing, I did hit a slightly weird issue with some of
> the remote boards (e.g. remote-stdio-gdbserver).  As a consequence of
> this issue I see some output like this in the gdb.log:
> 
>   (gdb) PASS: gdb.python/py-progspace-events.exp: inferior 1
>   step
>   FreeProgspaceEvent: <gdb.Progspace object at 0x7fb7e1d19c10>
>   warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
>   Use the "interrupt" command to stop the target
>   and then try again.
>   warning: cannot close "target:/lib64/libc.so.6": Cannot execute this command while the target is running.
>   Use the "interrupt" command to stop the target
>   and then try again.
>   warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Cannot execute this command while the target is running.
>   Use the "interrupt" command to stop the target
>   and then try again.
>   do_parent_stuff () at py-progspace-events.c:41
>   41        ++global_var;
>   (gdb) PASS: gdb.python/py-progspace-events.exp: step
> 
> The 'FreeProgspaceEvent ...' line is expected, that's my test Python
> extension logging the event.  What isn't expected are all the blocks
> like:
> 
>   warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
>   Use the "interrupt" command to stop the target
>   and then try again.
> 
> It turns out that this has nothing to do with my changes, this is just
> a consequence of reading files over the remote protocol.  The test
> forks a child process which GDB stays attached too.  When the child
> exits, GDB cleans up by calling prune_inferiors, which in turn can
> result in GDB trying to close some files that are open because of the
> inferior being deleted.
> 
> If the prune_inferiors call occurs when the remote target is
> running (and in non-async mode) then GDB will try to send a fileio
> packet while the remote target is waiting for a stop reply, and the
> remote target will throw an error, see remote_target::putpkt_binary in
> remote.c for details.
> 
> I'm going to look at fixing this, but, as I said, this is nothing to
> do with this change, I just mention it because I ended up needing to
> account for these warning messages in one of my tests, and it all
> looks a bit weird.
> 
> Approved-By: Tom Tromey <tom@tromey.com>

Hi Andrew,

The new test fails with native-gdbserver:


    Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.python/py-progspace-events.exp ...
    ERROR: GDB process no longer exists

    # of expected passes            9
    # of unresolved testcases       4

GDB aborts when calling error here, apparently nothing catches the
exception so std::terminate is called:

    #4  0x000055a4a0fae18d in error (fmt=0x55a49adcd020 "Cannot execute this command while the target is running.\nUse the \"interrupt\" command to stop the target\nand then try again.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:46
    #5  0x000055a49ec2f495 in remote_target::putpkt_binary (this=0x61b00003f080, buf=0x62d00371e400 "qXfer:auxv:read::0,1000", cnt=23) at /home/smarchi/src/binutils-gdb/gdb/remote.c:9740
    #6  0x000055a49ec2efc4 in remote_target::putpkt (this=0x61b00003f080, buf=0x62d00371e400 "qXfer:auxv:read::0,1000") at /home/smarchi/src/binutils-gdb/gdb/remote.c:9698
    #7  0x000055a49ec8b287 in remote_target::putpkt (this=0x61b00003f080, buf=std::__debug::vector of length 36862, capacity 36862 = {...}) at /home/smarchi/src/binutils-gdb/gdb/remote.c:1211
    #8  0x000055a49ec45c59 in remote_target::remote_read_qxfer (this=0x61b00003f080, object_name=0x55a49adced00 "auxv", annex=0x0, readbuf=0x62100057f900 '\276' <repeats 200 times>..., offset=0, len=3904966623561266976, xfered_len=0x7f5f31c5f220, which_packet=18) at /home/smarchi/src/binutils-gdb/gdb/remote.c:11316
    #9  0x000055a49ec47573 in remote_target::xfer_partial (this=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0, readbuf=0x62100057f900 '\276' <repeats 200 times>..., writebuf=0x0, offset=0, len=4096, xfered_len=0x7f5f31c5f220) at /home/smarchi/src/binutils-gdb/gdb/remote.c:11438
    #10 0x000055a49f4595b1 in target_xfer_partial (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0, readbuf=0x62100057f900 '\276' <repeats 200 times>..., writebuf=0x0, offset=0, len=4096, xfered_len=0x7f5f31c5f220) at /home/smarchi/src/binutils-gdb/gdb/target.c:1717
    #11 0x000055a49f45aed9 in target_read_partial (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0, buf=0x62100057f900 '\276' <repeats 200 times>..., offset=0, len=4096, xfered_len=0x7f5f31c5f220) at /home/smarchi/src/binutils-gdb/gdb/target.c:1951
    #12 0x000055a49f501472 in target_read_alloc_1<unsigned char> (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0) at /home/smarchi/src/binutils-gdb/gdb/target.c:2286
    #13 0x000055a49f45c9c2 in target_read_alloc (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0) at /home/smarchi/src/binutils-gdb/gdb/target.c:2315
    #14 0x000055a49c920733 in target_read_auxv_raw (ops=0x61b00003f080) at /home/smarchi/src/binutils-gdb/gdb/auxv.c:379
    #15 0x000055a49c920501 in target_read_auxv () at /home/smarchi/src/binutils-gdb/gdb/auxv.c:368
    #16 0x000055a49c920c06 in target_auxv_search (match=0, valp=0x7f5f31a2fc60) at /home/smarchi/src/binutils-gdb/gdb/auxv.c:415
    #17 0x000055a49e13e669 in linux_is_uclinux () at /home/smarchi/src/binutils-gdb/gdb/linux-tdep.c:433
    #18 0x000055a49e13e6fa in linux_has_shared_address_space (gdbarch=0x61c00001e080) at /home/smarchi/src/binutils-gdb/gdb/linux-tdep.c:440
    #19 0x000055a49c720d6f in gdbarch_has_shared_address_space (gdbarch=0x61c00001e080) at /home/smarchi/src/binutils-gdb/gdb/gdbarch.c:4889
    #20 0x000055a49e7ce076 in program_space::~program_space (this=0x61300004af00, __in_chrg=<optimized out>) at /home/smarchi/src/binutils-gdb/gdb/progspace.c:124
    #21 0x000055a49de5d91e in delete_inferior (inf=0x61800007f080) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:290
    #22 0x000055a49de604b7 in prune_inferiors () at /home/smarchi/src/binutils-gdb/gdb/inferior.c:480
    #23 0x000055a49debd2d9 in fetch_inferior_event () at /home/smarchi/src/binutils-gdb/gdb/infrun.c:4558

Simon

[PUSHED] gdb/python: reformat file with black

[Andrew Burgess]

Reformat a Python file with black after this commit:

  commit 59912fb2d22f8a4bb0862487f12a5cc65b6a013f
  Date:   Tue Sep 19 11:45:36 2023 +0100

      gdb: add Python events for program space addition and removal

There should be no functional change with this commit.
---
 gdb/testsuite/gdb.python/py-progspace-events.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-progspace-events.py b/gdb/testsuite/gdb.python/py-progspace-events.py
index 3abb421cffa..555221dcd4e 100644
--- a/gdb/testsuite/gdb.python/py-progspace-events.py
+++ b/gdb/testsuite/gdb.python/py-progspace-events.py
@@ -19,11 +19,14 @@
 import gdb
 import gdb.events
 
+
 def new_progspace(event):
     print("NewProgspaceEvent: %s" % str(event.progspace))
 
+
 def free_progspace(event):
     print("FreeProgspaceEvent: %s" % str(event.progspace))
 
+
 gdb.events.new_progspace.connect(new_progspace)
 gdb.events.free_progspace.connect(free_progspace)

base-commit: 57c699398c1a61f1a57574a5ed64654cbadd2a50
-- 
2.25.4

Re: [PATCHv3] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Simon Marchi <simark@simark.ca> writes:

> On 9/29/23 09:24, Andrew Burgess via Gdb-patches wrote:
>> In v3:
>> 
>>   - Rebased onto current master (there were some conflicts),
>> 
>>   - No other changes.
>> 
>> Code is already approved, just needs a doc review.
>> 
>> Thanks,
>> Andrew
>> 
>> 
>> ---
>> 
>> Initially I just wanted a Python event for when GDB removes a program
>> space, I'm writing a Python extension that caches information for each
>> program space, and need to know when I should discard entries for a
>> particular program space.
>> 
>> But, it seemed easy enough to also add an event for when GDB adds a
>> new program space, so I went ahead and added both new events.
>> 
>> Of course, we don't currently have an observable for program space
>> addition or removal, so I first needed to add these.  After that it's
>> pretty simple to add two new Python events and have these trigger.
>> 
>> The two new event registries are:
>> 
>>   events.new_progspace
>>   events.free_progspace
>> 
>> These emit NewProgspaceEvent and FreeProgspaceEvent objects
>> respectively, each of these new event types has a 'progspace'
>> attribute that contains the relevant gdb.Progspace object.
>> 
>> There's a couple of things to be mindful of.
>> 
>> First, it is not possible to catch the NewProgspaceEvent for the very
>> first program space, the one that is created when GDB first starts, as
>> this program space is created before any Python scripts are sourced.
>> 
>> In order to allow this event to be caught we would need to defer
>> creating the first program space, and as a consequence the first
>> inferior, until some later time.  But, existing scripts could easily
>> depend on there being an initial inferior, so I really don't think we
>> should change that -- and so, we end up with the consequence that we
>> can't catch the event for the first program space.
>> 
>> The second, I think minor, issue, is that GDB doesn't clean up its
>> program spaces upon exit -- or at least, they are not cleaned up
>> before Python is shut down.  As a result, any program spaces in use at
>> the time GDB exits don't generate a FreeProgspaceEvent.  I'm not
>> particularly worried about this for my use case, I'm using the event
>> to ensure that a cache doesn't hold stale entries within a single GDB
>> session.  It's also easy enough to add a Python at-exit callback which
>> can do any final cleanup if needed.
>> 
>> Finally, when testing, I did hit a slightly weird issue with some of
>> the remote boards (e.g. remote-stdio-gdbserver).  As a consequence of
>> this issue I see some output like this in the gdb.log:
>> 
>>   (gdb) PASS: gdb.python/py-progspace-events.exp: inferior 1
>>   step
>>   FreeProgspaceEvent: <gdb.Progspace object at 0x7fb7e1d19c10>
>>   warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
>>   Use the "interrupt" command to stop the target
>>   and then try again.
>>   warning: cannot close "target:/lib64/libc.so.6": Cannot execute this command while the target is running.
>>   Use the "interrupt" command to stop the target
>>   and then try again.
>>   warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Cannot execute this command while the target is running.
>>   Use the "interrupt" command to stop the target
>>   and then try again.
>>   do_parent_stuff () at py-progspace-events.c:41
>>   41        ++global_var;
>>   (gdb) PASS: gdb.python/py-progspace-events.exp: step
>> 
>> The 'FreeProgspaceEvent ...' line is expected, that's my test Python
>> extension logging the event.  What isn't expected are all the blocks
>> like:
>> 
>>   warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
>>   Use the "interrupt" command to stop the target
>>   and then try again.
>> 
>> It turns out that this has nothing to do with my changes, this is just
>> a consequence of reading files over the remote protocol.  The test
>> forks a child process which GDB stays attached too.  When the child
>> exits, GDB cleans up by calling prune_inferiors, which in turn can
>> result in GDB trying to close some files that are open because of the
>> inferior being deleted.
>> 
>> If the prune_inferiors call occurs when the remote target is
>> running (and in non-async mode) then GDB will try to send a fileio
>> packet while the remote target is waiting for a stop reply, and the
>> remote target will throw an error, see remote_target::putpkt_binary in
>> remote.c for details.
>> 
>> I'm going to look at fixing this, but, as I said, this is nothing to
>> do with this change, I just mention it because I ended up needing to
>> account for these warning messages in one of my tests, and it all
>> looks a bit weird.
>> 
>> Approved-By: Tom Tromey <tom@tromey.com>
>
> Hi Andrew,
>
> The new test fails with native-gdbserver:
>
>
>     Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.python/py-progspace-events.exp ...
>     ERROR: GDB process no longer exists
>
>     # of expected passes            9
>     # of unresolved testcases       4
>
> GDB aborts when calling error here, apparently nothing catches the
> exception so std::terminate is called:

Thanks for bringing this to my attention.  I was sure I'd tested this on
all boards (I've been trying to make use of make-check-all.sh lately),
but I guess I messed up here :-/

I will 100% look into this issue, however, it's worth nothing that this
is not something my patch broke, it's just my test exposed this issue,
which is a regression since GDB 13.

I say this only in case anyone else has any thoughts.

I will pick this up tomorrow to investigate.

Thanks,
Andrew


>
>     #4  0x000055a4a0fae18d in error (fmt=0x55a49adcd020 "Cannot execute this command while the target is running.\nUse the \"interrupt\" command to stop the target\nand then try again.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:46
>     #5  0x000055a49ec2f495 in remote_target::putpkt_binary (this=0x61b00003f080, buf=0x62d00371e400 "qXfer:auxv:read::0,1000", cnt=23) at /home/smarchi/src/binutils-gdb/gdb/remote.c:9740
>     #6  0x000055a49ec2efc4 in remote_target::putpkt (this=0x61b00003f080, buf=0x62d00371e400 "qXfer:auxv:read::0,1000") at /home/smarchi/src/binutils-gdb/gdb/remote.c:9698
>     #7  0x000055a49ec8b287 in remote_target::putpkt (this=0x61b00003f080, buf=std::__debug::vector of length 36862, capacity 36862 = {...}) at /home/smarchi/src/binutils-gdb/gdb/remote.c:1211
>     #8  0x000055a49ec45c59 in remote_target::remote_read_qxfer (this=0x61b00003f080, object_name=0x55a49adced00 "auxv", annex=0x0, readbuf=0x62100057f900 '\276' <repeats 200 times>..., offset=0, len=3904966623561266976, xfered_len=0x7f5f31c5f220, which_packet=18) at /home/smarchi/src/binutils-gdb/gdb/remote.c:11316
>     #9  0x000055a49ec47573 in remote_target::xfer_partial (this=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0, readbuf=0x62100057f900 '\276' <repeats 200 times>..., writebuf=0x0, offset=0, len=4096, xfered_len=0x7f5f31c5f220) at /home/smarchi/src/binutils-gdb/gdb/remote.c:11438
>     #10 0x000055a49f4595b1 in target_xfer_partial (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0, readbuf=0x62100057f900 '\276' <repeats 200 times>..., writebuf=0x0, offset=0, len=4096, xfered_len=0x7f5f31c5f220) at /home/smarchi/src/binutils-gdb/gdb/target.c:1717
>     #11 0x000055a49f45aed9 in target_read_partial (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0, buf=0x62100057f900 '\276' <repeats 200 times>..., offset=0, len=4096, xfered_len=0x7f5f31c5f220) at /home/smarchi/src/binutils-gdb/gdb/target.c:1951
>     #12 0x000055a49f501472 in target_read_alloc_1<unsigned char> (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0) at /home/smarchi/src/binutils-gdb/gdb/target.c:2286
>     #13 0x000055a49f45c9c2 in target_read_alloc (ops=0x61b00003f080, object=TARGET_OBJECT_AUXV, annex=0x0) at /home/smarchi/src/binutils-gdb/gdb/target.c:2315
>     #14 0x000055a49c920733 in target_read_auxv_raw (ops=0x61b00003f080) at /home/smarchi/src/binutils-gdb/gdb/auxv.c:379
>     #15 0x000055a49c920501 in target_read_auxv () at /home/smarchi/src/binutils-gdb/gdb/auxv.c:368
>     #16 0x000055a49c920c06 in target_auxv_search (match=0, valp=0x7f5f31a2fc60) at /home/smarchi/src/binutils-gdb/gdb/auxv.c:415
>     #17 0x000055a49e13e669 in linux_is_uclinux () at /home/smarchi/src/binutils-gdb/gdb/linux-tdep.c:433
>     #18 0x000055a49e13e6fa in linux_has_shared_address_space (gdbarch=0x61c00001e080) at /home/smarchi/src/binutils-gdb/gdb/linux-tdep.c:440
>     #19 0x000055a49c720d6f in gdbarch_has_shared_address_space (gdbarch=0x61c00001e080) at /home/smarchi/src/binutils-gdb/gdb/gdbarch.c:4889
>     #20 0x000055a49e7ce076 in program_space::~program_space (this=0x61300004af00, __in_chrg=<optimized out>) at /home/smarchi/src/binutils-gdb/gdb/progspace.c:124
>     #21 0x000055a49de5d91e in delete_inferior (inf=0x61800007f080) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:290
>     #22 0x000055a49de604b7 in prune_inferiors () at /home/smarchi/src/binutils-gdb/gdb/inferior.c:480
>     #23 0x000055a49debd2d9 in fetch_inferior_event () at /home/smarchi/src/binutils-gdb/gdb/infrun.c:4558
>
> Simon

Re: [PATCHv3] gdb: add Python events for program space addition and removal

[Simon Marchi]

> Thanks for bringing this to my attention.  I was sure I'd tested this on
> all boards (I've been trying to make use of make-check-all.sh lately),
> but I guess I messed up here :-/
> 
> I will 100% look into this issue, however, it's worth nothing that this
> is not something my patch broke, it's just my test exposed this issue,
> which is a regression since GDB 13.
> 
> I say this only in case anyone else has any thoughts.
> 
> I will pick this up tomorrow to investigate.
I have not investigated more than that, so I'm not sure which regression
you are referring to.  Do you mean something worked in GDB 13 and
doesn't in current master?  If so, it would be worth filing a bug (if
there isn't already) and informing Joel that this might be a blocker for
the release (not necessarily the branching).

Simon

Re: [PATCHv3] gdb: add Python events for program space addition and removal

[Andrew Burgess]

Simon Marchi <simark@simark.ca> writes:

>> Thanks for bringing this to my attention.  I was sure I'd tested this on
>> all boards (I've been trying to make use of make-check-all.sh lately),
>> but I guess I messed up here :-/
>> 
>> I will 100% look into this issue, however, it's worth nothing that this
>> is not something my patch broke, it's just my test exposed this issue,
>> which is a regression since GDB 13.
>> 
>> I say this only in case anyone else has any thoughts.
>> 
>> I will pick this up tomorrow to investigate.
> I have not investigated more than that, so I'm not sure which regression
> you are referring to.  Do you mean something worked in GDB 13 and
> doesn't in current master?  If so, it would be worth filing a bug (if
> there isn't already) and informing Joel that this might be a blocker for
> the release (not necessarily the branching).

Sorry for not being clear -- it's getting late here.

Yes, the issue you reported is not something I broke (with this commit
at least), but is something that is broken that was exposed by the new
test.

When I run the same test on GDB 13 I don't see GDB crash, so I think
this is a regression.

I created: https://sourceware.org/bugzilla/show_bug.cgi?id=30935

And I've set this as 14.1 for now, until we understand what's going on
here more.

Thanks,
Andrew