[PATCH v5 0/2] Make sure autoload happens before notifying Python side in new_objfile event

[PATCH v5 0/2] Make sure autoload happens before notifying Python side in new_objfile event

[Simon Marchi]

This is a new version of Michael Weghorn's patchset here:

https://sourceware.org/pipermail/gdb-patches/2021-April/178069.html

I rebased it after merging my "set debug observer" changes.

In addition to just rebasing, I made many style fixes here and there.  I
also did some changes in the test to use some more recent GDB testsuite
idioms (that you would miss if you copied an older test, and that I
wouldn't expect a new-ish contributor to know).

I also changed the test to make it work for the native-gdbserver board.
Previously, the test checked the output of the "run" command.  This
doesn't work for native-gdbserver, because GDB is connected using the
"remote" protocol, which doesn't support running.  Instead, I made the
new_objfile handler set a global variable if everything looks good.  We
then check the global variable value in the test.

Michael, can you please take a look and check it is still ok with you?

Michael Weghorn (2):
  gdbsupport: allow to specify dependencies between observers
  gdb: do autoload before notifying Python side in new_objfile event

 gdb/auto-load.c                               |   9 +-
 gdb/auto-load.h                               |   8 ++
 gdb/python/py-inferior.c                      |   7 +-
 ...tty-printers-in-newobjfile-event.so-gdb.py |  43 +++++++
 ...pretty-printers-in-newobjfile-event-lib.cc |  28 +++++
 ...-pretty-printers-in-newobjfile-event-lib.h |  31 +++++
 ...retty-printers-in-newobjfile-event-main.cc |  27 +++++
 ...ed-pretty-printers-in-newobjfile-event.exp |  85 +++++++++++++
 ...ded-pretty-printers-in-newobjfile-event.py |  50 ++++++++
 gdb/unittests/observable-selftests.c          | 112 ++++++++++++++++++
 gdbsupport/observable.h                       | 111 ++++++++++++++---
 11 files changed, 492 insertions(+), 19 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py

-- 
2.30.1

[PATCH v5 1/2] gdbsupport: allow to specify dependencies between observers

[Simon Marchi]

From: Michael Weghorn <m.weghorn@posteo.de>

Previously, the observers attached to an observable were always notified
in the order in which they had been attached.  That order is not easily
controlled, because observers are typically attached in _initialize_*
functions, we are called in an undefined order.

However, an observer may require that another observer attached only
later is called before itself is.

Therefore, extend the 'observable' class to allow explicitly specifying
dependencies when attaching observers, by adding the possibility to
specify tokens for observers that it depends on.

To make sure dependencies are notified before observers depending on
them, the vector holding the observers is sorted in a way that
dependencies come before observers depending on them.  The current
implementation for sorting uses the depth-first search algorithm for
topological sorting as described at [1].

Extend the observable unit tests to cover this case as well.  Check that
this works for a few different orders in which the observers are
attached.

This newly introduced mechanism to explicitly specify dependencies will
be used in a follow-up commit.

[1] https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search

Tested on x86_64-linux (Debian testing).

gdb/ChangeLog:

	* unittests/observable-selftests.c (dependency_test_counters):
	New.
	(observer_token0, observer_token1, observer_token2,
	observer_token3, observer_token4, observer_token5): New.
	(struct dependency_observer_data): New struct.
	(observer_dependency_test_callback): New function.
	(test_observers): New.
	(run_dependency_test): New function.
	(test_dependency): New.
	(_initialize_observer_selftest): Register dependency test.

gdbsupport/ChangeLog:

	* observable.h (class observable): Extend to allow specifying
	dependencies between observers, keep vector holding observers
	sorted so that dependencies are notified before observers
	depending on them.

Change-Id: I5399def1eeb69ca99e28c9f1fdf321d78b530bdb
---
 gdb/unittests/observable-selftests.c | 112 +++++++++++++++++++++++++++
 gdbsupport/observable.h              | 111 ++++++++++++++++++++++----
 2 files changed, 207 insertions(+), 16 deletions(-)

diff --git a/gdb/unittests/observable-selftests.c b/gdb/unittests/observable-selftests.c
index 0e3b53f555b0..88b009f1188d 100644
--- a/gdb/unittests/observable-selftests.c
+++ b/gdb/unittests/observable-selftests.c
@@ -30,6 +30,60 @@ static int test_first_observer = 0;
 static int test_second_observer = 0;
 static int test_third_observer = 0;
 
+/* Counters for observers used for dependency tests.  */
+static std::vector<int> dependency_test_counters;
+
+/* Tokens for observers used for dependency tests.  */
+static gdb::observers::token observer_token0;
+static gdb::observers::token observer_token1;
+static gdb::observers::token observer_token2;
+static gdb::observers::token observer_token3;
+static gdb::observers::token observer_token4;
+static gdb::observers::token observer_token5;
+
+/* Data for one observer used for checking that dependencies work as expected;
+   dependencies are specified using their indices into the 'test_observers'
+   vector here for simplicity and mapped to corresponding tokens later.  */
+struct dependency_observer_data
+{
+  gdb::observers::token *token;
+
+  /* Name of the observer to use on attach.  */
+  const char *name;
+
+  /* Indices of observers that this one directly depends on.  */
+  std::vector<int> direct_dependencies;
+
+  /* Indices for all dependencies, including transitive ones.  */
+  std::vector<int> all_dependencies;
+
+  /* Function to attach to the observable for this observer.  */
+  std::function<void (int)> callback;
+};
+
+static void observer_dependency_test_callback (size_t index);
+
+/* Data for observers to use for dependency tests, using some sample
+   dependencies between the observers.  */
+static std::vector<dependency_observer_data> test_observers = {
+  {&observer_token0, "test0", {}, {},
+   [] (int) { observer_dependency_test_callback (0); }},
+  {&observer_token1, "test1", {0}, {0},
+   [] (int) { observer_dependency_test_callback (1); }},
+  {&observer_token2, "test2", {1}, {0, 1},
+   [] (int) { observer_dependency_test_callback (2); }},
+  {&observer_token3, "test3", {1}, {0, 1},
+   [] (int) { observer_dependency_test_callback (3); }},
+  {&observer_token4, "test4", {2, 3, 5}, {0, 1, 2, 3, 5},
+   [] (int) { observer_dependency_test_callback (4); }},
+  {&observer_token5, "test5", {0}, {0},
+   [] (int) { observer_dependency_test_callback (5); }},
+  {nullptr, "test6", {4}, {0, 1, 2, 3, 4, 5},
+   [] (int) { observer_dependency_test_callback (6); }},
+  {nullptr, "test7", {0}, {0},
+   [] (int) { observer_dependency_test_callback (7); }},
+};
+
 static void
 test_first_notification_function (int arg)
 {
@@ -63,6 +117,62 @@ notify_check_counters (int one, int two, int three)
   SELF_CHECK (three == test_third_observer);
 }
 
+/* Function for each observer to run when being notified during the
+   dependency tests. Verifies that dependencies have been notified earlier
+   by checking their counters, then increases the own counter.  */
+static void
+observer_dependency_test_callback (size_t index)
+{
+  /* Check that dependencies have already been notified.  */
+  for (int i : test_observers[index].all_dependencies)
+    SELF_CHECK (dependency_test_counters[i] == 1);
+
+  /* Increase own counter.  */
+  dependency_test_counters[index]++;
+}
+
+/* Run a dependency test by attaching the observers in the specified order
+   then notifying them.  */
+static void
+run_dependency_test (std::vector<int> insertion_order)
+{
+  gdb::observers::observable<int> dependency_test_notification
+    ("dependency_test_notification");
+
+  /* Reset counters.  */
+  dependency_test_counters = std::vector<int> (test_observers.size (), 0);
+
+  /* Attach all observers in the given order, specifying dependencies.  */
+  for (int i : insertion_order)
+    {
+      const dependency_observer_data &o = test_observers[i];
+
+      /* Get tokens for dependencies using their indices.  */
+      std::vector<const gdb::observers::token *> dependency_tokens;
+      for (int index : o.all_dependencies)
+	dependency_tokens.emplace_back (test_observers[index].token);
+
+      if (o.token != nullptr)
+	dependency_test_notification.attach
+	  (o.callback, *o.token, o.name, dependency_tokens);
+      else
+	dependency_test_notification.attach (o.callback, o.name,
+					     dependency_tokens);
+    }
+
+  /* Notify observers, they check that their dependencies were notified.  */
+  dependency_test_notification.notify (1);
+}
+
+static void
+test_dependency ()
+{
+  /* Run dependency tests with different insertion orders.  */
+  run_dependency_test ({0, 1, 2, 3, 4, 5, 6, 7});
+  run_dependency_test ({7, 6, 5, 4, 3, 2, 1, 0});
+  run_dependency_test ({0, 3, 2, 1, 7, 6, 4, 5});
+}
+
 static void
 run_tests ()
 {
@@ -133,4 +243,6 @@ _initialize_observer_selftest ()
 {
   selftests::register_test ("gdb::observers",
 			    selftests::observers::run_tests);
+  selftests::register_test ("gdb::observers dependency",
+			    selftests::observers::test_dependency);
 }
diff --git a/gdbsupport/observable.h b/gdbsupport/observable.h
index 4ba47bb988f4..648158c8ca23 100644
--- a/gdbsupport/observable.h
+++ b/gdbsupport/observable.h
@@ -71,13 +71,15 @@ class observable
 private:
   struct observer
   {
-    observer (const struct token *token, func_type func, const char *name)
-      : token (token), func (func), name (name)
+    observer (const struct token *token, func_type func, const char *name,
+	      const std::vector<const struct token *> &dependencies)
+      : token (token), func (func), name (name), dependencies (dependencies)
     {}
 
     const struct token *token;
     func_type func;
     const char *name;
+    std::vector<const struct token *> dependencies;
   };
 
 public:
@@ -88,30 +90,29 @@ class observable
 
   DISABLE_COPY_AND_ASSIGN (observable);
 
-  /* Attach F as an observer to this observable.  F cannot be
-     detached.
+  /* Attach F as an observer to this observable.  F cannot be detached or
+     specified as a dependency.
 
      NAME is the name of the observer, used for debug output purposes.  Its
      lifetime must be at least as long as the observer is attached.  */
-  void attach (const func_type &f, const char *name)
+  void attach (const func_type &f, const char *name,
+	       const std::vector<const struct token *> &dependencies = {})
   {
-    observer_debug_printf ("Attaching observable %s to observer %s",
-			   name, m_name);
-
-    m_observers.emplace_back (nullptr, f, name);
+    attach (f, nullptr, name, dependencies);
   }
 
-  /* Attach F as an observer to this observable.  T is a reference to
-     a token that can be used to later remove F.
+  /* Attach F as an observer to this observable.
+
+     T is a reference to a token that can be used to later remove F or specify F
+     as a dependency of another observer.  Dependencies are notified before the
+     observer depending on them.
 
      NAME is the name of the observer, used for debug output purposes.  Its
      lifetime must be at least as long as the observer is attached.  */
-  void attach (const func_type &f, const token &t, const char *name)
+  void attach (const func_type &f, const token &t, const char *name,
+	       const std::vector<const struct token *> &dependencies = {})
   {
-    observer_debug_printf ("Attaching observable %s to observer %s",
-			   name, m_name);
-
-    m_observers.emplace_back (&t, f, name);
+    attach (f, &t, name, dependencies);
   }
 
   /* Remove observers associated with T from this observable.  T is
@@ -149,6 +150,84 @@ class observable
 
   std::vector<observer> m_observers;
   const char *m_name;
+
+  /* Use for sorting algorithm, to indicate which observer we have visited.  */
+  enum class visit_state
+  {
+    NOT_VISITED,
+    VISITING,
+    VISITED,
+  };
+
+  /* Helper method for topological sort using depth-first search algorithm.
+
+     Visit all dependencies of observer at INDEX in M_OBSERVERS (later referred
+     to as "the observer").  Then append the observer to SORTED_OBSERVERS.
+
+     If the observer is already visited, do nothing.  */
+  void visit_for_sorting (std::vector<observer> &sorted_observers,
+                          std::vector<visit_state> &visit_states, int index)
+  {
+    if (visit_states[index] == visit_state::VISITED)
+      return;
+
+    /* If we are already visiting this observer, it means there's a cycle.  */
+    gdb_assert (visit_states[index] != visit_state::VISITING);
+
+    visit_states[index] = visit_state::VISITING;
+
+    /* For each dependency of this observer...  */
+    for (const token *dep : m_observers[index].dependencies)
+      {
+	/* ... find the observer that has token DEP.  If found, visit it.  */
+        auto it_dep
+            = std::find_if (m_observers.begin (), m_observers.end (),
+                            [&] (observer o) { return o.token == dep; });
+        if (it_dep != m_observers.end ())
+          {
+            int i = std::distance (m_observers.begin (), it_dep);
+            visit_for_sorting (sorted_observers, visit_states, i);
+          }
+      }
+
+    visit_states[index] = visit_state::VISITED;
+    sorted_observers.push_back (m_observers[index]);
+  }
+
+  /* Sort the observers, so that dependencies come before observers
+     depending on them.
+
+     Uses depth-first search algorithm for topological sorting, see
+     https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search .  */
+  void sort_observers ()
+  {
+    std::vector<observer> sorted_observers;
+    std::vector<visit_state> visit_states (m_observers.size (),
+					   visit_state::NOT_VISITED);
+
+    for (size_t i = 0; i < m_observers.size (); i++)
+      visit_for_sorting (sorted_observers, visit_states, i);
+
+    m_observers = std::move (sorted_observers);
+  }
+
+  void attach (const func_type &f, const token *t, const char *name,
+               const std::vector<const struct token *> &dependencies)
+  {
+
+    observer_debug_printf ("Attaching observable %s to observer %s",
+                           name, m_name);
+
+    m_observers.emplace_back (t, f, name, dependencies);
+
+    /* The observer has been inserted at the end of the vector, so it will be
+       after any of its potential dependencies attached earlier.  If the
+       observer has a token, it means that other observers can specify it as
+       a dependency, so sorting is necessary to ensure those will be after the
+       newly inserted observer afterwards.  */
+    if (t != nullptr)
+      sort_observers ();
+  };
 };
 
 } /* namespace observers */
-- 
2.30.1

[PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Simon Marchi]

From: Michael Weghorn <m.weghorn@posteo.de>

Without any explicit dependencies specified, the observers attached
to the 'gdb::observers::new_objfile' observable are always notified
in the order in which they have been attached.

