[v9,09/10] btrace, python: Enable ptwrite filter registration.
Commit Message
By default GDB will be printing the hex payload of the ptwrite package as
auxiliary information. To customize this, the user can register a ptwrite
filter function in python, that takes the payload and the PC as arguments and
returns a string which will be printed instead. Registering the filter
function is done using a factory pattern to make per-thread filtering easier.
---
gdb/btrace.c | 4 ++
gdb/btrace.h | 11 ++++
gdb/data-directory/Makefile.in | 1 +
gdb/extension-priv.h | 5 ++
gdb/extension.c | 13 +++++
gdb/extension.h | 3 ++
gdb/guile/guile.c | 1 +
gdb/python/lib/gdb/ptwrite.py | 77 ++++++++++++++++++++++++++++
gdb/python/py-record-btrace.c | 91 ++++++++++++++++++++++++++++++++++
gdb/python/python-internal.h | 3 ++
gdb/python/python.c | 2 +
11 files changed, 211 insertions(+)
create mode 100644 gdb/python/lib/gdb/ptwrite.py
Comments
>>>>> Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:
> By default GDB will be printing the hex payload of the ptwrite package as
> auxiliary information. To customize this, the user can register a ptwrite
> filter function in python, that takes the payload and the PC as arguments and
> returns a string which will be printed instead. Registering the filter
> function is done using a factory pattern to make per-thread filtering easier.
Thank you for the patch.
> + /* Function pointer to the ptwrite callback. Returns the string returned
> + by the ptwrite filter function. */
> + gdb::optional<std::string> (*ptw_callback_fun) (const uint64_t payload,
> + const uint64_t ip,
> + const void *ptw_context)
> + = nullptr;
> +
> + /* Context for the ptw_callback_fun. */
> + void *ptw_context = nullptr;
I tend to think it's better to use std::function here, rather than the
C-style pointer-and-data approach.
> diff --git a/gdb/python/lib/gdb/ptwrite.py b/gdb/python/lib/gdb/ptwrite.py
I gather the new stuff is documented in the next patch.
> +# _ptwrite_filter contains the per thread copies of the filter function.
> +# The keys are tuples of inferior id and thread id.
> +# The filter functions are created for each thread by calling the
> +# _ptwrite_filter_factory.
> +_ptwrite_filter = {}
It seems like when an inferior or thread is destroyed, the entries
should be removed from this map.
> + gdbpy_ref<> py_payload (PyLong_FromUnsignedLongLong (payload));
> + gdbpy_ref<> py_ip (PyLong_FromUnsignedLongLong (ip));
Both of these calls require null checks.
> + gdbpy_ref<> py_result (PyObject_CallFunctionObjArgs ((PyObject *) ptw_filter,
> + py_payload.get (),
> + py_ip.get (),
> + nullptr));
> +
> + if (PyErr_Occurred ())
It's more idiomatic to check == nullptr than PyErr_Occurred.
> + result = gdbpy_obj_to_string (py_result.get ()).get ();
> +
> + if (PyErr_Occurred ())
Here too.
Tom
> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Donnerstag, 6. Juli 2023 18:31
> To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Metzger, Markus T <markus.t.metzger@intel.com>; simark@simark.ca;
> Willgerodt, Felix <felix.willgerodt@intel.com>
> Subject: Re: [PATCH v9 09/10] btrace, python: Enable ptwrite filter
> registration.
>
> >>>>> Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>
> writes:
>
> > By default GDB will be printing the hex payload of the ptwrite package as
> > auxiliary information. To customize this, the user can register a ptwrite
> > filter function in python, that takes the payload and the PC as arguments
> and
> > returns a string which will be printed instead. Registering the filter
> > function is done using a factory pattern to make per-thread filtering easier.
>
> Thank you for the patch.
>
> > + /* Function pointer to the ptwrite callback. Returns the string returned
> > + by the ptwrite filter function. */
> > + gdb::optional<std::string> (*ptw_callback_fun) (const uint64_t payload,
> > + const uint64_t ip,
> > + const void *ptw_context)
> > + = nullptr;
> > +
> > + /* Context for the ptw_callback_fun. */
> > + void *ptw_context = nullptr;
>
> I tend to think it's better to use std::function here, rather than the
> C-style pointer-and-data approach.
I had this comment before in this series.
In the end I need to call PyObject_CallFunctionObjArgs() with ptw_context.
This function requires a function pointer, as it is C, not C++.
Then there is also the topic of the GDB extension language interface, if you
are also talking about ptw_callback_fun.
There GDB also uses function pointers right now. We can of course change
that, but I think that should not be part of this series.
> > diff --git a/gdb/python/lib/gdb/ptwrite.py
> b/gdb/python/lib/gdb/ptwrite.py
>
> I gather the new stuff is documented in the next patch.
Correct.
> > +# _ptwrite_filter contains the per thread copies of the filter function.
> > +# The keys are tuples of inferior id and thread id.
> > +# The filter functions are created for each thread by calling the
> > +# _ptwrite_filter_factory.
> > +_ptwrite_filter = {}
>
> It seems like when an inferior or thread is destroyed, the entries
> should be removed from this map.
That is done by _ptwrite_exit_handler being registered as a thread_exit handler.
> > + gdbpy_ref<> py_payload (PyLong_FromUnsignedLongLong (payload));
> > + gdbpy_ref<> py_ip (PyLong_FromUnsignedLongLong (ip));
>
> Both of these calls require null checks.
Thanks, I will add those.
> > + gdbpy_ref<> py_result (PyObject_CallFunctionObjArgs ((PyObject *)
> ptw_filter,
> > + py_payload.get (),
> > + py_ip.get (),
> > + nullptr));
> > +
> > + if (PyErr_Occurred ())
>
> It's more idiomatic to check == nullptr than PyErr_Occurred.
Will do.
> > + result = gdbpy_obj_to_string (py_result.get ()).get ();
> > +
> > + if (PyErr_Occurred ())
>
> Here too.
Thanks! This actually made me realize a bigger problem.
gdbpy_obj_to_string can return nullptr, and I try to assign it to a std::string.
Now I need to think if I should just make everything a
gdb::unique_xmalloc_ptr<char> again instead of using gdb::optional<std::string>
or whether I just copy or std::move the string I get.
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://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
@@ -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. */
@@ -1317,6 +1318,9 @@ ftrace_add_pt (struct btrace_thread_info *btinfo,
uint64_t offset;
int status;
+ /* Register the ptwrite filter. */
+ apply_ext_lang_ptwrite_filter (btinfo);
+
for (;;)
{
struct pt_insn insn;
@@ -35,6 +35,7 @@
#endif
#include <vector>
+#include <string>
struct thread_info;
struct btrace_function;
@@ -352,6 +353,16 @@ struct btrace_thread_info
displaying or stepping through the execution history. */
std::vector<std::string> aux_data;
+ /* Function pointer to the ptwrite callback. Returns the string returned
+ by the ptwrite filter function. */
+ gdb::optional<std::string> (*ptw_callback_fun) (const uint64_t payload,
+ const uint64_t ip,
+ const void *ptw_context)
+ = nullptr;
+
+ /* Context for the ptw_callback_fun. */
+ void *ptw_context = nullptr;
+
/* The function level offset. When added to each function's LEVEL,
this normalizes the function levels such that the smallest level
becomes zero. */
@@ -75,6 +75,7 @@ PYTHON_FILE_LIST = \
gdb/frames.py \
gdb/printing.py \
gdb/prompt.py \
+ gdb/ptwrite.py \
gdb/styling.py \
gdb/types.py \
gdb/unwinder.py \
@@ -184,6 +184,11 @@ 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 filter to the current thread. */
+ void (*apply_ptwrite_filter)
+ (const struct extension_language_defn *extlang,
+ struct btrace_thread_info *btinfo);
+
/* 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.
@@ -551,6 +551,19 @@ apply_ext_lang_frame_filter (frame_info_ptr frame,
return EXT_LANG_BT_NO_FILTERS;
}
+/* Used for registering the ptwrite filter to the current thread. */
+
+void
+apply_ext_lang_ptwrite_filter (btrace_thread_info *btinfo)
+{
+ for (const struct extension_language_defn *extlang : extension_languages)
+ {
+ if (extlang->ops != nullptr
+ && extlang->ops->apply_ptwrite_filter != nullptr)
+ extlang->ops->apply_ptwrite_filter (extlang, btinfo);
+ }
+}
+
/* 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.
@@ -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);
+extern void apply_ext_lang_ptwrite_filter
+ (struct btrace_thread_info *btinfo);
+
extern void preserve_ext_lang_values (struct objfile *, htab_t copied_types);
extern const struct extension_language_defn *get_breakpoint_cond_ext_lang
@@ -125,6 +125,7 @@ static const struct extension_language_ops guile_extension_ops =
gdbscm_apply_val_pretty_printer,
NULL, /* gdbscm_apply_frame_filter, */
+ NULL, /* gdbscm_load_ptwrite_filter, */
gdbscm_preserve_values,
new file mode 100644
@@ -0,0 +1,77 @@
+# Ptwrite utilities.
+# Copyright (C) 2023 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/>.
+
+"""Utilities for working with ptwrite filters."""
+
+import gdb
+
+# _ptwrite_filter contains the per thread copies of the filter function.
+# The keys are tuples of inferior id and thread id.
+# The filter functions are created for each thread by calling the
+# _ptwrite_filter_factory.
+_ptwrite_filter = {}
+_ptwrite_filter_factory = None
+
+
+def _ptwrite_exit_handler(event):
+ """Exit handler to prune _ptwrite_filter on thread exit."""
+ _ptwrite_filter.pop(event.inferior_thread.ptid, None)
+
+
+gdb.events.thread_exited.connect(_ptwrite_exit_handler)
+
+
+def _clear_traces():
+ """Helper function to clear the trace of all threads."""
+ current_thread = gdb.selected_thread()
+
+ for inferior in gdb.inferiors():
+ for thread in inferior.threads():
+ thread.switch()
+ recording = gdb.current_recording()
+ if recording is not None:
+ recording.clear()
+
+ current_thread.switch()
+
+
+def register_filter_factory(filter_factory_):
+ """Register the ptwrite filter factory."""
+ if filter_factory_ is not None and not callable(filter_factory_):
+ raise TypeError("The filter factory must be callable or 'None'.")
+
+ # Clear the traces of all threads of all inferiors to force
+ # re-decoding with the new filter.
+ _clear_traces()
+
+ _ptwrite_filter.clear()
+ global _ptwrite_filter_factory
+ _ptwrite_filter_factory = filter_factory_
+
+
+def get_filter():
+ """Returns the filter of the current thread."""
+ thread = gdb.selected_thread()
+ key = thread.ptid
+
+ # Create a new filter for new threads.
+ if key not in _ptwrite_filter:
+ if _ptwrite_filter_factory is not None:
+ _ptwrite_filter[key] = _ptwrite_filter_factory(thread)
+ else:
+ return None
+
+ return _ptwrite_filter[key]
@@ -806,6 +806,97 @@ recpy_bt_function_call_history (PyObject *self, void *closure)
return btpy_list_new (tinfo, first, last, 1, &recpy_func_type);
}
+/* Helper function that calls PTW_FILTER with PAYLOAD and IP as arguments.
+ Returns the string that will be printed, if there is a filter to call. */
+static gdb::optional<std::string>
+recpy_call_filter (const uint64_t payload, const uint64_t ip,
+ const void *ptw_filter)
+{
+ gdb::optional<std::string> result;
+
+ gdb_assert (ptw_filter != nullptr);
+ if ((PyObject *) ptw_filter == Py_None)
+ return result;
+
+ gdbpy_enter enter_py;
+
+ gdbpy_ref<> py_payload (PyLong_FromUnsignedLongLong (payload));
+ gdbpy_ref<> py_ip (PyLong_FromUnsignedLongLong (ip));
+
+ if (ip == 0)
+ py_ip = gdbpy_ref<>::new_reference (Py_None);
+
+ gdbpy_ref<> py_result (PyObject_CallFunctionObjArgs ((PyObject *) ptw_filter,
+ py_payload.get (),
+ py_ip.get (),
+ nullptr));
+
+ if (PyErr_Occurred ())
+ {
+ gdbpy_print_stack ();
+ gdbpy_error (_("Couldn't call the ptwrite filter."));
+ }
+
+ /* Py_None is valid and results in no output. */
+ if (py_result == Py_None)
+ {
+ result = "";
+ return result;
+ }
+
+ result = gdbpy_obj_to_string (py_result.get ()).get ();
+
+ if (PyErr_Occurred ())
+ {
+ gdbpy_print_stack ();
+ gdbpy_error (_("The ptwrite filter didn't return a string."));
+ }
+
+ return result;
+}
+
+/* Helper function returning the current ptwrite filter. */
+
+static PyObject *
+get_ptwrite_filter ()
+{
+ gdbpy_ref<> module (PyImport_ImportModule ("gdb.ptwrite"));
+
+ if (PyErr_Occurred ())
+ {
+ gdbpy_print_stack ();
+ return nullptr;
+ }
+
+ /* We need to keep the reference count. */
+ gdbpy_ref<> ptw_filter (PyObject_CallMethod (module.get (), "get_filter",
+ nullptr));
+
+ if (PyErr_Occurred ())
+ {
+ gdbpy_print_stack ();
+ gdbpy_error (_("Couldn't get the ptwrite filter."));
+ }
+
+ return ptw_filter.get();
+}
+
+/* Used for registering the default ptwrite filter to the current thread. A
+ pointer to this function is stored in the python extension interface. */
+
+void
+gdbpy_load_ptwrite_filter (const struct extension_language_defn *extlang,
+ struct btrace_thread_info *btinfo)
+{
+ gdb_assert (btinfo != nullptr);
+
+ gdbpy_enter enter_py;
+
+ btinfo->ptw_context = get_ptwrite_filter ();
+ if (btinfo->ptw_context != nullptr)
+ btinfo->ptw_callback_fun = &recpy_call_filter;
+}
+
/* Implementation of BtraceRecord.goto (self, BtraceInstruction) -> None. */
PyObject *
@@ -378,6 +378,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 void gdbpy_load_ptwrite_filter
+ (const struct extension_language_defn *extlang,
+ struct btrace_thread_info *btinfo);
extern enum ext_lang_bt_status gdbpy_apply_frame_filter
(const struct extension_language_defn *,
frame_info_ptr frame, frame_filter_flags flags,
@@ -153,6 +153,8 @@ static const struct extension_language_ops python_extension_ops =
gdbpy_apply_frame_filter,
+ gdbpy_load_ptwrite_filter,
+
gdbpy_preserve_values,
gdbpy_breakpoint_has_cond,