RE: [PATCH v9 06/10] python: Add clear() to gdb.Record.

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

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Donnerstag, 6. Juli 2023 18:12
> 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 06/10] python: Add clear() to gdb.Record.
> 
> >>>>> Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> > This function allows to clear the trace data from python, forcing to
> > re-decode the trace for successive commands.
> > This will be used in future ptwrite patches, to trigger re-decoding when
> > the ptwrite filter changes.
> 
> > +PyObject *
> > +recpy_bt_clear (PyObject *self, PyObject *args)
> > +{
> > +  const recpy_record_object * const record = (recpy_record_object *) self;
> > +  thread_info *const tinfo = record->thread;
> 
> Normally in the Python layer, some care must be taken to ensure that
> something sensible happens when a Python object outlives some underlying
> gdb object.  That is why some types have an 'is_valid' method and why
> there are the various *_REQUIRE_VALID macros.
> 
> It isn't a problem with this patch per se but it seems to me that this
> code does not handle this situation properly.
> 
> Tom

Thanks for the review, I am not sure I understand your point completely.

In this patch, standalone, I don't see any such problem, so you are probably
referring to the whole series.
Note that there is already "maintenance btrace clear" in CLI. So if anything
bad could happen, it would already be exposed by that command as well.
But yes, that is a maintenance command so maybe not 100% comparable.

Why we need this patch in this series, is that once a function call history
or instruction call history is decoded, it will not be re-decoded until you
continue to the next BP and re-issue the command.
And that makes some sense, as the decoding can be costly.

Now, if a user registers a new ptwrite filter, and we don't re-decode the
trace, he will be confused. As he still sees the old trace and not the
output of his newly registered filter function. We could state that that
is a limitation, or check for this and warn the user. But I decided to instead
clear the trace data to force re-decoding the function and instruction
history. 
I tested this a bit, and you can too with upstream using the maintenance
command. I didn't see any problems, any subsequent commands would
use the new trace data. And any gdb python objects would also fetch that
new trace data or print an error if the trace data is empty.

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