[binutils-gdb] gdb: do autoload before notifying Python side in new_objfile event

[binutils-gdb] gdb: do autoload before notifying Python side in new_objfile event

[Simon Marchi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2c473def12b08100e6b56261f01112db7f6aeab5

commit 2c473def12b08100e6b56261f01112db7f6aeab5
Author: Michael Weghorn <m.weghorn@posteo.de>
Date:   Tue Apr 27 10:02:42 2021 -0400

    gdb: do autoload before notifying Python side in new_objfile event
    
    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

Diff:
---
 gdb/ChangeLog                                      | 13 ++++
 gdb/auto-load.c                                    |  9 ++-
 gdb/auto-load.h                                    |  8 +++
 gdb/python/py-inferior.c                           |  7 +-
 gdb/testsuite/ChangeLog                            | 10 +++
 ...d-pretty-printers-in-newobjfile-event.so-gdb.py | 43 ++++++++++++
 ...aded-pretty-printers-in-newobjfile-event-lib.cc | 28 ++++++++
 ...oaded-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++++
 ...ded-pretty-printers-in-newobjfile-event-main.cc | 27 ++++++++
 ...oloaded-pretty-printers-in-newobjfile-event.exp | 80 ++++++++++++++++++++++
 ...toloaded-pretty-printers-in-newobjfile-event.py | 50 ++++++++++++++
 11 files changed, 303 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cdad68a7254..e8f7ce59170 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2021-04-27  Michael Weghorn  <m.weghorn@posteo.de>
+	    Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* 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.
+
 2021-04-27  Michael Weghorn  <m.weghorn@posteo.de>
 	    Simon Marchi  <simon.marchi@polymtl.ca>
 
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 239efa34606..d1ae6deacee 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 f726126c554..4372ec4f4dd 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 c2861ccb735..febd2a73ece 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/ChangeLog b/gdb/testsuite/ChangeLog
index 45bca311a39..10c77864fe6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+2021-04-27  Michael Weghorn  <m.weghorn@posteo.de>
+	    Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* 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.
+
 2021-04-26  Tom Tromey  <tromey@adacore.com>
 
 	PR gdb/27743:
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 00000000000..aeb39a6c483
--- /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 00000000000..7f13cd2b741
--- /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 00000000000..3714ecd2ef0
--- /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 00000000000..2cc89a3befd
--- /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 00000000000..96ae41b0752
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
@@ -0,0 +1,80 @@
+# 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.
+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"
+
+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 00000000000..1d9c75bd461
--- /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 = 1")
+        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)