From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Michael Weghorn <m.weghorn@posteo.de>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: Change init order so pretty printers are set in new_objfile event
Date: Thu, 11 Feb 2021 09:42:34 +0000 [thread overview]
Message-ID: <20210211094234.GM265215@embecosm.com> (raw)
In-Reply-To: <20210210154053.82927-1-m.weghorn@posteo.de>
* Michael Weghorn via Gdb-patches <gdb-patches@sourceware.org> [2021-02-10 16:40:53 +0100]:
> The observers attached to the 'gdb::observers::new_objfile' observable
> are notified in the order in which they have been attached (s. class
> 'observable' in gdbsupport/observable.h).
>
> 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 the previous initialization order, '_initialize_python' happened
> before '_initialize_auto_load', with the consequence 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), change
> the initialization order so that autoloading happens before the
> propagation of the 'new_objfile' event to the Python side.
>
> The initialization happens in 'initialize_all_files' in 'gdb/init.c'
> which is autogenerated by Makefile 'gdb/Makefile.in', so change the
> order there accordingly by moving the 'CONFIG_OBS' part (which includes
> the Python part) after the 'COMMON_SFILES' one (which includes the
> autoload part).
I have a crazy thought, but I want to mention it in case it has any
value.
The core of the idea is to modify the attach functions in
gdbsupport/observable.h like this:P
void attach (const func_type &f, int bias = 0)
{
m_observers.emplace_back (nullptr, f, bias);
if (m_sorted || bias != 0)
{
m_sorted = true;
std::sort (m_observers.begin (), m_observers.end (), sort_by_bias);
}
}
Where 'm_sorted' is just a boolean flag, and `sort_by_bias` just
orders the observers from highest bias to lowest. Then, if you have
an observer that you want to run late in the process you just attach
it with a low bias. So in py-inferior.c:
/* Pass a low bias to ensure this observer triggers after any
GDB internal observers. */
gdb::observers::new_objfile.attach (python_new_objfile, -10);
That said, I looked through the patch and it all looked good except
for a couple of nits, pointed out below.
Thanks,
Andrew
>
> 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 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:
>
> * Makefile.in: Change order so initialization for 'CONFIG_OBS'
> happens after 'COMMON_SFILES' so e.g. autoloaded Python pretty
> printers are available in handler for Python new_objfile event handler.
>
> 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.
> ---
> gdb/Makefile.in | 3 +-
> ...tty-printers-in-newobjfile-event.so-gdb.py | 44 +++++++++
> ...pretty-printers-in-newobjfile-event-lib.cc | 26 ++++++
> ...-pretty-printers-in-newobjfile-event-lib.h | 31 +++++++
> ...retty-printers-in-newobjfile-event-main.cc | 23 +++++
> ...ed-pretty-printers-in-newobjfile-event.exp | 93 +++++++++++++++++++
> ...ded-pretty-printers-in-newobjfile-event.py | 34 +++++++
> 7 files changed, 253 insertions(+), 1 deletion(-)
> 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/Makefile.in b/gdb/Makefile.in
> index 36ef45d4559..2c34714b66d 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1573,11 +1573,12 @@ TAGFILES_NO_SRCDIR = $(SFILES) $(HFILES_NO_SRCDIR) $(ALLDEPFILES) \
> $(CONFIG_SRCS)
> TAGFILES_WITH_SRCDIR = $(HFILES_WITH_SRCDIR)
>
> -COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
> +COMMON_OBS = $(DEPFILES) $(YYOBJ) \
> mi/mi-common.o \
> version.o \
> xml-builtin.o \
> $(patsubst %.c,%.o,$(COMMON_SFILES)) \
> + $(CONFIG_OBS) \
> $(SUBDIR_CLI_OBS) \
> $(SUBDIR_TARGET_OBS) \
> $(SUBDIR_GCC_COMPILE_OBS)
> 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..2327e4a7384
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/libpy-autoloaded-pretty-printers-in-newobjfile-event.so-gdb.py
> @@ -0,0 +1,44 @@
> +# 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..7e06cf3903f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-lib.cc
> @@ -0,0 +1,26 @@
> +/* 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..fa2501bf6de
> --- /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..6e66bbe3d43
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event-main.cc
> @@ -0,0 +1,23 @@
> +/* 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"
> +
> +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..28ca712a3ce
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.exp
> @@ -0,0 +1,93 @@
> +# 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.
> +
> +if [use_gdb_stub] {
> + return 0
> +}
> +
> +load_lib gdb-python.exp
> +
> +set test "py-autoloaded-pretty-printers-in-newobjfile-event"
> +set srcfile_main "${test}-main.cc"
> +set executable_main ${test}-test
> +set binfile_main [standard_output_file ${executable_main}]
> +set srcfile_lib "${test}-lib.cc"
> +set libname "lib${test}"
> +set pyscriptfile_lib "${libname}.so-gdb.py"
> +set binfile_lib [standard_output_file ${libname}.so]
> +
> +
> +# Start with a fresh gdb.
> +gdb_exit
> +gdb_start
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +if {[gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} [list debug c++ -Wl,-soname,${libname}.so]] != ""} {
This long line should probably be split.
> + return -1
> +}
> +
> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile_main}" "${binfile_main}.o" object [list debug c++]] != ""} {
And again.
> + return -1
> +}
> +
> +set testobjdir [standard_output_file {}]
> +if {[gdb_compile "${binfile_main}.o" "${binfile_main}" executable \
> + [list debug "additional_flags=-L$testobjdir -l${test} \
> + -Wl,-rpath=$testobjdir"]] != ""} {
If feels like you could reduce the whitespace on the last line here to
keep it under 80 characters.
> + return -1
> +}
> +
> +# Start with a fresh gdb.
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +
> +# 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_file_autoload [gdb_remote_download host \
> + ${srcdir}/${subdir}/${pyscriptfile_lib}]
> +
> +gdb_test_no_output "set auto-load safe-path ${remote_python_file_autoload}" "set auto-load safe-path"
Long line again.
> +
> +# 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_file [gdb_remote_download host \
> + ${srcdir}/${subdir}/${test}.py]
> +gdb_test_no_output "source ${remote_python_file}" "load python file"
> +
> +gdb_load ${binfile_main}
> +
> +gdb_test_no_output "set print pretty on"
> +
> +gdb_breakpoint [gdb_get_line_number "break to inspect" ${srcfile_main}]
> +
> +# check that the handler prints output when test library is loaded
> +# and that the pretty printer from autoloaded python file has been registered
> +gdb_test "run" "new_objfile event for test library.*
> +.*number of pretty printers: 1.*
> +.*name of the first pretty printer: my_library.*"
> +
> +# check that pretty printer actually works
> +gdb_test "print test" " .*MyClassTestLib object, id: 1.*"
> +
> +gdb_continue_to_end
> 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..924f304fd33
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-autoloaded-pretty-printers-in-newobjfile-event.py
> @@ -0,0 +1,34 @@
> +# 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))
> + # only observe the custom test library
> + if os.path.basename(event.new_objfile.filename).find("libpy-autoloaded-pretty-printers-in-newobjfile-event") == 0:
> + print("new_objfile event for test library")
> + assert(len(event.new_objfile.pretty_printers) == 1)
> + print("number of pretty printers: {}".format(len(event.new_objfile.pretty_printers)))
> + print("name of the first pretty printer: {}".format(event.new_objfile.pretty_printers[0].name))
> +
> +gdb.events.new_objfile.connect(new_objfile_handler)
> --
> 2.30.0
>
next prev parent reply other threads:[~2021-02-11 9:42 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-10 15:40 Michael Weghorn
2021-02-11 9:42 ` Andrew Burgess [this message]
2021-02-11 11:23 ` Michael Weghorn
2021-03-02 7:18 ` Michael Weghorn
2021-03-23 18:55 ` Simon Marchi
2021-03-24 9:45 ` Andrew Burgess
2021-03-24 13:51 ` Simon Marchi
2021-03-26 8:33 ` Michael Weghorn
2021-02-11 11:22 ` [PATCH v2] " Michael Weghorn
2021-03-23 8:01 ` [PING] " Michael Weghorn
2021-03-26 8:29 ` [PATCH v3] gdb: Do autoload before notifying Python side " Michael Weghorn
2021-04-12 13:37 ` [PING] " Michael Weghorn
2021-04-20 11:38 ` [PING 2] " Michael Weghorn
2021-04-20 21:57 ` Simon Marchi
2021-04-22 15:46 ` Michael Weghorn
2021-04-22 16:06 ` Simon Marchi
2021-04-23 6:41 ` Michael Weghorn
2021-04-23 10:48 ` Simon Marchi
2021-04-22 15:44 ` [PATCH v4 0/2] Make sure autoload happens " Michael Weghorn
2021-04-22 15:44 ` [PATCH v4 1/2] gdbsupport: Allow to specify dependencies between observers Michael Weghorn
2021-04-22 15:44 ` [PATCH v4 2/2] gdb: Do autoload before notifying Python side in new_objfile event Michael Weghorn
2021-04-25 1:46 ` [PATCH v4 0/2] Make sure autoload happens " Simon Marchi
2021-04-26 8:18 ` Michael Weghorn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210211094234.GM265215@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
--cc=m.weghorn@posteo.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).