Re: [PATCH 08/10] btrace, python: Enable ptwrite listener registration.

["Willgerodt, Felix" <felix.willgerodt@intel.com>]

On 6/4/19 2:36 PM, Metzger, Markus T wrote:
> Hello Felix,
>
>> 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.
>>
>> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
>>
>> gdb/ChangeLog:
>> * 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                | 24 ++++++++++
>>   gdb/extension.h                |  3 ++
>>   gdb/guile/guile.c              |  1 +
>>   gdb/python/lib/gdb/ptwrite.py  | 83
>> ++++++++++++++++++++++++++++++++++
>>   gdb/python/py-record-btrace.c  | 40 ++++++++++++++++
>>   gdb/python/py-record-btrace.h  |  3 ++
>>   gdb/python/py-record.c         | 29 ++++++++++++
>>   gdb/python/python-internal.h   |  3 ++
>>   gdb/python/python.c            |  2 +
>>   13 files changed, 199 insertions(+)
>>   create mode 100644 gdb/python/lib/gdb/ptwrite.py

Markus> When registering a new PTWRITE listener, could GDB check the libipt version and give
Markus> an error if it does not support PTW?

In the current design GDB doesn't know if a there is a new listener (different to the default),
until it needs to use it. We would need to expose the libipt version somehow in python.
Not sure how that should look like, in CLI we only have it exposed in "maintenance info btrace".


>> @@ -1308,6 +1309,8 @@ ftrace_add_pt (struct btrace_thread_info *btinfo,
>>     uint64_t offset;
>>     int status;
>>
>> +  apply_ext_lang_ptwrite_listener (inferior_ptid);

Markus > We may decode the trace in chunks by calling ftrace_add_pt () multiple times.
Markus > This would need to preserve (the state of) the installed listener.

The state is already preserved. This just fetches the (C++) pointer to
be able to call the listener from GDB. If the listener wasn't changed,
the same pointer/object will be fetched as in the first call to ftrace_add_pt
().


>> @@ -0,0 +1,83 @@
>> +# Ptwrite utilities.
>> +# Copyright (C) 2018 Free Software Foundation, Inc.

Markus> It's 2019, meanwhile.

Fixed.

>> +
>> +# 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 listeners."""
>> +
>> +import gdb
>> +from copy import deepcopy
>> +
>> +
>> +def default_listener(payload, ip):

Markus> I somehow expected some PtwritePrinter class.

I don't see any advantage of classifying the default one. And as we allow every
callable, I don't see anything else we need to provide here. Please elaborate if you do.


>> +    """Default listener that is active upon starting GDB."""
>> +    return "payload: {:#x}".format(payload)

Markus> Should we return the payload (in hex) without the prefix?

I don't really have a preference. Done.


>> @@ -735,3 +735,32 @@ gdbpy_stop_recording (PyObject *self, PyObject *args)
>>
>>     Py_RETURN_NONE;
>>   }
>> +
>> +/* 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)

Markus> PTWRITE is very PT specific.  Any chance this could go into py-record-btrace.c?

Done.

>> +{
>> +  struct gdbarch *gdbarch = NULL;
>> +
>> +  if (!gdb_python_initialized)
>> +    return EXT_LANG_BT_NO_FILTERS;
>> +
>> +  try
>> +    {
>> +      gdbarch = target_gdbarch ();
>> +    }
>> +  catch (const gdb_exception &except)
>> +    {
>> +      /* Let gdb try to print the stack trace.  */
>> +      return EXT_LANG_BT_NO_FILTERS;

Markus> I fail to understand the comment.

I copied it from gdbpy_apply_frame_filter() when creating the function. Removed.

Thanks,
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