The new_objfile observer callback to auto-load scripts is attached in
'_initialize_auto_load'.
The new_objfile observer callback that propagates the new_objfile event
to the Python side is attached in 'gdbpy_initialize_inferior', which is
called via '_initialize_python'.
With '_initialize_python' happening before '_initialize_auto_load',
the consequence was that the new_objfile event was emitted on the Python
side before autoloaded scripts had been executed when a new objfile was
loaded.
As a result, trying to access the objfile's pretty printers (defined in
the autoloaded script) from a handler for the Python-side
'new_objfile' event would fail. Those would only be initialized later on
(when the 'auto_load_new_objfile' callback was called).

To make sure that the objfile passed to the Python event handler
is properly initialized (including its 'pretty_printers' member),
make sure that the 'auto_load_new_objfile' observer is notified
before the 'python_new_objfile' one that propagates the event
to the Python side.

To do this, make use of the mechanism to explicitly specify
dependencies between observers (introduced in a preparatory commit).

Add a corresponding testcase that involves a test library with an autoloaded
Python script and a handler for the Python 'new_objfile' event.

(The real world use case where I came across this issue was in an attempt
to extend handling for GDB pretty printers for dynamically loaded
objfiles in the Qt Creator IDE, s. [1] and [2] for more background.)

[1] https://bugreports.qt.io/browse/QTCREATORBUG-25339
[2] https://codereview.qt-project.org/c/qt-creator/qt-creator/+/333857/1

Tested on x86_64-linux (Debian testing).

gdb/ChangeLog:

        * gdb/auto-load.c (_initialize_auto_load): 'Specify token
        when attaching the 'auto_load_new_objfile' observer, so
        other observers can specify it as a dependency.

        * gdb/auto-load.h (struct token): Declare
        'auto_load_new_objfile_observer_token' as token to be used
        for the 'auto_load_new_objfile' observer.
        * gdb/python/py-inferior.c (gdbpy_initialize_inferior): Make
        'python_new_objfile' observer depend on 'auto_load_new_objfile'
        observer, so it gets notified after the latter.

gdb/testsuite/ChangeLog:

        * gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py: New test.
        * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc: New test.
        * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h: New test.
        * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc: New test.
        * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp: New test.
        * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py: New test.

Change-Id: I8275b3f4c3bec32e56dd7892f9a59d89544edf89
---
 gdb/auto-load.c                               |  9 +-
 gdb/auto-load.h                               |  8 ++
 gdb/python/py-inferior.c                      |  7 +-
 ...tty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++
 ...pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++
 ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
 ...retty-printers-in-newobjfile-event-main.cc | 27 ++++++
 ...ed-pretty-printers-in-newobjfile-event.exp | 85 +++++++++++++++++++
 ...ded-pretty-printers-in-newobjfile-event.py | 50 +++++++++++
 9 files changed, 285 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
 create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 239efa346064..d1ae6deacee7 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1494,6 +1494,10 @@ found and/or loaded."),
   return &retval;
 }
 
+/* See auto-load.h.  */
+
+gdb::observers::token auto_load_new_objfile_observer_token;
+
 void _initialize_auto_load ();
 void
 _initialize_auto_load ()
@@ -1503,8 +1507,9 @@ _initialize_auto_load ()
   char *guile_name_help;
   const char *suffix;
 
