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

Andrew Burgess andrew.burgess@embecosm.com
Tue Apr 27 08:39:23 GMT 2021


* 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
> 


More information about the Gdb-patches mailing list