From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id 440C73861838 for ; Tue, 15 Jun 2021 23:41:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 440C73861838 Received: from Plymouth (unknown [IPv6:2a02:390:9086:0:536c:722b:f4d9:ae6f]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id C9898827B7; Tue, 15 Jun 2021 23:41:38 +0000 (UTC) Date: Wed, 16 Jun 2021 00:41:34 +0100 From: Lancelot SIX To: Felix Willgerodt Cc: markus.t.metzger@intel.com, gdb-patches@sourceware.org Subject: Re: [PATCH v2 09/12] btrace, python: Enable ptwrite listener registration. Message-ID: <20210615234134.lez2lnpnpzsc73pn@Plymouth> References: <20210614145411.689277-1-felix.willgerodt@intel.com> <20210614145411.689277-5-felix.willgerodt@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210614145411.689277-5-felix.willgerodt@intel.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Tue, 15 Jun 2021 23:41:39 +0000 (UTC) X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Jun 2021 23:41:43 -0000 Hi I have few remarks later in the patch. On Mon, Jun 14, 2021 at 04:54:08PM +0200, Felix Willgerodt via Gdb-patches wrote: > With this patch a default ptwrite listener is registered upon start of GDB. > It prints the plain ptwrite payload as hex. The default listener can be > overwritten by registering a custom listener in python or by registering > "None", for no output at all. Registering a listener function creates per > thread copies to allow unique internal states per thread. > > gdb/ChangeLog: > 2021-06-14 Felix Willgerodt > > * btrace.c (ftrace_add_pt): Add call to > apply_ext_lang_ptwrite_listener. > * btrace.h (btrace_thread_info): New member ptw_listener. > * data-directory/Makefile.in: Add ptwrite.py. > * extension-priv.h (struct extension_language_ops): New member > apply_ptwrite_listener. > * extension.c (apply_ext_lang_ptwrite_listener): New function. > * extension.h (apply_ext_lang_ptwrite_listener): New declaration. > * guile/guile.c (guile_extension_ops): Add > gdbscm_load_ptwrite_listener. > * python/lib/gdb/ptwrite.py: New file. > * python/py-record-btrace.c (recpy_initialize_listener, > get_ptwrite_listener): New function. > * python/py-record-btrace.h (recpy_initialize_listener): New > declaration. > * python/py-record.c (gdbpy_load_ptwrite_listener): New function. > * python/python-internal.h (gdbpy_load_ptwrite_listener): New > declaration. > * python/python.c (python_extension_ops): New member > gdbpy_load_ptwrite_listener. > --- > gdb/btrace.c | 3 ++ > gdb/btrace.h | 3 ++ > gdb/data-directory/Makefile.in | 1 + > gdb/extension-priv.h | 4 ++ > gdb/extension.c | 21 +++++++++ > gdb/extension.h | 3 ++ > gdb/guile/guile.c | 1 + > gdb/python/lib/gdb/ptwrite.py | 86 ++++++++++++++++++++++++++++++++++ > gdb/python/py-record-btrace.c | 67 ++++++++++++++++++++++++++ > gdb/python/py-record-btrace.h | 6 +++ > gdb/python/python-internal.h | 3 ++ > gdb/python/python.c | 2 + > 12 files changed, 200 insertions(+) > create mode 100644 gdb/python/lib/gdb/ptwrite.py > > diff --git a/gdb/btrace.c b/gdb/btrace.c > index 2c4f3f67c6b..d66c3e21f11 100644 > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -34,6 +34,7 @@ > #include "gdbsupport/rsp-low.h" > #include "gdbcmd.h" > #include "cli/cli-utils.h" > +#include "extension.h" > #include "gdbarch.h" > > /* For maintenance commands. */ > @@ -1311,6 +1312,8 @@ ftrace_add_pt (struct btrace_thread_info *btinfo, > uint64_t offset; > int status; > > + apply_ext_lang_ptwrite_listener (inferior_ptid); > + > for (;;) > { > struct pt_insn insn; > diff --git a/gdb/btrace.h b/gdb/btrace.h > index e6694551c70..b15efcd147c 100644 > --- a/gdb/btrace.h > +++ b/gdb/btrace.h > @@ -352,6 +352,9 @@ struct btrace_thread_info > stepping through the execution history. */ > std::vector aux_data; > > + /* PyObject pointer to the ptwrite listener function. */ > + void *ptw_listener = nullptr; > + > /* The function level offset. When added to each function's LEVEL, > this normalizes the function levels such that the smallest level > becomes zero. */ > diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in > index 888325f974e..c759920a343 100644 > --- a/gdb/data-directory/Makefile.in > +++ b/gdb/data-directory/Makefile.in > @@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \ > gdb/frames.py \ > gdb/printing.py \ > gdb/prompt.py \ > + gdb/ptwrite.py \ > gdb/types.py \ > gdb/unwinder.py \ > gdb/xmethod.py \ > diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h > index 77f23e0f911..36da4211738 100644 > --- a/gdb/extension-priv.h > +++ b/gdb/extension-priv.h > @@ -183,6 +183,10 @@ struct extension_language_ops > enum ext_lang_frame_args args_type, > struct ui_out *out, int frame_low, int frame_high); > > + /* Used for registering the ptwrite listener to the current thread. */ > + enum ext_lang_bt_status (*apply_ptwrite_listener) > + (const struct extension_language_defn *, ptid_t inferior_ptid); > + > /* Update values held by the extension language when OBJFILE is discarded. > New global types must be created for every such value, which must then be > updated to use the new types. > diff --git a/gdb/extension.c b/gdb/extension.c > index 27dce9befa0..826cb0285c3 100644 > --- a/gdb/extension.c > +++ b/gdb/extension.c > @@ -550,6 +550,27 @@ apply_ext_lang_frame_filter (struct frame_info *frame, > return EXT_LANG_BT_NO_FILTERS; > } > > +/* Used for registering the ptwrite listener to the current thread. */ > + > +enum ext_lang_bt_status > +apply_ext_lang_ptwrite_listener (ptid_t inferior_ptid) > +{ > + for (const struct extension_language_defn *extlang : extension_languages) > + { > + enum ext_lang_bt_status status; > + > + if (extlang->ops == nullptr > + || extlang->ops->apply_ptwrite_listener == NULL) > + continue; > + status = extlang->ops->apply_ptwrite_listener (extlang, inferior_ptid); > + > + if (status != EXT_LANG_BT_NO_FILTERS) > + return status; > + } > + > + return EXT_LANG_BT_NO_FILTERS; > +} > + > /* Update values held by the extension language when OBJFILE is discarded. > New global types must be created for every such value, which must then be > updated to use the new types. > diff --git a/gdb/extension.h b/gdb/extension.h > index 56f57560de3..97562304ae4 100644 > --- a/gdb/extension.h > +++ b/gdb/extension.h > @@ -295,6 +295,9 @@ extern enum ext_lang_bt_status apply_ext_lang_frame_filter > enum ext_lang_frame_args args_type, > struct ui_out *out, int frame_low, int frame_high); > > +enum ext_lang_bt_status apply_ext_lang_ptwrite_listener > + (ptid_t inferior_ptid); > + > extern void preserve_ext_lang_values (struct objfile *, htab_t copied_types); > > extern const struct extension_language_defn *get_breakpoint_cond_ext_lang > diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c > index a28dee37ed8..ef784885e87 100644 > --- a/gdb/guile/guile.c > +++ b/gdb/guile/guile.c > @@ -124,6 +124,7 @@ static const struct extension_language_ops guile_extension_ops = > gdbscm_apply_val_pretty_printer, > > NULL, /* gdbscm_apply_frame_filter, */ > + NULL, /* gdbscm_load_ptwrite_listener, */ > > gdbscm_preserve_values, > > diff --git a/gdb/python/lib/gdb/ptwrite.py b/gdb/python/lib/gdb/ptwrite.py > new file mode 100644 > index 00000000000..e52b1e7c88e > --- /dev/null > +++ b/gdb/python/lib/gdb/ptwrite.py > @@ -0,0 +1,86 @@ > +# Ptwrite utilities. > +# Copyright (C) 2020 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 . > + > +"""Utilities for working with ptwrite listeners.""" > + > +import gdb > +from copy import deepcopy > + > + > +def default_listener(payload, ip): > + """Default listener that is active upon starting GDB.""" > + return "{:x}".format(payload) > + > +# This dict contains the per thread copies of the listener function and the > +# global template listener, from which the copies are created. > +_ptwrite_listener = {"global" : default_listener} > + > + > +def _update_listener_dict(thread_list): > + """Helper function to update the listener dict. > + > + Discards listener copies of threads that already exited and registers > + copies of the listener for new threads.""" > + # thread_list[x].ptid returns the tuple (pid, lwp, tid) > + lwp_list = [i.ptid[1] for i in thread_list] > + > + # clean-up old listeners > + for key in _ptwrite_listener.keys(): > + if key not in lwp_list and not "global": > + _ptwrite_listener.pop(key) Did you mean if key not in lwp_list and key != "global": ? Otherwise, `not "global"` will always evaluate to `False`, and the pop call will never be evaluated. > + > + # Register listener for new threads > + for key in lwp_list: > + if key not in _ptwrite_listener.keys(): > + _ptwrite_listener[key] = deepcopy(_ptwrite_listener["global"]) > + > + > +def _clear_traces(thread_list): > + """Helper function to clear the trace of all threads.""" > + current_thread = gdb.selected_thread() > + > + recording = gdb.current_recording() > + > + if (recording is not None): > + for thread in thread_list: > + thread.switch() > + recording.clear() > + > + current_thread.switch() > + > + > +def register_listener(listener): > + """Register the ptwrite listener function.""" > + global _ptwrite_listener Since _ptwrite_listener is never (re)assigned in this function, the global statement is not necessary. It could be dropped. I guess at some point a new dict was assigned to _ptwrite_listener, it eventually got replaced with the `clear` call and the `global` remained. Best, Lancelot. > + > + if listener is not None and not callable(listener): > + raise TypeError("Listener must be callable!") > + > + thread_list = gdb.Inferior.threads(gdb.selected_inferior()) > + _clear_traces(thread_list) > + > + _ptwrite_listener.clear() > + _ptwrite_listener["global"] = listener > + > + _update_listener_dict(thread_list) > + > + > +def get_listener(): > + """Returns the listeners of the current thread.""" > + thread_list = gdb.Inferior.threads(gdb.selected_inferior()) > + _update_listener_dict(thread_list) > + > + return _ptwrite_listener[gdb.selected_thread().ptid[1]] > diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c > index 97649b206a5..82f0e00d675 100644 > --- a/gdb/python/py-record-btrace.c > +++ b/gdb/python/py-record-btrace.c > @@ -776,6 +776,73 @@ recpy_bt_function_call_history (PyObject *self, void *closure) > return btpy_list_new (tinfo, first, last, 1, &recpy_func_type); > } > > +/* Helper function returning the current ptwrite listener. Returns nullptr > + in case of errors. */ > + > +PyObject * > +get_ptwrite_listener () > +{ > + PyObject *module = PyImport_ImportModule ("gdb.ptwrite"); > + > + if (PyErr_Occurred ()) > + { > + gdbpy_print_stack (); > + return nullptr; > + } > + > + PyObject *default_ptw_listener = PyObject_CallMethod (module, "get_listener", > + NULL); > + > + gdb_Py_DECREF (module); > + > + if (PyErr_Occurred ()) > + gdbpy_print_stack (); > + > + return default_ptw_listener; > +} > + > +/* Helper function to initialize the ptwrite listener. Returns nullptr in > + case of errors. */ > + > +PyObject * > +recpy_initialize_listener (ptid_t inferior_ptid) > +{ > + process_stratum_target *proc_target = current_inferior ()->process_target (); > + struct thread_info * const tinfo = find_thread_ptid (proc_target, inferior_ptid); > + > + tinfo->btrace.ptw_listener = get_ptwrite_listener (); > + > + return (PyObject *) tinfo->btrace.ptw_listener; > +} > + > +/* Used for registering the default ptwrite listener to the current thread. A > + pointer to this function is stored in the python extension interface. */ > + > +enum ext_lang_bt_status > +gdbpy_load_ptwrite_listener (const struct extension_language_defn *extlang, > + ptid_t inferior_ptid) > +{ > + struct gdbarch *gdbarch = NULL; > + > + if (!gdb_python_initialized) > + return EXT_LANG_BT_NO_FILTERS; > + > + try > + { > + gdbarch = target_gdbarch (); > + } > + catch (const gdb_exception &except) > + { > + return EXT_LANG_BT_NO_FILTERS; > + } > + > + gdbpy_enter enter_py (gdbarch, current_language); > + > + recpy_initialize_listener (inferior_ptid); > + > + return EXT_LANG_BT_OK; > +} > + > /* Implementation of BtraceRecord.goto (self, BtraceInstruction) -> None. */ > > PyObject * > diff --git a/gdb/python/py-record-btrace.h b/gdb/python/py-record-btrace.h > index adb03c5eab4..36389e99ab8 100644 > --- a/gdb/python/py-record-btrace.h > +++ b/gdb/python/py-record-btrace.h > @@ -91,4 +91,10 @@ extern PyObject *recpy_bt_func_prev (PyObject *self, void *closure); > /* Implementation of RecordFunctionSegment.next [RecordFunctionSegment]. */ > extern PyObject *recpy_bt_func_next (PyObject *self, void *closure); > > +/* Helper function to initialize the ptwrite listener. */ > +extern PyObject *recpy_initialize_listener (ptid_t inferior_ptid); > + > +/* Helper function returning the current ptwrite listener. */ > +extern PyObject *get_ptwrite_listener (); > + > #endif /* PYTHON_PY_RECORD_BTRACE_H */ > diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h > index 690d2fb43c0..fcbd7a0b1b8 100644 > --- a/gdb/python/python-internal.h > +++ b/gdb/python/python-internal.h > @@ -392,6 +392,9 @@ extern enum ext_lang_rc gdbpy_apply_val_pretty_printer > struct ui_file *stream, int recurse, > const struct value_print_options *options, > const struct language_defn *language); > +extern enum ext_lang_bt_status gdbpy_load_ptwrite_listener > + (const struct extension_language_defn *extlang, > + ptid_t inferior_ptid); > extern enum ext_lang_bt_status gdbpy_apply_frame_filter > (const struct extension_language_defn *, > struct frame_info *frame, frame_filter_flags flags, > diff --git a/gdb/python/python.c b/gdb/python/python.c > index e42cbc4fd5e..ad23532f9d8 100644 > --- a/gdb/python/python.c > +++ b/gdb/python/python.c > @@ -174,6 +174,8 @@ const struct extension_language_ops python_extension_ops = > > gdbpy_apply_frame_filter, > > + gdbpy_load_ptwrite_listener, > + > gdbpy_preserve_values, > > gdbpy_breakpoint_has_cond, > -- > 2.25.4 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 > -- Lancelot SIX