-  gdb::observers::new_objfile.attach (auto_load_new_objfile, "auto-load");
-
+  gdb::observers::new_objfile.attach (auto_load_new_objfile,
+                                      auto_load_new_objfile_observer_token,
+                                      "auto-load");
   add_setshow_boolean_cmd ("gdb-scripts", class_support,
 			   &auto_load_gdb_scripts, _("\
 Enable or disable auto-loading of canned sequences of commands scripts."), _("\
diff --git a/gdb/auto-load.h b/gdb/auto-load.h
index f726126c5541..4372ec4f4dd7 100644
--- a/gdb/auto-load.h
+++ b/gdb/auto-load.h
@@ -25,6 +25,10 @@ struct program_space;
 struct auto_load_pspace_info;
 struct extension_language_defn;
 
+namespace gdb::observers {
+struct token;
+}
+
 /* Value of the 'set debug auto-load' configuration variable.  */
 
 extern bool debug_auto_load;
@@ -40,6 +44,10 @@ extern bool auto_load_local_gdbinit;
 extern char *auto_load_local_gdbinit_pathname;
 extern bool auto_load_local_gdbinit_loaded;
 
+/* Token used for the auto_load_new_objfile observer, so other observers can
+   specify it as a dependency. */
+extern gdb::observers::token auto_load_new_objfile_observer_token;
+
 extern struct auto_load_pspace_info *
   get_auto_load_pspace_data_for_loading (struct program_space *pspace);
 extern void auto_load_objfile_script (struct objfile *objfile,
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index c2861ccb735c..febd2a73ece3 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "auto-load.h"
 #include "gdbcore.h"
 #include "gdbthread.h"
 #include "inferior.h"
@@ -917,7 +918,11 @@ gdbpy_initialize_inferior (void)
   gdb::observers::register_changed.attach (python_on_register_change,
 					   "py-inferior");
   gdb::observers::inferior_exit.attach (python_inferior_exit, "py-inferior");
-  gdb::observers::new_objfile.attach (python_new_objfile, "py-inferior");
+  /* Need to run after auto-load's new_objfile observer, so that
+     auto-loaded pretty-printers are available.  */
+  gdb::observers::new_objfile.attach
+    (python_new_objfile, "py-inferior",
+     { &auto_load_new_objfile_observer_token });
   gdb::observers::inferior_added.attach (python_new_inferior, "py-inferior");
   gdb::observers::inferior_removed.attach (python_inferior_deleted,
 					   "py-inferior");
diff --git a/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
new file mode 100644
index 000000000000..aeb39a6c483a
--- /dev/null
+++ b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
@@ -0,0 +1,43 @@
+# Copyright (C) 2021 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 that python pretty
+# printers defined in a python script that is autoloaded have been
+# registered when a custom event handler for the new_objfile event
+# is called.
+
+import gdb.printing
+
+
+class MyClassTestLibPrinter(object):
+    "Print a MyClassTestLib"
+
+    def __init__(self, val):
+        self.val = val
+
+    def to_string(self):
+        return "MyClassTestLib object, id: {}".format(self.val["id"])
+
+    def display_hint(self):
+        return "string"
+
+
+def build_pretty_printer():
+    pp = gdb.printing.RegexpCollectionPrettyPrinter("my_library")
+    pp.add_printer("MyClassTestLib", "^MyClassTestLib$", MyClassTestLibPrinter)
+    return pp
+
+
+gdb.printing.register_pretty_printer(gdb.current_objfile(), build_pretty_printer())
diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
new file mode 100644
index 000000000000..7f13cd2b741e
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
+
+MyClassTestLib::MyClassTestLib (int theId)
+{
+  id = theId;
+}
+
+int MyClassTestLib::getId ()
+{
+  return id;
+}
diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
new file mode 100644
index 000000000000..3714ecd2ef08
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+#ifndef TESTLIBRARY_H
+#define TESTLIBRARY_H
+
+class MyClassTestLib
+{
+public:
+  explicit MyClassTestLib (int theId);
+  int getId ();
+
+private:
+  int id;
+};
+
+#endif /* TESTLIBRARY_H */
diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
new file mode 100644
index 000000000000..2cc89a3befd5
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
+
+bool all_good = false;
+
+int
+main ()
+{
+  MyClassTestLib test (1);
+  return 0; /* break to inspect */
+}
diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
new file mode 100644
index 000000000000..444466109e8f
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
@@ -0,0 +1,85 @@
+# Copyright (C) 2021 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 that Python pretty-printers
+# defined in a Python script that is autoloaded are registered when an event
+# handler for the new_objfile event is called.
+
+load_lib gdb-python.exp
+
+standard_testfile -main.cc
+
+set srcfile_lib "${testfile}-lib.cc"
+set python_event_handler_file "${srcdir}/${subdir}/${testfile}.py"
+set libname "lib${testfile}"
+set python_autoload_file "${srcdir}/${subdir}/${libname}.so-gdb.py"
+set binfile_lib [standard_output_file "${libname}.so"]
+
+# Start GDB first - needed for skip_python_tests.
+clean_restart
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+# Compile library.
+if { [gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} \
+      {debug c++}] != "" } {
+    return -1
+}
+
+# Compile main program.
+if { [gdb_compile ${srcdir}/${subdir}/${srcfile} \
+      ${binfile} \
+      executable \
+      [list debug c++ shlib=$binfile_lib]] != "" } {
+    return -1
+}
+
+# Make the -gdb.py script available to gdb, it is automatically loaded by
+# gdb if it is put in the same directory as the library.
+set remote_python_autoload_file \
+    [gdb_remote_download host $python_autoload_file]
+
+gdb_test_no_output \
+    "set auto-load safe-path ${remote_python_autoload_file}" \
+    "set auto-load safe-path"
+
+# Load the Python file that defines a handler for the new_objfile event,
+# which will generate the output to check later
+# (prints information on available pretty-printers for objfile).
+set remote_python_event_handler_file\
+    [gdb_remote_download host $python_event_handler_file]
+gdb_test_no_output "source ${remote_python_event_handler_file}" "load python file"
+
+gdb_load ${binfile}
+
+gdb_test_no_output "set print pretty on"
+
+# Check that the handler prints output when test library is loaded
+# and that the pretty-printer from the auto-loaded Python file has been
+# registered.
+if { ![runto_main] } {
+    fail "failed to run to main"
+    return
+}
+
+# Check that the new_objfile handler saw the pretty-printer.
+gdb_test "print all_good" " = true"
+
+# Check that the pretty-printer actually works.
+gdb_test "info pretty-printer" "my_library.*MyClassTestLib.*"
+gdb_breakpoint [gdb_get_line_number "break to inspect"]
+gdb_test "continue" "Breakpoint $decimal, main .*"
+gdb_test "print test" "MyClassTestLib object, id: 1.*"
diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
new file mode 100644
index 000000000000..85d60fc51c31
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
@@ -0,0 +1,50 @@
+# Copyright (C) 2021 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 that python pretty
+# printers defined in a python script that is autoloaded have been
+# registered when a custom event handler for the new_objfile event
+# is called.
+
+import gdb
+import os
+
+
+def new_objfile_handler(event):
+    assert isinstance(event, gdb.NewObjFileEvent)
+    objfile = event.new_objfile
+
+    # Only observe the custom test library.
+    libname = "libpy-autoloaded-pretty-printers-in-newobjfile-event"
+    if libname in os.path.basename(objfile.filename):
+        # If everything went well and the pretty-printer auto-load happened
+        # before notifying the Python listeners, we expect to see one pretty
+        # printer, and it must be ours.
+        all_good = (
+            len(objfile.pretty_printers) == 1
+            and objfile.pretty_printers[0].name == "my_library"
+        )
+
+        if all_good:
+            gdb.parse_and_eval("all_good = true")
+        else:
+            print("Oops, not all good:")
+            print("pretty printer count: {}".format(len(objfile.pretty_printers)))
+
+            for pp in objfile.pretty_printers:
+                print("  - {}".format(pp.name))
+
+
+gdb.events.new_objfile.connect(new_objfile_handler)
-- 
2.30.1

Re: [PATCH v5 1/2] gdbsupport: allow to specify dependencies between observers

[Michael Weghorn]

[-- Attachment #1.1: Type: text/plain, Size: 1521 bytes --]

On 26/04/2021 16.53, Simon Marchi wrote:
> From: Michael Weghorn <m.weghorn@posteo.de>
> 
> Previously, the observers attached to an observable were always notified
> in the order in which they had been attached.  That order is not easily
> controlled, because observers are typically attached in _initialize_*
> functions, we are called in an undefined order.

Just a nit-pick, should that read "functions, WHICH are called in an 
undefined order."?

> 
> However, an observer may require that another observer attached only
> later is called before itself is.
> 
> Therefore, extend the 'observable' class to allow explicitly specifying
> dependencies when attaching observers, by adding the possibility to
> specify tokens for observers that it depends on.
> 
> To make sure dependencies are notified before observers depending on
> them, the vector holding the observers is sorted in a way that
> dependencies come before observers depending on them.  The current
> implementation for sorting uses the depth-first search algorithm for
> topological sorting as described at [1].
> 
> Extend the observable unit tests to cover this case as well.  Check that
> this works for a few different orders in which the observers are
> attached.
> 
> This newly introduced mechanism to explicitly specify dependencies will
> be used in a follow-up commit.
> 
> [1] https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search
> 
> Tested on x86_64-linux (Debian testing).

Michael


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Michael Weghorn]

[-- Attachment #1.1: Type: text/plain, Size: 20741 bytes --]

On 26/04/2021 16.53, Simon Marchi wrote:
> From: Michael Weghorn <m.weghorn@posteo.de>
> 
> Without any explicit dependencies specified, the observers attached
> to the 'gdb::observers::new_objfile' observable are always notified
> in the order in which they have been attached.
> 
> The new_objfile observer callback to auto-load scripts is attached in
> '_initialize_auto_load'.
> The new_objfile observer callback that propagates the new_objfile event
> to the Python side is attached in 'gdbpy_initialize_inferior', which is
> called via '_initialize_python'.
> With '_initialize_python' happening before '_initialize_auto_load',
> the consequence was that the new_objfile event was emitted on the Python
> side before autoloaded scripts had been executed when a new objfile was
> loaded.
> As a result, trying to access the objfile's pretty printers (defined in
> the autoloaded script) from a handler for the Python-side
> 'new_objfile' event would fail. Those would only be initialized later on
> (when the 'auto_load_new_objfile' callback was called).
> 
> To make sure that the objfile passed to the Python event handler
> is properly initialized (including its 'pretty_printers' member),
> make sure that the 'auto_load_new_objfile' observer is notified
> before the 'python_new_objfile' one that propagates the event
> to the Python side.
> 
> To do this, make use of the mechanism to explicitly specify
> dependencies between observers (introduced in a preparatory commit).
> 
> Add a corresponding testcase that involves a test library with an autoloaded
> Python script and a handler for the Python 'new_objfile' event.
> 
> (The real world use case where I came across this issue was in an attempt
> to extend handling for GDB pretty printers for dynamically loaded
> objfiles in the Qt Creator IDE, s. [1] and [2] for more background.)
> 
> [1] https://bugreports.qt.io/browse/QTCREATORBUG-25339
> [2] https://codereview.qt-project.org/c/qt-creator/qt-creator/+/333857/1
> 
> Tested on x86_64-linux (Debian testing).
> 
> gdb/ChangeLog:
> 
>          * gdb/auto-load.c (_initialize_auto_load): 'Specify token
>          when attaching the 'auto_load_new_objfile' observer, so
>          other observers can specify it as a dependency.
> 
>          * gdb/auto-load.h (struct token): Declare
>          'auto_load_new_objfile_observer_token' as token to be used
>          for the 'auto_load_new_objfile' observer.
>          * gdb/python/py-inferior.c (gdbpy_initialize_inferior): Make
>          'python_new_objfile' observer depend on 'auto_load_new_objfile'
>          observer, so it gets notified after the latter.
> 
> gdb/testsuite/ChangeLog:
> 
>          * gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py: New test.
>          * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc: New test.
>          * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h: New test.
>          * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc: New test.
>          * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp: New test.
>          * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py: New test.
> 
> Change-Id: I8275b3f4c3bec32e56dd7892f9a59d89544edf89
> ---
>   gdb/auto-load.c                               |  9 +-
>   gdb/auto-load.h                               |  8 ++
>   gdb/python/py-inferior.c                      |  7 +-
>   ...tty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++
>   ...pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++
>   ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
>   ...retty-printers-in-newobjfile-event-main.cc | 27 ++++++
>   ...ed-pretty-printers-in-newobjfile-event.exp | 85 +++++++++++++++++++
>   ...ded-pretty-printers-in-newobjfile-event.py | 50 +++++++++++
>   9 files changed, 285 insertions(+), 3 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
>   create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
>   create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
>   create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
>   create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
>   create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> 
> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> index 239efa346064..d1ae6deacee7 100644
> --- a/gdb/auto-load.c
> +++ b/gdb/auto-load.c
> @@ -1494,6 +1494,10 @@ found and/or loaded."),
>     return &retval;
>   }
>   
> +/* See auto-load.h.  */
> +
> +gdb::observers::token auto_load_new_objfile_observer_token;
> +
>   void _initialize_auto_load ();
>   void
>   _initialize_auto_load ()
> @@ -1503,8 +1507,9 @@ _initialize_auto_load ()
>     char *guile_name_help;
>     const char *suffix;
>   
> -  gdb::observers::new_objfile.attach (auto_load_new_objfile, "auto-load");
> -
> +  gdb::observers::new_objfile.attach (auto_load_new_objfile,
> +                                      auto_load_new_objfile_observer_token,
> +                                      "auto-load");
>     add_setshow_boolean_cmd ("gdb-scripts", class_support,
>   			   &auto_load_gdb_scripts, _("\
>   Enable or disable auto-loading of canned sequences of commands scripts."), _("\
> diff --git a/gdb/auto-load.h b/gdb/auto-load.h
> index f726126c5541..4372ec4f4dd7 100644
> --- a/gdb/auto-load.h
> +++ b/gdb/auto-load.h
> @@ -25,6 +25,10 @@ struct program_space;
>   struct auto_load_pspace_info;
>   struct extension_language_defn;
>   
> +namespace gdb::observers {
> +struct token;
> +}
> +
>   /* Value of the 'set debug auto-load' configuration variable.  */
>   
>   extern bool debug_auto_load;
> @@ -40,6 +44,10 @@ extern bool auto_load_local_gdbinit;
>   extern char *auto_load_local_gdbinit_pathname;
>   extern bool auto_load_local_gdbinit_loaded;
>   
> +/* Token used for the auto_load_new_objfile observer, so other observers can
> +   specify it as a dependency. */
> +extern gdb::observers::token auto_load_new_objfile_observer_token;
> +
>   extern struct auto_load_pspace_info *
>     get_auto_load_pspace_data_for_loading (struct program_space *pspace);
>   extern void auto_load_objfile_script (struct objfile *objfile,
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index c2861ccb735c..febd2a73ece3 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -18,6 +18,7 @@
>      along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>   
>   #include "defs.h"
> +#include "auto-load.h"
>   #include "gdbcore.h"
>   #include "gdbthread.h"
>   #include "inferior.h"
> @@ -917,7 +918,11 @@ gdbpy_initialize_inferior (void)
>     gdb::observers::register_changed.attach (python_on_register_change,
>   					   "py-inferior");
>     gdb::observers::inferior_exit.attach (python_inferior_exit, "py-inferior");
> -  gdb::observers::new_objfile.attach (python_new_objfile, "py-inferior");
> +  /* Need to run after auto-load's new_objfile observer, so that
> +     auto-loaded pretty-printers are available.  */
> +  gdb::observers::new_objfile.attach
> +    (python_new_objfile, "py-inferior",
> +     { &auto_load_new_objfile_observer_token });
>     gdb::observers::inferior_added.attach (python_new_inferior, "py-inferior");
>     gdb::observers::inferior_removed.attach (python_inferior_deleted,
>   					   "py-inferior");
> diff --git a/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
> new file mode 100644
> index 000000000000..aeb39a6c483a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
> @@ -0,0 +1,43 @@
> +# Copyright (C) 2021 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 that python pretty
> +# printers defined in a python script that is autoloaded have been
> +# registered when a custom event handler for the new_objfile event
> +# is called.
> +
> +import gdb.printing
> +
> +
> +class MyClassTestLibPrinter(object):
> +    "Print a MyClassTestLib"
> +
> +    def __init__(self, val):
> +        self.val = val
> +
> +    def to_string(self):
> +        return "MyClassTestLib object, id: {}".format(self.val["id"])
> +
> +    def display_hint(self):
> +        return "string"
> +
> +
> +def build_pretty_printer():
> +    pp = gdb.printing.RegexpCollectionPrettyPrinter("my_library")
> +    pp.add_printer("MyClassTestLib", "^MyClassTestLib$", MyClassTestLibPrinter)
> +    return pp
> +
> +
> +gdb.printing.register_pretty_printer(gdb.current_objfile(), build_pretty_printer())
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
> new file mode 100644
> index 000000000000..7f13cd2b741e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
> @@ -0,0 +1,28 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
> +
> +MyClassTestLib::MyClassTestLib (int theId)
> +{
> +  id = theId;
> +}
> +
> +int MyClassTestLib::getId ()
> +{
> +  return id;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
> new file mode 100644
> index 000000000000..3714ecd2ef08
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
> @@ -0,0 +1,31 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +#ifndef TESTLIBRARY_H
> +#define TESTLIBRARY_H
> +
> +class MyClassTestLib
> +{
> +public:
> +  explicit MyClassTestLib (int theId);
> +  int getId ();
> +
> +private:
> +  int id;
> +};
> +
> +#endif /* TESTLIBRARY_H */
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
> new file mode 100644
> index 000000000000..2cc89a3befd5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
> @@ -0,0 +1,27 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
> +
> +bool all_good = false;
> +
> +int
> +main ()
> +{
> +  MyClassTestLib test (1);
> +  return 0; /* break to inspect */
> +}
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
> new file mode 100644
> index 000000000000..444466109e8f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
> @@ -0,0 +1,85 @@
> +# Copyright (C) 2021 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 that Python pretty-printers
> +# defined in a Python script that is autoloaded are registered when an 
event
> +# handler for the new_objfile event is called.
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile -main.cc
> +
> +set srcfile_lib "${testfile}-lib.cc"
> +set python_event_handler_file "${srcdir}/${subdir}/${testfile}.py"
> +set libname "lib${testfile}"
> +set python_autoload_file "${srcdir}/${subdir}/${libname}.so-gdb.py"
> +set binfile_lib [standard_output_file "${libname}.so"]
> +
> +# Start GDB first - needed for skip_python_tests.
> +clean_restart
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +# Compile library.
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} \
> +      {debug c++}] != "" } {
> +    return -1
> +}
> +
> +# Compile main program.
> +if { [gdb_compile ${srcdir}/${subdir}/${srcfile} \
> +      ${binfile} \
> +      executable \
> +      [list debug c++ shlib=$binfile_lib]] != "" } {
> +    return -1
> +}
> +
> +# Make the -gdb.py script available to gdb, it is automatically loaded 
by
> +# gdb if it is put in the same directory as the library.
> +set remote_python_autoload_file \
> +    [gdb_remote_download host $python_autoload_file]
> +
> +gdb_test_no_output \
> +    "set auto-load safe-path ${remote_python_autoload_file}" \
> +    "set auto-load safe-path"
> +
> +# Load the Python file that defines a handler for the new_objfile event,
> +# which will generate the output to check later
> +# (prints information on available pretty-printers for objfile).

In v5, the handler no longer generates output (for the good case), so 
maybe this comment should be changed to something like this:

# Load the Python file that defines a handler for the new_objfile event,
# which will set a global variable if the pretty-printer is available.

> +set remote_python_event_handler_file\
> +    [gdb_remote_download host $python_event_handler_file]
> +gdb_test_no_output "source ${remote_python_event_handler_file}" "load python file"
> +
> +gdb_load ${binfile}
> +
> +gdb_test_no_output "set print pretty on"
> +
> +# Check that the handler prints output when test library is loaded
> +# and that the pretty-printer from the auto-loaded Python file has been
> +# registered.

Same as for the comment above (no more output generated by the handler). 
I guess this comment can just be dropped, since there's a separate one 
for the check below now.

> +if { ![runto_main] } {
> +    fail "failed to run to main"
> +    return
> +}
> +
> +# Check that the new_objfile handler saw the pretty-printer.
> +gdb_test "print all_good" " = true"
> +
> +# Check that the pretty-printer actually works.
> +gdb_test "info pretty-printer" "my_library.*MyClassTestLib.*"
> +gdb_breakpoint [gdb_get_line_number "break to inspect"]
> +gdb_test "continue" "Breakpoint $decimal, main .*"
> +gdb_test "print test" "MyClassTestLib object, id: 1.*"
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> new file mode 100644
> index 000000000000..85d60fc51c31
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> @@ -0,0 +1,50 @@
> +# Copyright (C) 2021 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 that python pretty
> +# printers defined in a python script that is autoloaded have been
> +# registered when a custom event handler for the new_objfile event
> +# is called.
> +
> +import gdb
> +import os
> +
> +
> +def new_objfile_handler(event):
> +    assert isinstance(event, gdb.NewObjFileEvent)
> +    objfile = event.new_objfile
> +
> +    # Only observe the custom test library.
> +    libname = "libpy-autoloaded-pretty-printers-in-newobjfile-event"
> +    if libname in os.path.basename(objfile.filename):
> +        # If everything went well and the pretty-printer auto-load happened
> +        # before notifying the Python listeners, we expect to see one pretty
> +        # printer, and it must be ours.
> +        all_good = (
> +            len(objfile.pretty_printers) == 1
> +            and objfile.pretty_printers[0].name == "my_library"
> +        )
> +
> +        if all_good:
> +            gdb.parse_and_eval("all_good = true")
> +        else:
> +            print("Oops, not all good:")
> +            print("pretty printer count: {}".format(len(objfile.pretty_printers)))
> +
> +            for pp in objfile.pretty_printers:
> +                print("  - {}".format(pp.name))
> +
> +
> +gdb.events.new_objfile.connect(new_objfile_handler)
> 

Michael


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v5 0/2] Make sure autoload happens before notifying Python side in new_objfile event

[Michael Weghorn]

[-- Attachment #1.1: Type: text/plain, Size: 1375 bytes --]

Hi Simon,

On 26/04/2021 16.53, Simon Marchi wrote:
> This is a new version of Michael Weghorn's patchset here:
> 
> https://sourceware.org/pipermail/gdb-patches/2021-April/178069.html
> 
> I rebased it after merging my "set debug observer" changes.
> 
> In addition to just rebasing, I made many style fixes here and there.  I
> also did some changes in the test to use some more recent GDB testsuite
> idioms (that you would miss if you copied an older test, and that I
> wouldn't expect a new-ish contributor to know).
> 
> I also changed the test to make it work for the native-gdbserver board.
> Previously, the test checked the output of the "run" command.  This
> doesn't work for native-gdbserver, because GDB is connected using the
> "remote" protocol, which doesn't support running.  Instead, I made the
> new_objfile handler set a global variable if everything looks good.  We
> then check the global variable value in the test.

A big thanks for doing all of this!

> 
> Michael, can you please take a look and check it is still ok with you?

Yes, that looks good (and better than before your changes).
I just found a potential typo in the first commit message and two 
comments in the test that have not been updated to the fact that the 
handler now sets a global variable instead of generating output.

Thanks again,
Michael


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Simon Marchi]

>> +# Make the -gdb.py script available to gdb, it is automatically loaded 
> by
>> +# gdb if it is put in the same directory as the library.
>> +set remote_python_autoload_file \
>> +    [gdb_remote_download host $python_autoload_file]
>> +
>> +gdb_test_no_output \
>> +    "set auto-load safe-path ${remote_python_autoload_file}" \
>> +    "set auto-load safe-path"
>> +
>> +# Load the Python file that defines a handler for the new_objfile event,
>> +# which will generate the output to check later
>> +# (prints information on available pretty-printers for objfile).
> 
> In v5, the handler no longer generates output (for the good case), so maybe this comment should be changed to something like this:
> 
> # Load the Python file that defines a handler for the new_objfile event,
> # which will set a global variable if the pretty-printer is available.


Shortened it to just:

# Load the Python file that defines a handler for the new_objfile event.

I think that's enough details.

>> +set remote_python_event_handler_file\
>> +    [gdb_remote_download host $python_event_handler_file]
>> +gdb_test_no_output "source ${remote_python_event_handler_file}" "load python file"
>> +
>> +gdb_load ${binfile}
>> +
>> +gdb_test_no_output "set print pretty on"
>> +
>> +# Check that the handler prints output when test library is loaded
>> +# and that the pretty-printer from the auto-loaded Python file has been
>> +# registered.
> 
> Same as for the comment above (no more output generated by the handler). I guess this comment can just be dropped, since there's a separate one for the check below now.

Indeed, did that.

Thanks,

Simon

Re: [PATCH v5 1/2] gdbsupport: allow to specify dependencies between observers

[Simon Marchi]

On 2021-04-26 3:56 p.m., Michael Weghorn wrote:
> On 26/04/2021 16.53, Simon Marchi wrote:
>> From: Michael Weghorn <m.weghorn@posteo.de>
>>
>> Previously, the observers attached to an observable were always notified
>> in the order in which they had been attached.  That order is not easily
>> controlled, because observers are typically attached in _initialize_*
>> functions, we are called in an undefined order.
> 
> Just a nit-pick, should that read "functions, WHICH are called in an undefined order."?

Yes, thanks for spotting that.

Simon

Re: [PATCH v5 1/2] gdbsupport: allow to specify dependencies between observers

[Andrew Burgess]

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-26 10:53:39 -0400]:

> From: Michael Weghorn <m.weghorn@posteo.de>
> 
> Previously, the observers attached to an observable were always notified
> in the order in which they had been attached.  That order is not easily
> controlled, because observers are typically attached in _initialize_*
> functions, we are called in an undefined order.
> 
> However, an observer may require that another observer attached only
> later is called before itself is.
> 
> Therefore, extend the 'observable' class to allow explicitly specifying
> dependencies when attaching observers, by adding the possibility to
> specify tokens for observers that it depends on.
> 
> To make sure dependencies are notified before observers depending on
> them, the vector holding the observers is sorted in a way that
> dependencies come before observers depending on them.  The current
> implementation for sorting uses the depth-first search algorithm for
> topological sorting as described at [1].
> 
> Extend the observable unit tests to cover this case as well.  Check that
> this works for a few different orders in which the observers are
> attached.
> 
> This newly introduced mechanism to explicitly specify dependencies will
> be used in a follow-up commit.
> 
> [1] https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search
> 
> Tested on x86_64-linux (Debian testing).
> 
> gdb/ChangeLog:
> 
> 	* unittests/observable-selftests.c (dependency_test_counters):
> 	New.
> 	(observer_token0, observer_token1, observer_token2,
> 	observer_token3, observer_token4, observer_token5): New.
> 	(struct dependency_observer_data): New struct.
> 	(observer_dependency_test_callback): New function.
> 	(test_observers): New.
> 	(run_dependency_test): New function.
> 	(test_dependency): New.
> 	(_initialize_observer_selftest): Register dependency test.
> 
> gdbsupport/ChangeLog:
> 
> 	* observable.h (class observable): Extend to allow specifying
> 	dependencies between observers, keep vector holding observers
> 	sorted so that dependencies are notified before observers
> 	depending on them.

I had some really minor feedback on some comments, otherwise looks
good.

> 
> Change-Id: I5399def1eeb69ca99e28c9f1fdf321d78b530bdb
> ---
>  gdb/unittests/observable-selftests.c | 112 +++++++++++++++++++++++++++
>  gdbsupport/observable.h              | 111 ++++++++++++++++++++++----
>  2 files changed, 207 insertions(+), 16 deletions(-)
> 
> diff --git a/gdb/unittests/observable-selftests.c b/gdb/unittests/observable-selftests.c
> index 0e3b53f555b0..88b009f1188d 100644
> --- a/gdb/unittests/observable-selftests.c
> +++ b/gdb/unittests/observable-selftests.c
> @@ -30,6 +30,60 @@ static int test_first_observer = 0;
>  static int test_second_observer = 0;
>  static int test_third_observer = 0;
>  
> +/* Counters for observers used for dependency tests.  */
> +static std::vector<int> dependency_test_counters;
> +
> +/* Tokens for observers used for dependency tests.  */
> +static gdb::observers::token observer_token0;
> +static gdb::observers::token observer_token1;
> +static gdb::observers::token observer_token2;
> +static gdb::observers::token observer_token3;
> +static gdb::observers::token observer_token4;
> +static gdb::observers::token observer_token5;
> +
> +/* Data for one observer used for checking that dependencies work as expected;
> +   dependencies are specified using their indices into the 'test_observers'
> +   vector here for simplicity and mapped to corresponding tokens later.  */
> +struct dependency_observer_data
> +{
> +  gdb::observers::token *token;
> +
> +  /* Name of the observer to use on attach.  */
> +  const char *name;
> +
> +  /* Indices of observers that this one directly depends on.  */
> +  std::vector<int> direct_dependencies;
> +
> +  /* Indices for all dependencies, including transitive ones.  */
> +  std::vector<int> all_dependencies;
> +
> +  /* Function to attach to the observable for this observer.  */
> +  std::function<void (int)> callback;
> +};
> +
> +static void observer_dependency_test_callback (size_t index);
> +
> +/* Data for observers to use for dependency tests, using some sample
> +   dependencies between the observers.  */
> +static std::vector<dependency_observer_data> test_observers = {
> +  {&observer_token0, "test0", {}, {},
> +   [] (int) { observer_dependency_test_callback (0); }},
> +  {&observer_token1, "test1", {0}, {0},
> +   [] (int) { observer_dependency_test_callback (1); }},
> +  {&observer_token2, "test2", {1}, {0, 1},
> +   [] (int) { observer_dependency_test_callback (2); }},
> +  {&observer_token3, "test3", {1}, {0, 1},
> +   [] (int) { observer_dependency_test_callback (3); }},
> +  {&observer_token4, "test4", {2, 3, 5}, {0, 1, 2, 3, 5},
> +   [] (int) { observer_dependency_test_callback (4); }},
> +  {&observer_token5, "test5", {0}, {0},
> +   [] (int) { observer_dependency_test_callback (5); }},
> +  {nullptr, "test6", {4}, {0, 1, 2, 3, 4, 5},
> +   [] (int) { observer_dependency_test_callback (6); }},
> +  {nullptr, "test7", {0}, {0},
> +   [] (int) { observer_dependency_test_callback (7); }},
> +};
> +
>  static void
>  test_first_notification_function (int arg)
>  {
> @@ -63,6 +117,62 @@ notify_check_counters (int one, int two, int three)
>    SELF_CHECK (three == test_third_observer);
>  }
>  
> +/* Function for each observer to run when being notified during the
> +   dependency tests. Verifies that dependencies have been notified earlier
> +   by checking their counters, then increases the own counter.  */

I don't think this sentence is quite correct, maybe ", then increases
the observer's counter." ?

> +static void
> +observer_dependency_test_callback (size_t index)
> +{
> +  /* Check that dependencies have already been notified.  */
> +  for (int i : test_observers[index].all_dependencies)
> +    SELF_CHECK (dependency_test_counters[i] == 1);
> +
> +  /* Increase own counter.  */
> +  dependency_test_counters[index]++;
> +}
> +
> +/* Run a dependency test by attaching the observers in the specified order
> +   then notifying them.  */
> +static void
> +run_dependency_test (std::vector<int> insertion_order)
> +{
> +  gdb::observers::observable<int> dependency_test_notification
> +    ("dependency_test_notification");
> +
> +  /* Reset counters.  */
> +  dependency_test_counters = std::vector<int> (test_observers.size (), 0);
> +
> +  /* Attach all observers in the given order, specifying dependencies.  */
> +  for (int i : insertion_order)
> +    {
> +      const dependency_observer_data &o = test_observers[i];
> +
> +      /* Get tokens for dependencies using their indices.  */
> +      std::vector<const gdb::observers::token *> dependency_tokens;
> +      for (int index : o.all_dependencies)
> +	dependency_tokens.emplace_back (test_observers[index].token);
> +
> +      if (o.token != nullptr)
> +	dependency_test_notification.attach
> +	  (o.callback, *o.token, o.name, dependency_tokens);
> +      else
> +	dependency_test_notification.attach (o.callback, o.name,
> +					     dependency_tokens);
> +    }
> +
> +  /* Notify observers, they check that their dependencies were notified.  */
> +  dependency_test_notification.notify (1);
> +}
> +
> +static void
> +test_dependency ()
> +{
> +  /* Run dependency tests with different insertion orders.  */
> +  run_dependency_test ({0, 1, 2, 3, 4, 5, 6, 7});
> +  run_dependency_test ({7, 6, 5, 4, 3, 2, 1, 0});
> +  run_dependency_test ({0, 3, 2, 1, 7, 6, 4, 5});
> +}
> +
>  static void
>  run_tests ()
>  {
> @@ -133,4 +243,6 @@ _initialize_observer_selftest ()
>  {
>    selftests::register_test ("gdb::observers",
>  			    selftests::observers::run_tests);
> +  selftests::register_test ("gdb::observers dependency",
> +			    selftests::observers::test_dependency);
>  }
> diff --git a/gdbsupport/observable.h b/gdbsupport/observable.h
> index 4ba47bb988f4..648158c8ca23 100644
> --- a/gdbsupport/observable.h
> +++ b/gdbsupport/observable.h
> @@ -71,13 +71,15 @@ class observable
>  private:
>    struct observer
>    {
> -    observer (const struct token *token, func_type func, const char *name)
> -      : token (token), func (func), name (name)
> +    observer (const struct token *token, func_type func, const char *name,
> +	      const std::vector<const struct token *> &dependencies)
> +      : token (token), func (func), name (name), dependencies (dependencies)
>      {}
>  
>      const struct token *token;
>      func_type func;
>      const char *name;
> +    std::vector<const struct token *> dependencies;
>    };
>  
>  public:
> @@ -88,30 +90,29 @@ class observable
>  
>    DISABLE_COPY_AND_ASSIGN (observable);
>  
> -  /* Attach F as an observer to this observable.  F cannot be
> -     detached.
> +  /* Attach F as an observer to this observable.  F cannot be detached or
> +     specified as a dependency.

We should probably document DEPENDENCIES here too, like you do for the
alternative attach below.

>  
>       NAME is the name of the observer, used for debug output purposes.  Its
>       lifetime must be at least as long as the observer is attached.  */
> -  void attach (const func_type &f, const char *name)
> +  void attach (const func_type &f, const char *name,
> +	       const std::vector<const struct token *> &dependencies = {})
>    {
> -    observer_debug_printf ("Attaching observable %s to observer %s",
> -			   name, m_name);
> -
> -    m_observers.emplace_back (nullptr, f, name);
> +    attach (f, nullptr, name, dependencies);
>    }
>  
> -  /* Attach F as an observer to this observable.  T is a reference to
> -     a token that can be used to later remove F.
> +  /* Attach F as an observer to this observable.
> +
> +     T is a reference to a token that can be used to later remove F or specify F
> +     as a dependency of another observer.  Dependencies are notified before the
> +     observer depending on them.

The wording here, though completely accurate, confused me at first.
How about:

  "DEPENDENCIES are notified before this observer."

It confused me because attach is attaching an observer, but you talk
about "the observer depending on them", which (for a moment) made we
wonder if there was some other observer being referenced.

Otherwise looks good.

Thanks,
Andrew

>
>       NAME is the name of the observer, used for debug output purposes.  Its
>       lifetime must be at least as long as the observer is attached.  */
> -  void attach (const func_type &f, const token &t, const char *name)
> +  void attach (const func_type &f, const token &t, const char *name,
> +	       const std::vector<const struct token *> &dependencies = {})
>    {
> -    observer_debug_printf ("Attaching observable %s to observer %s",
> -			   name, m_name);
> -
> -    m_observers.emplace_back (&t, f, name);
> +    attach (f, &t, name, dependencies);
>    }
>  
>    /* Remove observers associated with T from this observable.  T is
> @@ -149,6 +150,84 @@ class observable
>  
>    std::vector<observer> m_observers;
>    const char *m_name;
> +
> +  /* Use for sorting algorithm, to indicate which observer we have visited.  */
> +  enum class visit_state
> +  {
> +    NOT_VISITED,
> +    VISITING,
> +    VISITED,
> +  };
> +
> +  /* Helper method for topological sort using depth-first search algorithm.
> +
> +     Visit all dependencies of observer at INDEX in M_OBSERVERS (later referred
> +     to as "the observer").  Then append the observer to SORTED_OBSERVERS.
> +
> +     If the observer is already visited, do nothing.  */
> +  void visit_for_sorting (std::vector<observer> &sorted_observers,
> +                          std::vector<visit_state> &visit_states, int index)
> +  {
> +    if (visit_states[index] == visit_state::VISITED)
> +      return;
> +
> +    /* If we are already visiting this observer, it means there's a cycle.  */
> +    gdb_assert (visit_states[index] != visit_state::VISITING);
> +
> +    visit_states[index] = visit_state::VISITING;
> +
> +    /* For each dependency of this observer...  */
> +    for (const token *dep : m_observers[index].dependencies)
> +      {
> +	/* ... find the observer that has token DEP.  If found, visit it.  */
> +        auto it_dep
> +            = std::find_if (m_observers.begin (), m_observers.end (),
> +                            [&] (observer o) { return o.token == dep; });
> +        if (it_dep != m_observers.end ())
> +          {
> +            int i = std::distance (m_observers.begin (), it_dep);
> +            visit_for_sorting (sorted_observers, visit_states, i);
> +          }
> +      }
> +
> +    visit_states[index] = visit_state::VISITED;
> +    sorted_observers.push_back (m_observers[index]);
> +  }
> +
> +  /* Sort the observers, so that dependencies come before observers
> +     depending on them.
> +
> +     Uses depth-first search algorithm for topological sorting, see
> +     https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search .  */
> +  void sort_observers ()
> +  {
> +    std::vector<observer> sorted_observers;
> +    std::vector<visit_state> visit_states (m_observers.size (),
> +					   visit_state::NOT_VISITED);
> +
> +    for (size_t i = 0; i < m_observers.size (); i++)
> +      visit_for_sorting (sorted_observers, visit_states, i);
> +
> +    m_observers = std::move (sorted_observers);
> +  }
> +
> +  void attach (const func_type &f, const token *t, const char *name,
> +               const std::vector<const struct token *> &dependencies)
> +  {
> +
> +    observer_debug_printf ("Attaching observable %s to observer %s",
> +                           name, m_name);
> +
> +    m_observers.emplace_back (t, f, name, dependencies);
> +
> +    /* The observer has been inserted at the end of the vector, so it will be
> +       after any of its potential dependencies attached earlier.  If the
> +       observer has a token, it means that other observers can specify it as
> +       a dependency, so sorting is necessary to ensure those will be after the
> +       newly inserted observer afterwards.  */
> +    if (t != nullptr)
> +      sort_observers ();
> +  };
>  };
>  
>  } /* namespace observers */
> -- 
> 2.30.1
> 

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Andrew Burgess]

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-26 10:53:40 -0400]:

> From: Michael Weghorn <m.weghorn@posteo.de>
> 
> Without any explicit dependencies specified, the observers attached
> to the 'gdb::observers::new_objfile' observable are always notified
> in the order in which they have been attached.
> 
> The new_objfile observer callback to auto-load scripts is attached in
> '_initialize_auto_load'.
> The new_objfile observer callback that propagates the new_objfile event
> to the Python side is attached in 'gdbpy_initialize_inferior', which is
> called via '_initialize_python'.
> With '_initialize_python' happening before '_initialize_auto_load',
> the consequence was that the new_objfile event was emitted on the Python
> side before autoloaded scripts had been executed when a new objfile was
> loaded.
> As a result, trying to access the objfile's pretty printers (defined in
> the autoloaded script) from a handler for the Python-side
> 'new_objfile' event would fail. Those would only be initialized later on
> (when the 'auto_load_new_objfile' callback was called).
> 
> To make sure that the objfile passed to the Python event handler
> is properly initialized (including its 'pretty_printers' member),
> make sure that the 'auto_load_new_objfile' observer is notified
> before the 'python_new_objfile' one that propagates the event
> to the Python side.
> 
> To do this, make use of the mechanism to explicitly specify
> dependencies between observers (introduced in a preparatory commit).
> 
> Add a corresponding testcase that involves a test library with an autoloaded
> Python script and a handler for the Python 'new_objfile' event.
> 
> (The real world use case where I came across this issue was in an attempt
> to extend handling for GDB pretty printers for dynamically loaded
> objfiles in the Qt Creator IDE, s. [1] and [2] for more background.)
> 
> [1] https://bugreports.qt.io/browse/QTCREATORBUG-25339
> [2] https://codereview.qt-project.org/c/qt-creator/qt-creator/+/333857/1
> 
> Tested on x86_64-linux (Debian testing).
> 
> gdb/ChangeLog:
> 
>         * gdb/auto-load.c (_initialize_auto_load): 'Specify token
>         when attaching the 'auto_load_new_objfile' observer, so
>         other observers can specify it as a dependency.
> 
>         * gdb/auto-load.h (struct token): Declare
>         'auto_load_new_objfile_observer_token' as token to be used
>         for the 'auto_load_new_objfile' observer.
>         * gdb/python/py-inferior.c (gdbpy_initialize_inferior): Make
>         'python_new_objfile' observer depend on 'auto_load_new_objfile'
>         observer, so it gets notified after the latter.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py: New test.
>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc: New test.
>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h: New test.
>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc: New test.
>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp: New test.
>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py: New test.
> 
> Change-Id: I8275b3f4c3bec32e56dd7892f9a59d89544edf89
> ---
>  gdb/auto-load.c                               |  9 +-
>  gdb/auto-load.h                               |  8 ++
>  gdb/python/py-inferior.c                      |  7 +-
>  ...tty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++
>  ...pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++
>  ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
>  ...retty-printers-in-newobjfile-event-main.cc | 27 ++++++
>  ...ed-pretty-printers-in-newobjfile-event.exp | 85 +++++++++++++++++++
>  ...ded-pretty-printers-in-newobjfile-event.py | 50 +++++++++++
>  9 files changed, 285 insertions(+), 3 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> 
> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> index 239efa346064..d1ae6deacee7 100644
> --- a/gdb/auto-load.c
> +++ b/gdb/auto-load.c
> @@ -1494,6 +1494,10 @@ found and/or loaded."),
>    return &retval;
>  }
>  
> +/* See auto-load.h.  */
> +
> +gdb::observers::token auto_load_new_objfile_observer_token;
> +
>  void _initialize_auto_load ();
>  void
>  _initialize_auto_load ()
> @@ -1503,8 +1507,9 @@ _initialize_auto_load ()
>    char *guile_name_help;
>    const char *suffix;
>  
> -  gdb::observers::new_objfile.attach (auto_load_new_objfile, "auto-load");
> -
> +  gdb::observers::new_objfile.attach (auto_load_new_objfile,
> +                                      auto_load_new_objfile_observer_token,
> +                                      "auto-load");
>    add_setshow_boolean_cmd ("gdb-scripts", class_support,
>  			   &auto_load_gdb_scripts, _("\
>  Enable or disable auto-loading of canned sequences of commands scripts."), _("\
> diff --git a/gdb/auto-load.h b/gdb/auto-load.h
> index f726126c5541..4372ec4f4dd7 100644
> --- a/gdb/auto-load.h
> +++ b/gdb/auto-load.h
> @@ -25,6 +25,10 @@ struct program_space;
>  struct auto_load_pspace_info;
>  struct extension_language_defn;
>  
> +namespace gdb::observers {
> +struct token;
> +}
> +

I wonder if we should move the declaration of gdb::observers::token
out of observable.h into observable-token.h, then it would be cheap
enough to just include observable-token.h into other header files?

Otherwise, all looks good.

Thanks,
Andrew

>  /* Value of the 'set debug auto-load' configuration variable.  */
>  
>  extern bool debug_auto_load;
> @@ -40,6 +44,10 @@ extern bool auto_load_local_gdbinit;
>  extern char *auto_load_local_gdbinit_pathname;
>  extern bool auto_load_local_gdbinit_loaded;
>  
> +/* Token used for the auto_load_new_objfile observer, so other observers can
> +   specify it as a dependency. */
> +extern gdb::observers::token auto_load_new_objfile_observer_token;
> +
>  extern struct auto_load_pspace_info *
>    get_auto_load_pspace_data_for_loading (struct program_space *pspace);
>  extern void auto_load_objfile_script (struct objfile *objfile,
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index c2861ccb735c..febd2a73ece3 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "auto-load.h"
>  #include "gdbcore.h"
>  #include "gdbthread.h"
>  #include "inferior.h"
> @@ -917,7 +918,11 @@ gdbpy_initialize_inferior (void)
>    gdb::observers::register_changed.attach (python_on_register_change,
>  					   "py-inferior");
>    gdb::observers::inferior_exit.attach (python_inferior_exit, "py-inferior");
> -  gdb::observers::new_objfile.attach (python_new_objfile, "py-inferior");
> +  /* Need to run after auto-load's new_objfile observer, so that
> +     auto-loaded pretty-printers are available.  */
> +  gdb::observers::new_objfile.attach
> +    (python_new_objfile, "py-inferior",
> +     { &auto_load_new_objfile_observer_token });
>    gdb::observers::inferior_added.attach (python_new_inferior, "py-inferior");
>    gdb::observers::inferior_removed.attach (python_inferior_deleted,
>  					   "py-inferior");
> diff --git a/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
> new file mode 100644
> index 000000000000..aeb39a6c483a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
> @@ -0,0 +1,43 @@
> +# Copyright (C) 2021 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 that python pretty
> +# printers defined in a python script that is autoloaded have been
> +# registered when a custom event handler for the new_objfile event
> +# is called.
> +
> +import gdb.printing
> +
> +
> +class MyClassTestLibPrinter(object):
> +    "Print a MyClassTestLib"
> +
> +    def __init__(self, val):
> +        self.val = val
> +
> +    def to_string(self):
> +        return "MyClassTestLib object, id: {}".format(self.val["id"])
> +
> +    def display_hint(self):
> +        return "string"
> +
> +
> +def build_pretty_printer():
> +    pp = gdb.printing.RegexpCollectionPrettyPrinter("my_library")
> +    pp.add_printer("MyClassTestLib", "^MyClassTestLib$", MyClassTestLibPrinter)
> +    return pp
> +
> +
> +gdb.printing.register_pretty_printer(gdb.current_objfile(), build_pretty_printer())
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
> new file mode 100644
> index 000000000000..7f13cd2b741e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
> @@ -0,0 +1,28 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
> +
> +MyClassTestLib::MyClassTestLib (int theId)
> +{
> +  id = theId;
> +}
> +
> +int MyClassTestLib::getId ()
> +{
> +  return id;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
> new file mode 100644
> index 000000000000..3714ecd2ef08
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
> @@ -0,0 +1,31 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +#ifndef TESTLIBRARY_H
> +#define TESTLIBRARY_H
> +
> +class MyClassTestLib
> +{
> +public:
> +  explicit MyClassTestLib (int theId);
> +  int getId ();
> +
> +private:
> +  int id;
> +};
> +
> +#endif /* TESTLIBRARY_H */
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
> new file mode 100644
> index 000000000000..2cc89a3befd5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
> @@ -0,0 +1,27 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
> +
> +bool all_good = false;
> +
> +int
> +main ()
> +{
> +  MyClassTestLib test (1);
> +  return 0; /* break to inspect */
> +}
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
> new file mode 100644
> index 000000000000..444466109e8f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
> @@ -0,0 +1,85 @@
> +# Copyright (C) 2021 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 that Python pretty-printers
> +# defined in a Python script that is autoloaded are registered when an event
> +# handler for the new_objfile event is called.
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile -main.cc
> +
> +set srcfile_lib "${testfile}-lib.cc"
> +set python_event_handler_file "${srcdir}/${subdir}/${testfile}.py"
> +set libname "lib${testfile}"
> +set python_autoload_file "${srcdir}/${subdir}/${libname}.so-gdb.py"
> +set binfile_lib [standard_output_file "${libname}.so"]
> +
> +# Start GDB first - needed for skip_python_tests.
> +clean_restart
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +# Compile library.
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} \
> +      {debug c++}] != "" } {
> +    return -1
> +}
> +
> +# Compile main program.
> +if { [gdb_compile ${srcdir}/${subdir}/${srcfile} \
> +      ${binfile} \
> +      executable \
> +      [list debug c++ shlib=$binfile_lib]] != "" } {
> +    return -1
> +}
> +
> +# Make the -gdb.py script available to gdb, it is automatically loaded by
> +# gdb if it is put in the same directory as the library.
> +set remote_python_autoload_file \
> +    [gdb_remote_download host $python_autoload_file]
> +
> +gdb_test_no_output \
> +    "set auto-load safe-path ${remote_python_autoload_file}" \
> +    "set auto-load safe-path"
> +
> +# Load the Python file that defines a handler for the new_objfile event,
> +# which will generate the output to check later
> +# (prints information on available pretty-printers for objfile).
> +set remote_python_event_handler_file\
> +    [gdb_remote_download host $python_event_handler_file]
> +gdb_test_no_output "source ${remote_python_event_handler_file}" "load python file"
> +
> +gdb_load ${binfile}
> +
> +gdb_test_no_output "set print pretty on"
> +
> +# Check that the handler prints output when test library is loaded
> +# and that the pretty-printer from the auto-loaded Python file has been
> +# registered.
> +if { ![runto_main] } {
> +    fail "failed to run to main"
> +    return
> +}
> +
> +# Check that the new_objfile handler saw the pretty-printer.
> +gdb_test "print all_good" " = true"
> +
> +# Check that the pretty-printer actually works.
> +gdb_test "info pretty-printer" "my_library.*MyClassTestLib.*"
> +gdb_breakpoint [gdb_get_line_number "break to inspect"]
> +gdb_test "continue" "Breakpoint $decimal, main .*"
> +gdb_test "print test" "MyClassTestLib object, id: 1.*"
> diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> new file mode 100644
> index 000000000000..85d60fc51c31
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> @@ -0,0 +1,50 @@
> +# Copyright (C) 2021 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 that python pretty
> +# printers defined in a python script that is autoloaded have been
> +# registered when a custom event handler for the new_objfile event
> +# is called.
> +
> +import gdb
> +import os
> +
> +
> +def new_objfile_handler(event):
> +    assert isinstance(event, gdb.NewObjFileEvent)
> +    objfile = event.new_objfile
> +
> +    # Only observe the custom test library.
> +    libname = "libpy-autoloaded-pretty-printers-in-newobjfile-event"
> +    if libname in os.path.basename(objfile.filename):
> +        # If everything went well and the pretty-printer auto-load happened
> +        # before notifying the Python listeners, we expect to see one pretty
> +        # printer, and it must be ours.
> +        all_good = (
> +            len(objfile.pretty_printers) == 1
> +            and objfile.pretty_printers[0].name == "my_library"
> +        )
> +
> +        if all_good:
> +            gdb.parse_and_eval("all_good = true")
> +        else:
> +            print("Oops, not all good:")
> +            print("pretty printer count: {}".format(len(objfile.pretty_printers)))
> +
> +            for pp in objfile.pretty_printers:
> +                print("  - {}".format(pp.name))
> +
> +
> +gdb.events.new_objfile.connect(new_objfile_handler)
> -- 
> 2.30.1
> 

Re: [PATCH v5 1/2] gdbsupport: allow to specify dependencies between observers

[Simon Marchi]

>> +/* Function for each observer to run when being notified during the
>> +   dependency tests. Verifies that dependencies have been notified earlier
>> +   by checking their counters, then increases the own counter.  */
> 
> I don't think this sentence is quite correct, maybe ", then increases
> the observer's counter." ?

Indeed.  I ended up doing a bit more changes:

120 /* Function for each observer to run when being notified during the dependency
121    tests.  Verify that the observer's dependencies have been notified before the
122    observer itself by checking their counters, then increase the observer's own
123    counter.  */

>> @@ -88,30 +90,29 @@ class observable
>>  
>>    DISABLE_COPY_AND_ASSIGN (observable);
>>  
>> -  /* Attach F as an observer to this observable.  F cannot be
>> -     detached.
>> +  /* Attach F as an observer to this observable.  F cannot be detached or
>> +     specified as a dependency.
> 
> We should probably document DEPENDENCIES here too, like you do for the
> alternative attach below.

Indeed, I ended up re-wording see below.

>>  
>>       NAME is the name of the observer, used for debug output purposes.  Its
>>       lifetime must be at least as long as the observer is attached.  */
>> -  void attach (const func_type &f, const char *name)
>> +  void attach (const func_type &f, const char *name,
>> +	       const std::vector<const struct token *> &dependencies = {})
>>    {
>> -    observer_debug_printf ("Attaching observable %s to observer %s",
>> -			   name, m_name);
>> -
>> -    m_observers.emplace_back (nullptr, f, name);
>> +    attach (f, nullptr, name, dependencies);
>>    }
>>  
>> -  /* Attach F as an observer to this observable.  T is a reference to
>> -     a token that can be used to later remove F.
>> +  /* Attach F as an observer to this observable.
>> +
>> +     T is a reference to a token that can be used to later remove F or specify F
>> +     as a dependency of another observer.  Dependencies are notified before the
>> +     observer depending on them.
> 
> The wording here, though completely accurate, confused me at first.
> How about:
> 
>   "DEPENDENCIES are notified before this observer."
> 
> It confused me because attach is attaching an observer, but you talk
> about "the observer depending on them", which (for a moment) made we
> wonder if there was some other observer being referenced.

I ended up using this in both comments:

 96      DEPENDENCIES is a list of tokens of observers to be notified before this
 97      one.

> Otherwise looks good.

Thanks for the review!

Simon

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Simon Marchi]

On 2021-04-27 4:39 a.m., Andrew Burgess wrote:
> * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-26 10:53:40 -0400]:
> 
>> From: Michael Weghorn <m.weghorn@posteo.de>
>>
>> Without any explicit dependencies specified, the observers attached
>> to the 'gdb::observers::new_objfile' observable are always notified
>> in the order in which they have been attached.
>>
>> The new_objfile observer callback to auto-load scripts is attached in
>> '_initialize_auto_load'.
>> The new_objfile observer callback that propagates the new_objfile event
>> to the Python side is attached in 'gdbpy_initialize_inferior', which is
>> called via '_initialize_python'.
>> With '_initialize_python' happening before '_initialize_auto_load',
>> the consequence was that the new_objfile event was emitted on the Python
>> side before autoloaded scripts had been executed when a new objfile was
>> loaded.
>> As a result, trying to access the objfile's pretty printers (defined in
>> the autoloaded script) from a handler for the Python-side
>> 'new_objfile' event would fail. Those would only be initialized later on
>> (when the 'auto_load_new_objfile' callback was called).
>>
>> To make sure that the objfile passed to the Python event handler
>> is properly initialized (including its 'pretty_printers' member),
>> make sure that the 'auto_load_new_objfile' observer is notified
>> before the 'python_new_objfile' one that propagates the event
>> to the Python side.
>>
>> To do this, make use of the mechanism to explicitly specify
>> dependencies between observers (introduced in a preparatory commit).
>>
>> Add a corresponding testcase that involves a test library with an autoloaded
>> Python script and a handler for the Python 'new_objfile' event.
>>
>> (The real world use case where I came across this issue was in an attempt
>> to extend handling for GDB pretty printers for dynamically loaded
>> objfiles in the Qt Creator IDE, s. [1] and [2] for more background.)
>>
>> [1] https://bugreports.qt.io/browse/QTCREATORBUG-25339
>> [2] https://codereview.qt-project.org/c/qt-creator/qt-creator/+/333857/1
>>
>> Tested on x86_64-linux (Debian testing).
>>
>> gdb/ChangeLog:
>>
>>         * gdb/auto-load.c (_initialize_auto_load): 'Specify token
>>         when attaching the 'auto_load_new_objfile' observer, so
>>         other observers can specify it as a dependency.
>>
>>         * gdb/auto-load.h (struct token): Declare
>>         'auto_load_new_objfile_observer_token' as token to be used
>>         for the 'auto_load_new_objfile' observer.
>>         * gdb/python/py-inferior.c (gdbpy_initialize_inferior): Make
>>         'python_new_objfile' observer depend on 'auto_load_new_objfile'
>>         observer, so it gets notified after the latter.
>>
>> gdb/testsuite/ChangeLog:
>>
>>         * gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py: New test.
>>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc: New test.
>>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h: New test.
>>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc: New test.
>>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp: New test.
>>         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py: New test.
>>
>> Change-Id: I8275b3f4c3bec32e56dd7892f9a59d89544edf89
>> ---
>>  gdb/auto-load.c                               |  9 +-
>>  gdb/auto-load.h                               |  8 ++
>>  gdb/python/py-inferior.c                      |  7 +-
>>  ...tty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++
>>  ...pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++
>>  ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
>>  ...retty-printers-in-newobjfile-event-main.cc | 27 ++++++
>>  ...ed-pretty-printers-in-newobjfile-event.exp | 85 +++++++++++++++++++
>>  ...ded-pretty-printers-in-newobjfile-event.py | 50 +++++++++++
>>  9 files changed, 285 insertions(+), 3 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
>>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
>>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
>>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
>>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
>>  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
>>
>> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
>> index 239efa346064..d1ae6deacee7 100644
>> --- a/gdb/auto-load.c
>> +++ b/gdb/auto-load.c
>> @@ -1494,6 +1494,10 @@ found and/or loaded."),
>>    return &retval;
>>  }
>>  
>> +/* See auto-load.h.  */
>> +
>> +gdb::observers::token auto_load_new_objfile_observer_token;
>> +
>>  void _initialize_auto_load ();
>>  void
>>  _initialize_auto_load ()
>> @@ -1503,8 +1507,9 @@ _initialize_auto_load ()
>>    char *guile_name_help;
>>    const char *suffix;
>>  
>> -  gdb::observers::new_objfile.attach (auto_load_new_objfile, "auto-load");
>> -
>> +  gdb::observers::new_objfile.attach (auto_load_new_objfile,
>> +                                      auto_load_new_objfile_observer_token,
>> +                                      "auto-load");
>>    add_setshow_boolean_cmd ("gdb-scripts", class_support,
>>  			   &auto_load_gdb_scripts, _("\
>>  Enable or disable auto-loading of canned sequences of commands scripts."), _("\
>> diff --git a/gdb/auto-load.h b/gdb/auto-load.h
>> index f726126c5541..4372ec4f4dd7 100644
>> --- a/gdb/auto-load.h
>> +++ b/gdb/auto-load.h
>> @@ -25,6 +25,10 @@ struct program_space;
>>  struct auto_load_pspace_info;
>>  struct extension_language_defn;
>>  
>> +namespace gdb::observers {
>> +struct token;
>> +}
>> +
> 
> I wonder if we should move the declaration of gdb::observers::token
> out of observable.h into observable-token.h, then it would be cheap
> enough to just include observable-token.h into other header files?

I don't know, the point here is that we only have to forwad-declare it.
With your suggestion, the header here would see the full declaration of
the token class.  But then again, it's an empty class, so it's probably
not very expensive to include.  I'll try it and push with that if that
ends up making sense.

Thanks,

Simon

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Simon Marchi]

On 2021-04-27 9:43 a.m., Simon Marchi via Gdb-patches wrote:
>> I wonder if we should move the declaration of gdb::observers::token
>> out of observable.h into observable-token.h, then it would be cheap
>> enough to just include observable-token.h into other header files?
> 
> I don't know, the point here is that we only have to forwad-declare it.
> With your suggestion, the header here would see the full declaration of
> the token class.  But then again, it's an empty class, so it's probably
> not very expensive to include.  I'll try it and push with that if that
> ends up making sense.

Thinking a bit more about it, I'll leave it as-is.  That location
doesn't need to see the full declaration of the class, so it's really
not different from the other forward-declarations just before it.  It
just happens to have a namespace.

Simon

Re: [PATCH v5 0/2] Make sure autoload happens before notifying Python side in new_objfile event

[Simon Marchi]

On 2021-04-26 4:05 p.m., Michael Weghorn wrote:
> Hi Simon,
> 
> On 26/04/2021 16.53, Simon Marchi wrote:
>> This is a new version of Michael Weghorn's patchset here:
>>
>> https://sourceware.org/pipermail/gdb-patches/2021-April/178069.html
>>
>> I rebased it after merging my "set debug observer" changes.
>>
>> In addition to just rebasing, I made many style fixes here and there.  I
>> also did some changes in the test to use some more recent GDB testsuite
>> idioms (that you would miss if you copied an older test, and that I
>> wouldn't expect a new-ish contributor to know).
>>
>> I also changed the test to make it work for the native-gdbserver board.
>> Previously, the test checked the output of the "run" command.  This
>> doesn't work for native-gdbserver, because GDB is connected using the
>> "remote" protocol, which doesn't support running.  Instead, I made the
>> new_objfile handler set a global variable if everything looks good.  We
>> then check the global variable value in the test.
> 
> A big thanks for doing all of this!
> 
>>
>> Michael, can you please take a look and check it is still ok with you?
> 
> Yes, that looks good (and better than before your changes).
> I just found a potential typo in the first commit message and two comments in the test that have not been updated to the fact that the handler now sets a global variable instead of generating output.
> 
> Thanks again,
> Michael
> 

Thanks both, I've now pushed the patches.

Simon

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Vaseeharan Vinayagamoorthy]

After this commit, I am seeing these errors.

In file included from src/binutils-gdb--gdb/gdb/auto-load.c:22:0:
src/binutils-gdb--gdb/gdb/auto-load.h:28:14: error: expected ‘{’ before ‘::’ token
namespace gdb::observers {
              ^
src/binutils-gdb--gdb/gdb/auto-load.h:28:14: error: ‘observers’ in namespace ‘::’ does not name a type
src/binutils-gdb--gdb/gdb/auto-load.h:49:8: error: ‘observers’ in namespace ‘gdb’ does not name a type
extern gdb::observers::token auto_load_new_objfile_observer_token;
        ^
In file included from /usr/include/c++/4.8.2/set:60:0,
                 from src/binutils-gdb--gdb/gdb/symtab.h:26,
                 from src/binutils-gdb--gdb/gdb/infrun.h:21,
                 from src/binutils-gdb--gdb/gdb/target.h:42,
                 from src/binutils-gdb--gdb/gdb/progspace.h:24,
                 from src/binutils-gdb--gdb/gdb/auto-load.c:23:
/usr/include/c++/4.8.2/bits/stl_tree.h: In constructor ‘gdb::std::_Rb_tree_node<_Val>::_Rb_tree_node(_Args&& ...)’:
/usr/include/c++/4.8.2/bits/stl_tree.h:140:19: error: ‘forward’ is not a member of ‘gdb::std’
    _M_value_field(std::forward<_Args>(__args)...) { }

The build/host/target setup is:
Build: x86_64 (Linux), using GCC 4.8.
Host:  x86_64 (Linux)
Target: arm-none-eabi / aarch64_be-none-elf / aarch64-none-elf


Regards
Vasee

On 27/04/2021, 09:39, "Gdb-patches on behalf of Andrew Burgess" <gdb-patches-bounces@sourceware.org on behalf of andrew.burgess@embecosm.com> wrote:

    * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-26 10:53:40 -0400]:

    > From: Michael Weghorn <m.weghorn@posteo.de>
    > 
    > Without any explicit dependencies specified, the observers attached
    > to the 'gdb::observers::new_objfile' observable are always notified
    > in the order in which they have been attached.
    > 
    > The new_objfile observer callback to auto-load scripts is attached in
    > '_initialize_auto_load'.
    > The new_objfile observer callback that propagates the new_objfile event
    > to the Python side is attached in 'gdbpy_initialize_inferior', which is
    > called via '_initialize_python'.
    > With '_initialize_python' happening before '_initialize_auto_load',
    > the consequence was that the new_objfile event was emitted on the Python
    > side before autoloaded scripts had been executed when a new objfile was
    > loaded.
    > As a result, trying to access the objfile's pretty printers (defined in
    > the autoloaded script) from a handler for the Python-side
    > 'new_objfile' event would fail. Those would only be initialized later on
    > (when the 'auto_load_new_objfile' callback was called).
    > 
    > To make sure that the objfile passed to the Python event handler
    > is properly initialized (including its 'pretty_printers' member),
    > make sure that the 'auto_load_new_objfile' observer is notified
    > before the 'python_new_objfile' one that propagates the event
    > to the Python side.
    > 
    > To do this, make use of the mechanism to explicitly specify
    > dependencies between observers (introduced in a preparatory commit).
    > 
    > Add a corresponding testcase that involves a test library with an autoloaded
    > Python script and a handler for the Python 'new_objfile' event.
    > 
    > (The real world use case where I came across this issue was in an attempt
    > to extend handling for GDB pretty printers for dynamically loaded
    > objfiles in the Qt Creator IDE, s. [1] and [2] for more background.)
    > 
    > [1] https://bugreports.qt.io/browse/QTCREATORBUG-25339
    > [2] https://codereview.qt-project.org/c/qt-creator/qt-creator/+/333857/1
    > 
    > Tested on x86_64-linux (Debian testing).
    > 
    > gdb/ChangeLog:
    > 
    >         * gdb/auto-load.c (_initialize_auto_load): 'Specify token
    >         when attaching the 'auto_load_new_objfile' observer, so
    >         other observers can specify it as a dependency.
    > 
    >         * gdb/auto-load.h (struct token): Declare
    >         'auto_load_new_objfile_observer_token' as token to be used
    >         for the 'auto_load_new_objfile' observer.
    >         * gdb/python/py-inferior.c (gdbpy_initialize_inferior): Make
    >         'python_new_objfile' observer depend on 'auto_load_new_objfile'
    >         observer, so it gets notified after the latter.
    > 
    > gdb/testsuite/ChangeLog:
    > 
    >         * gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py: New test.
    >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc: New test.
    >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h: New test.
    >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc: New test.
    >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp: New test.
    >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py: New test.
    > 
    > Change-Id: I8275b3f4c3bec32e56dd7892f9a59d89544edf89
    > ---
    >  gdb/auto-load.c                               |  9 +-
    >  gdb/auto-load.h                               |  8 ++
    >  gdb/python/py-inferior.c                      |  7 +-
    >  ...tty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++
    >  ...pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++
    >  ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
    >  ...retty-printers-in-newobjfile-event-main.cc | 27 ++++++
    >  ...ed-pretty-printers-in-newobjfile-event.exp | 85 +++++++++++++++++++
    >  ...ded-pretty-printers-in-newobjfile-event.py | 50 +++++++++++
    >  9 files changed, 285 insertions(+), 3 deletions(-)
    >  create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
    >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
    >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
    >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
    >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
    >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
    > 
    > diff --git a/gdb/auto-load.c b/gdb/auto-load.c
    > index 239efa346064..d1ae6deacee7 100644
    > --- a/gdb/auto-load.c
    > +++ b/gdb/auto-load.c
    > @@ -1494,6 +1494,10 @@ found and/or loaded."),
    >    return &retval;
    >  }
    >  
    > +/* See auto-load.h.  */
    > +
    > +gdb::observers::token auto_load_new_objfile_observer_token;
    > +
    >  void _initialize_auto_load ();
    >  void
    >  _initialize_auto_load ()
    > @@ -1503,8 +1507,9 @@ _initialize_auto_load ()
    >    char *guile_name_help;
    >    const char *suffix;
    >  
    > -  gdb::observers::new_objfile.attach (auto_load_new_objfile, "auto-load");
    > -
    > +  gdb::observers::new_objfile.attach (auto_load_new_objfile,
    > +                                      auto_load_new_objfile_observer_token,
    > +                                      "auto-load");
    >    add_setshow_boolean_cmd ("gdb-scripts", class_support,
    >  			   &auto_load_gdb_scripts, _("\
    >  Enable or disable auto-loading of canned sequences of commands scripts."), _("\
    > diff --git a/gdb/auto-load.h b/gdb/auto-load.h
    > index f726126c5541..4372ec4f4dd7 100644
    > --- a/gdb/auto-load.h
    > +++ b/gdb/auto-load.h
    > @@ -25,6 +25,10 @@ struct program_space;
    >  struct auto_load_pspace_info;
    >  struct extension_language_defn;
    >  
    > +namespace gdb::observers {
    > +struct token;
    > +}
    > +

    I wonder if we should move the declaration of gdb::observers::token
    out of observable.h into observable-token.h, then it would be cheap
    enough to just include observable-token.h into other header files?

    Otherwise, all looks good.

    Thanks,
    Andrew

    >  /* Value of the 'set debug auto-load' configuration variable.  */
    >  
    >  extern bool debug_auto_load;
    > @@ -40,6 +44,10 @@ extern bool auto_load_local_gdbinit;
    >  extern char *auto_load_local_gdbinit_pathname;
    >  extern bool auto_load_local_gdbinit_loaded;
    >  
    > +/* Token used for the auto_load_new_objfile observer, so other observers can
    > +   specify it as a dependency. */
    > +extern gdb::observers::token auto_load_new_objfile_observer_token;
    > +
    >  extern struct auto_load_pspace_info *
    >    get_auto_load_pspace_data_for_loading (struct program_space *pspace);
    >  extern void auto_load_objfile_script (struct objfile *objfile,
    > diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
    > index c2861ccb735c..febd2a73ece3 100644
    > --- a/gdb/python/py-inferior.c
    > +++ b/gdb/python/py-inferior.c
    > @@ -18,6 +18,7 @@
    >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
    >  
    >  #include "defs.h"
    > +#include "auto-load.h"
    >  #include "gdbcore.h"
    >  #include "gdbthread.h"
    >  #include "inferior.h"
    > @@ -917,7 +918,11 @@ gdbpy_initialize_inferior (void)
    >    gdb::observers::register_changed.attach (python_on_register_change,
    >  					   "py-inferior");
    >    gdb::observers::inferior_exit.attach (python_inferior_exit, "py-inferior");
    > -  gdb::observers::new_objfile.attach (python_new_objfile, "py-inferior");
    > +  /* Need to run after auto-load's new_objfile observer, so that
    > +     auto-loaded pretty-printers are available.  */
    > +  gdb::observers::new_objfile.attach
    > +    (python_new_objfile, "py-inferior",
    > +     { &auto_load_new_objfile_observer_token });
    >    gdb::observers::inferior_added.attach (python_new_inferior, "py-inferior");
    >    gdb::observers::inferior_removed.attach (python_inferior_deleted,
    >  					   "py-inferior");
    > diff --git a/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
    > new file mode 100644
    > index 000000000000..aeb39a6c483a
    > --- /dev/null
    > +++ b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
    > @@ -0,0 +1,43 @@
    > +# Copyright (C) 2021 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 that python pretty
    > +# printers defined in a python script that is autoloaded have been
    > +# registered when a custom event handler for the new_objfile event
    > +# is called.
    > +
    > +import gdb.printing
    > +
    > +
    > +class MyClassTestLibPrinter(object):
    > +    "Print a MyClassTestLib"
    > +
    > +    def __init__(self, val):
    > +        self.val = val
    > +
    > +    def to_string(self):
    > +        return "MyClassTestLib object, id: {}".format(self.val["id"])
    > +
    > +    def display_hint(self):
    > +        return "string"
    > +
    > +
    > +def build_pretty_printer():
    > +    pp = gdb.printing.RegexpCollectionPrettyPrinter("my_library")
    > +    pp.add_printer("MyClassTestLib", "^MyClassTestLib$", MyClassTestLibPrinter)
    > +    return pp
    > +
    > +
    > +gdb.printing.register_pretty_printer(gdb.current_objfile(), build_pretty_printer())
    > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
    > new file mode 100644
    > index 000000000000..7f13cd2b741e
    > --- /dev/null
    > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
    > @@ -0,0 +1,28 @@
    > +/* This testcase is part of GDB, the GNU debugger.
    > +
    > +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
    > +
    > +MyClassTestLib::MyClassTestLib (int theId)
    > +{
    > +  id = theId;
    > +}
    > +
    > +int MyClassTestLib::getId ()
    > +{
    > +  return id;
    > +}
    > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
    > new file mode 100644
    > index 000000000000..3714ecd2ef08
    > --- /dev/null
    > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
    > @@ -0,0 +1,31 @@
    > +/* This testcase is part of GDB, the GNU debugger.
    > +
    > +   Copyright 2021 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/>.  */
    > +
    > +#ifndef TESTLIBRARY_H
    > +#define TESTLIBRARY_H
    > +
    > +class MyClassTestLib
    > +{
    > +public:
    > +  explicit MyClassTestLib (int theId);
    > +  int getId ();
    > +
    > +private:
    > +  int id;
    > +};
    > +
    > +#endif /* TESTLIBRARY_H */
    > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
    > new file mode 100644
    > index 000000000000..2cc89a3befd5
    > --- /dev/null
    > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
    > @@ -0,0 +1,27 @@
    > +/* This testcase is part of GDB, the GNU debugger.
    > +
    > +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
    > +
    > +bool all_good = false;
    > +
    > +int
    > +main ()
    > +{
    > +  MyClassTestLib test (1);
    > +  return 0; /* break to inspect */
    > +}
    > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
    > new file mode 100644
    > index 000000000000..444466109e8f
    > --- /dev/null
    > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
    > @@ -0,0 +1,85 @@
    > +# Copyright (C) 2021 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 that Python pretty-printers
    > +# defined in a Python script that is autoloaded are registered when an event
    > +# handler for the new_objfile event is called.
    > +
    > +load_lib gdb-python.exp
    > +
    > +standard_testfile -main.cc
    > +
    > +set srcfile_lib "${testfile}-lib.cc"
    > +set python_event_handler_file "${srcdir}/${subdir}/${testfile}.py"
    > +set libname "lib${testfile}"
    > +set python_autoload_file "${srcdir}/${subdir}/${libname}.so-gdb.py"
    > +set binfile_lib [standard_output_file "${libname}.so"]
    > +
    > +# Start GDB first - needed for skip_python_tests.
    > +clean_restart
    > +
    > +# Skip all tests if Python scripting is not enabled.
    > +if { [skip_python_tests] } { continue }
    > +
    > +# Compile library.
    > +if { [gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} \
    > +      {debug c++}] != "" } {
    > +    return -1
    > +}
    > +
    > +# Compile main program.
    > +if { [gdb_compile ${srcdir}/${subdir}/${srcfile} \
    > +      ${binfile} \
    > +      executable \
    > +      [list debug c++ shlib=$binfile_lib]] != "" } {
    > +    return -1
    > +}
    > +
    > +# Make the -gdb.py script available to gdb, it is automatically loaded by
    > +# gdb if it is put in the same directory as the library.
    > +set remote_python_autoload_file \
    > +    [gdb_remote_download host $python_autoload_file]
    > +
    > +gdb_test_no_output \
    > +    "set auto-load safe-path ${remote_python_autoload_file}" \
    > +    "set auto-load safe-path"
    > +
    > +# Load the Python file that defines a handler for the new_objfile event,
    > +# which will generate the output to check later
    > +# (prints information on available pretty-printers for objfile).
    > +set remote_python_event_handler_file\
    > +    [gdb_remote_download host $python_event_handler_file]
    > +gdb_test_no_output "source ${remote_python_event_handler_file}" "load python file"
    > +
    > +gdb_load ${binfile}
    > +
    > +gdb_test_no_output "set print pretty on"
    > +
    > +# Check that the handler prints output when test library is loaded
    > +# and that the pretty-printer from the auto-loaded Python file has been
    > +# registered.
    > +if { ![runto_main] } {
    > +    fail "failed to run to main"
    > +    return
    > +}
    > +
    > +# Check that the new_objfile handler saw the pretty-printer.
    > +gdb_test "print all_good" " = true"
    > +
    > +# Check that the pretty-printer actually works.
    > +gdb_test "info pretty-printer" "my_library.*MyClassTestLib.*"
    > +gdb_breakpoint [gdb_get_line_number "break to inspect"]
    > +gdb_test "continue" "Breakpoint $decimal, main .*"
    > +gdb_test "print test" "MyClassTestLib object, id: 1.*"
    > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
    > new file mode 100644
    > index 000000000000..85d60fc51c31
    > --- /dev/null
    > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
    > @@ -0,0 +1,50 @@
    > +# Copyright (C) 2021 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 that python pretty
    > +# printers defined in a python script that is autoloaded have been
    > +# registered when a custom event handler for the new_objfile event
    > +# is called.
    > +
    > +import gdb
    > +import os
    > +
    > +
    > +def new_objfile_handler(event):
    > +    assert isinstance(event, gdb.NewObjFileEvent)
    > +    objfile = event.new_objfile
    > +
    > +    # Only observe the custom test library.
    > +    libname = "libpy-autoloaded-pretty-printers-in-newobjfile-event"
    > +    if libname in os.path.basename(objfile.filename):
    > +        # If everything went well and the pretty-printer auto-load happened
    > +        # before notifying the Python listeners, we expect to see one pretty
    > +        # printer, and it must be ours.
    > +        all_good = (
    > +            len(objfile.pretty_printers) == 1
    > +            and objfile.pretty_printers[0].name == "my_library"
    > +        )
    > +
    > +        if all_good:
    > +            gdb.parse_and_eval("all_good = true")
    > +        else:
    > +            print("Oops, not all good:")
    > +            print("pretty printer count: {}".format(len(objfile.pretty_printers)))
    > +
    > +            for pp in objfile.pretty_printers:
    > +                print("  - {}".format(pp.name))
    > +
    > +
    > +gdb.events.new_objfile.connect(new_objfile_handler)
    > -- 
    > 2.30.1
    > 

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Michael Weghorn]

[-- Attachment #1.1: Type: text/plain, Size: 26174 bytes --]

On 29/04/2021 17.39, Vaseeharan Vinayagamoorthy wrote:
> After this commit, I am seeing these errors.
> 
> In file included from src/binutils-gdb--gdb/gdb/auto-load.c:22:0:
> src/binutils-gdb--gdb/gdb/auto-load.h:28:14: error: expected ‘{’ before ‘::’ token
> namespace gdb::observers {
>                ^
> src/binutils-gdb--gdb/gdb/auto-load.h:28:14: error: ‘observers’ in namespace ‘::’ does not name a type
> src/binutils-gdb--gdb/gdb/auto-load.h:49:8: error: ‘observers’ in namespace ‘gdb’ does not name a type
> extern gdb::observers::token auto_load_new_objfile_observer_token;
>          ^
> In file included from /usr/include/c++/4.8.2/set:60:0,
>                   from src/binutils-gdb--gdb/gdb/symtab.h:26,
>                   from src/binutils-gdb--gdb/gdb/infrun.h:21,
>                   from src/binutils-gdb--gdb/gdb/target.h:42,
>                   from src/binutils-gdb--gdb/gdb/progspace.h:24,
>                   from src/binutils-gdb--gdb/gdb/auto-load.c:23:
> /usr/include/c++/4.8.2/bits/stl_tree.h: In constructor ‘gdb::std::_Rb_tree_node<_Val>::_Rb_tree_node(_Args&& ...)’:
> /usr/include/c++/4.8.2/bits/stl_tree.h:140:19: error: ‘forward’ is not a member of ‘gdb::std’
>      _M_value_field(std::forward<_Args>(__args)...) { }
> 
> The build/host/target setup is:
> Build: x86_64 (Linux), using GCC 4.8.
> Host:  x86_64 (Linux)
> Target: arm-none-eabi / aarch64_be-none-elf / aarch64-none-elf

Nested namespace definitions are a C++17 feature and the entry for 
"Nested namespace definitions" at
https://gcc.gnu.org/projects/cxx-status.html suggests that GCC only 
supports that from version 6 on.

Not being involved in GDB development too much myself, I don't know 
whether there's an explicit minimum GCC version required for building GDB.

If necessary, replacing the forward-declaration


   namespace gdb::observers {
   struct token;
   }

by either the more verbose

   namespace gdb {
   namespace observers {
   struct token;
   }
   }

or an

   #include "gdbsupport/observable.h"

should make this work for GCC 4.8 as well.

Regards,
Michael

> 
> 
> Regards
> Vasee
> 
> On 27/04/2021, 09:39, "Gdb-patches on behalf of Andrew Burgess" <gdb-patches-bounces@sourceware.org on behalf of andrew.burgess@embecosm.com> wrote:
> 
>      * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-26 10:53:40 -0400]:
> 
>      > From: Michael Weghorn <m.weghorn@posteo.de>
>      >
>      > Without any explicit dependencies specified, the observers attached
>      > to the 'gdb::observers::new_objfile' observable are always notified
>      > in the order in which they have been attached.
>      >
>      > The new_objfile observer callback to auto-load scripts is attached in
>      > '_initialize_auto_load'.
>      > The new_objfile observer callback that propagates the new_objfile event
>      > to the Python side is attached in 'gdbpy_initialize_inferior', which is
>      > called via '_initialize_python'.
>      > With '_initialize_python' happening before '_initialize_auto_load',
>      > the consequence was that the new_objfile event was emitted on the Python
>      > side before autoloaded scripts had been executed when a new objfile was
>      > loaded.
>      > As a result, trying to access the objfile's pretty printers (defined in
>      > the autoloaded script) from a handler for the Python-side
>      > 'new_objfile' event would fail. Those would only be initialized later on
>      > (when the 'auto_load_new_objfile' callback was called).
>      >
>      > To make sure that the objfile passed to the Python event handler
>      > is properly initialized (including its 'pretty_printers' member),
>      > make sure that the 'auto_load_new_objfile' observer is notified
>      > before the 'python_new_objfile' one that propagates the event
>      > to the Python side.
>      >
>      > To do this, make use of the mechanism to explicitly specify
>      > dependencies between observers (introduced in a preparatory commit).
>      >
>      > Add a corresponding testcase that involves a test library with an autoloaded
>      > Python script and a handler for the Python 'new_objfile' event.
>      >
>      > (The real world use case where I came across this issue was in an attempt
>      > to extend handling for GDB pretty printers for dynamically loaded
>      > objfiles in the Qt Creator IDE, s. [1] and [2] for more background.)
>      >
>      > [1] https://bugreports.qt.io/browse/QTCREATORBUG-25339
>      > [2] https://codereview.qt-project.org/c/qt-creator/qt-creator/+/333857/1
>      >
>      > Tested on x86_64-linux (Debian testing).
>      >
>      > gdb/ChangeLog:
>      >
>      >         * gdb/auto-load.c (_initialize_auto_load): 'Specify token
>      >         when attaching the 'auto_load_new_objfile' observer, so
>      >         other observers can specify it as a dependency.
>      >
>      >         * gdb/auto-load.h (struct token): Declare
>      >         'auto_load_new_objfile_observer_token' as token to be used
>      >         for the 'auto_load_new_objfile' observer.
>      >         * gdb/python/py-inferior.c (gdbpy_initialize_inferior): Make
>      >         'python_new_objfile' observer depend on 'auto_load_new_objfile'
>      >         observer, so it gets notified after the latter.
>      >
>      > gdb/testsuite/ChangeLog:
>      >
>      >         * gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py: New test.
>      >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc: New test.
>      >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h: New test.
>      >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc: New test.
>      >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp: New test.
>      >         * gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py: New test.
>      >
>      > Change-Id: I8275b3f4c3bec32e56dd7892f9a59d89544edf89
>      > ---
>      >  gdb/auto-load.c                               |  9 +-
>      >  gdb/auto-load.h                               |  8 ++
>      >  gdb/python/py-inferior.c                      |  7 +-
>      >  ...tty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++
>      >  ...pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++
>      >  ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
>      >  ...retty-printers-in-newobjfile-event-main.cc | 27 ++++++
>      >  ...ed-pretty-printers-in-newobjfile-event.exp | 85 +++++++++++++++++++
>      >  ...ded-pretty-printers-in-newobjfile-event.py | 50 +++++++++++
>      >  9 files changed, 285 insertions(+), 3 deletions(-)
>      >  create mode 100644 gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
>      >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
>      >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
>      >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
>      >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
>      >  create mode 100644 gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
>      >
>      > diff --git a/gdb/auto-load.c b/gdb/auto-load.c
>      > index 239efa346064..d1ae6deacee7 100644
>      > --- a/gdb/auto-load.c
>      > +++ b/gdb/auto-load.c
>      > @@ -1494,6 +1494,10 @@ found and/or loaded."),
>      >    return &retval;
>      >  }
>      >
>      > +/* See auto-load.h.  */
>      > +
>      > +gdb::observers::token auto_load_new_objfile_observer_token;
>      > +
>      >  void _initialize_auto_load ();
>      >  void
>      >  _initialize_auto_load ()
>      > @@ -1503,8 +1507,9 @@ _initialize_auto_load ()
>      >    char *guile_name_help;
>      >    const char *suffix;
>      >
>      > -  gdb::observers::new_objfile.attach (auto_load_new_objfile, "auto-load");
>      > -
>      > +  gdb::observers::new_objfile.attach (auto_load_new_objfile,
>      > +                                      auto_load_new_objfile_observer_token,
>      > +                                      "auto-load");
>      >    add_setshow_boolean_cmd ("gdb-scripts", class_support,
>      >  			   &auto_load_gdb_scripts, _("\
>      >  Enable or disable auto-loading of canned sequences of commands scripts."), _("\
>      > diff --git a/gdb/auto-load.h b/gdb/auto-load.h
>      > index f726126c5541..4372ec4f4dd7 100644
>      > --- a/gdb/auto-load.h
>      > +++ b/gdb/auto-load.h
>      > @@ -25,6 +25,10 @@ struct program_space;
>      >  struct auto_load_pspace_info;
>      >  struct extension_language_defn;
>      >
>      > +namespace gdb::observers {
>      > +struct token;
>      > +}
>      > +
> 
>      I wonder if we should move the declaration of gdb::observers::token
>      out of observable.h into observable-token.h, then it would be cheap
>      enough to just include observable-token.h into other header files?
> 
>      Otherwise, all looks good.
> 
>      Thanks,
>      Andrew
> 
>      >  /* Value of the 'set debug auto-load' configuration variable.  */
>      >
>      >  extern bool debug_auto_load;
>      > @@ -40,6 +44,10 @@ extern bool auto_load_local_gdbinit;
>      >  extern char *auto_load_local_gdbinit_pathname;
>      >  extern bool auto_load_local_gdbinit_loaded;
>      >
>      > +/* Token used for the auto_load_new_objfile observer, so other observers can
>      > +   specify it as a dependency. */
>      > +extern gdb::observers::token auto_load_new_objfile_observer_token;
>      > +
>      >  extern struct auto_load_pspace_info *
>      >    get_auto_load_pspace_data_for_loading (struct program_space *pspace);
>      >  extern void auto_load_objfile_script (struct objfile *objfile,
>      > diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
>      > index c2861ccb735c..febd2a73ece3 100644
>      > --- a/gdb/python/py-inferior.c
>      > +++ b/gdb/python/py-inferior.c
>      > @@ -18,6 +18,7 @@
>      >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>      >
>      >  #include "defs.h"
>      > +#include "auto-load.h"
>      >  #include "gdbcore.h"
>      >  #include "gdbthread.h"
>      >  #include "inferior.h"
>      > @@ -917,7 +918,11 @@ gdbpy_initialize_inferior (void)
>      >    gdb::observers::register_changed.attach (python_on_register_change,
>      >  					   "py-inferior");
>      >    gdb::observers::inferior_exit.attach (python_inferior_exit, "py-inferior");
>      > -  gdb::observers::new_objfile.attach (python_new_objfile, "py-inferior");
>      > +  /* Need to run after auto-load's new_objfile observer, so that
>      > +     auto-loaded pretty-printers are available.  */
>      > +  gdb::observers::new_objfile.attach
>      > +    (python_new_objfile, "py-inferior",
>      > +     { &auto_load_new_objfile_observer_token });
>      >    gdb::observers::inferior_added.attach (python_new_inferior, "py-inferior");
>      >    gdb::observers::inferior_removed.attach (python_inferior_deleted,
>      >  					   "py-inferior");
>      > diff --git a/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
>      > new file mode 100644
>      > index 000000000000..aeb39a6c483a
>      > --- /dev/null
>      > +++ b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
>      > @@ -0,0 +1,43 @@
>      > +# Copyright (C) 2021 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 that python pretty
>      > +# printers defined in a python script that is autoloaded have been
>      > +# registered when a custom event handler for the new_objfile event
>      > +# is called.
>      > +
>      > +import gdb.printing
>      > +
>      > +
>      > +class MyClassTestLibPrinter(object):
>      > +    "Print a MyClassTestLib"
>      > +
>      > +    def __init__(self, val):
>      > +        self.val = val
>      > +
>      > +    def to_string(self):
>      > +        return "MyClassTestLib object, id: {}".format(self.val["id"])
>      > +
>      > +    def display_hint(self):
>      > +        return "string"
>      > +
>      > +
>      > +def build_pretty_printer():
>      > +    pp = gdb.printing.RegexpCollectionPrettyPrinter("my_library")
>      > +    pp.add_printer("MyClassTestLib", "^MyClassTestLib$", MyClassTestLibPrinter)
>      > +    return pp
>      > +
>      > +
>      > +gdb.printing.register_pretty_printer(gdb.current_objfile(), build_pretty_printer())
>      > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
>      > new file mode 100644
>      > index 000000000000..7f13cd2b741e
>      > --- /dev/null
>      > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
>      > @@ -0,0 +1,28 @@
>      > +/* This testcase is part of GDB, the GNU debugger.
>      > +
>      > +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
>      > +
>      > +MyClassTestLib::MyClassTestLib (int theId)
>      > +{
>      > +  id = theId;
>      > +}
>      > +
>      > +int MyClassTestLib::getId ()
>      > +{
>      > +  return id;
>      > +}
>      > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
>      > new file mode 100644
>      > index 000000000000..3714ecd2ef08
>      > --- /dev/null
>      > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.h
>      > @@ -0,0 +1,31 @@
>      > +/* This testcase is part of GDB, the GNU debugger.
>      > +
>      > +   Copyright 2021 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/>.  */
>      > +
>      > +#ifndef TESTLIBRARY_H
>      > +#define TESTLIBRARY_H
>      > +
>      > +class MyClassTestLib
>      > +{
>      > +public:
>      > +  explicit MyClassTestLib (int theId);
>      > +  int getId ();
>      > +
>      > +private:
>      > +  int id;
>      > +};
>      > +
>      > +#endif /* TESTLIBRARY_H */
>      > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
>      > new file mode 100644
>      > index 000000000000..2cc89a3befd5
>      > --- /dev/null
>      > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
>      > @@ -0,0 +1,27 @@
>      > +/* This testcase is part of GDB, the GNU debugger.
>      > +
>      > +   Copyright 2021 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 "py-autoloaded-pretty-printers-in-newobjfile-event-lib.h"
>      > +
>      > +bool all_good = false;
>      > +
>      > +int
>      > +main ()
>      > +{
>      > +  MyClassTestLib test (1);
>      > +  return 0; /* break to inspect */
>      > +}
>      > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
>      > new file mode 100644
>      > index 000000000000..444466109e8f
>      > --- /dev/null
>      > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
>      > @@ -0,0 +1,85 @@
>      > +# Copyright (C) 2021 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 that Python 
pretty-printers
>      > +# defined in a Python script that is autoloaded are registered when an event
>      > +# handler for the new_objfile event is called.
>      > +
>      > +load_lib gdb-python.exp
>      > +
>      > +standard_testfile -main.cc
>      > +
>      > +set srcfile_lib "${testfile}-lib.cc"
>      > +set python_event_handler_file "${srcdir}/${subdir}/${testfile}.py"
>      > +set libname "lib${testfile}"
>      > +set python_autoload_file "${srcdir}/${subdir}/${libname}.so-gdb.py"
>      > +set binfile_lib [standard_output_file "${libname}.so"]
>      > +
>      > +# Start GDB first - needed for skip_python_tests.
>      > +clean_restart
>      > +
>      > +# Skip all tests if Python scripting is not enabled.
>      > +if { [skip_python_tests] } { continue }
>      > +
>      > +# Compile library.
>      > +if { [gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} \
>      > +      {debug c++}] != "" } {
>      > +    return -1
>      > +}
>      > +
>      > +# Compile main program.
>      > +if { [gdb_compile ${srcdir}/${subdir}/${srcfile} \
>      > +      ${binfile} \
>      > +      executable \
>      > +      [list debug c++ shlib=$binfile_lib]] != "" } {
>      > +    return -1
>      > +}
>      > +
>      > +# Make the -gdb.py script available to gdb, it is automatically 
loaded by
>      > +# gdb if it is put in the same directory as the library.
>      > +set remote_python_autoload_file \
>      > +    [gdb_remote_download host $python_autoload_file]
>      > +
>      > +gdb_test_no_output \
>      > +    "set auto-load safe-path ${remote_python_autoload_file}" \
>      > +    "set auto-load safe-path"
>      > +
>      > +# Load the Python file that defines a handler for the new_objfile event,
>      > +# which will generate the output to check later
>      > +# (prints information on available pretty-printers for objfile).
>      > +set remote_python_event_handler_file\
>      > +    [gdb_remote_download host $python_event_handler_file]
>      > +gdb_test_no_output "source ${remote_python_event_handler_file}" 
"load python file"
>      > +
>      > +gdb_load ${binfile}
>      > +
>      > +gdb_test_no_output "set print pretty on"
>      > +
>      > +# Check that the handler prints output when test library is loaded
>      > +# and that the pretty-printer from the auto-loaded Python file has been
>      > +# registered.
>      > +if { ![runto_main] } {
>      > +    fail "failed to run to main"
>      > +    return
>      > +}
>      > +
>      > +# Check that the new_objfile handler saw the pretty-printer.
>      > +gdb_test "print all_good" " = true"
>      > +
>      > +# Check that the pretty-printer actually works.
>      > +gdb_test "info pretty-printer" "my_library.*MyClassTestLib.*"
>      > +gdb_breakpoint [gdb_get_line_number "break to inspect"]
>      > +gdb_test "continue" "Breakpoint $decimal, main .*"
>      > +gdb_test "print test" "MyClassTestLib object, id: 1.*"
>      > diff --git a/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
>      > new file mode 100644
>      > index 000000000000..85d60fc51c31
>      > --- /dev/null
>      > +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
>      > @@ -0,0 +1,50 @@
>      > +# Copyright (C) 2021 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 that python pretty
>      > +# printers defined in a python script that is autoloaded have been
>      > +# registered when a custom event handler for the new_objfile event
>      > +# is called.
>      > +
>      > +import gdb
>      > +import os
>      > +
>      > +
>      > +def new_objfile_handler(event):
>      > +    assert isinstance(event, gdb.NewObjFileEvent)
>      > +    objfile = event.new_objfile
>      > +
>      > +    # Only observe the custom test library.
>      > +    libname = "libpy-autoloaded-pretty-printers-in-newobjfile-event"
>      > +    if libname in os.path.basename(objfile.filename):
>      > +        # If everything went well and the pretty-printer auto-load happened
>      > +        # before notifying the Python listeners, we expect to see one pretty
>      > +        # printer, and it must be ours.
>      > +        all_good = (
>      > +            len(objfile.pretty_printers) == 1
>      > +            and objfile.pretty_printers[0].name == "my_library"
>      > +        )
>      > +
>      > +        if all_good:
>      > +            gdb.parse_and_eval("all_good = true")
>      > +        else:
>      > +            print("Oops, not all good:")
>      > +            print("pretty printer count: {}".format(len(objfile.pretty_printers)))
>      > +
>      > +            for pp in objfile.pretty_printers:
>      > +                print("  - {}".format(pp.name))
>      > +
>      > +
>      > +gdb.events.new_objfile.connect(new_objfile_handler)
>      > --
>      > 2.30.1
>      >
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v5 2/2] gdb: do autoload before notifying Python side in new_objfile event

[Simon Marchi]

On 2021-04-29 3:41 p.m., Michael Weghorn wrote:
> On 29/04/2021 17.39, Vaseeharan Vinayagamoorthy wrote:
>> After this commit, I am seeing these errors.
>>
>> In file included from src/binutils-gdb--gdb/gdb/auto-load.c:22:0:
>> src/binutils-gdb--gdb/gdb/auto-load.h:28:14: error: expected ‘{’ before ‘::’ token
>> namespace gdb::observers {
>>                ^
>> src/binutils-gdb--gdb/gdb/auto-load.h:28:14: error: ‘observers’ in namespace ‘::’ does not name a type
>> src/binutils-gdb--gdb/gdb/auto-load.h:49:8: error: ‘observers’ in namespace ‘gdb’ does not name a type
>> extern gdb::observers::token auto_load_new_objfile_observer_token;
>>          ^
>> In file included from /usr/include/c++/4.8.2/set:60:0,
>>                   from src/binutils-gdb--gdb/gdb/symtab.h:26,
>>                   from src/binutils-gdb--gdb/gdb/infrun.h:21,
>>                   from src/binutils-gdb--gdb/gdb/target.h:42,
>>                   from src/binutils-gdb--gdb/gdb/progspace.h:24,
>>                   from src/binutils-gdb--gdb/gdb/auto-load.c:23:
>> /usr/include/c++/4.8.2/bits/stl_tree.h: In constructor ‘gdb::std::_Rb_tree_node<_Val>::_Rb_tree_node(_Args&& ...)’:
>> /usr/include/c++/4.8.2/bits/stl_tree.h:140:19: error: ‘forward’ is not a member of ‘gdb::std’
>>      _M_value_field(std::forward<_Args>(__args)...) { }
>>
>> The build/host/target setup is:
>> Build: x86_64 (Linux), using GCC 4.8.
>> Host:  x86_64 (Linux)
>> Target: arm-none-eabi / aarch64_be-none-elf / aarch64-none-elf
> 
> Nested namespace definitions are a C++17 feature and the entry for "Nested namespace definitions" at
> https://gcc.gnu.org/projects/cxx-status.html suggests that GCC only supports that from version 6 on.
> 
> Not being involved in GDB development too much myself, I don't know whether there's an explicit minimum GCC version required for building GDB.
> 
> If necessary, replacing the forward-declaration
> 
> 
>   namespace gdb::observers {
>   struct token;
>   }
> 
> by either the more verbose
> 
>   namespace gdb {
>   namespace observers {
>   struct token;
>   }
>   }
> 
> or an
> 
>   #include "gdbsupport/observable.h"
> 
> should make this work for GCC 4.8 as well.

We currently require a C++11 compiler.  My bad, I didn't realize that
this was a C++17 feature, and that my compiler was using C++ > 11 as the
default.  I'll push a fix shortly.

Simon