This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Add an evaluation function hook to Python breakpoints.


>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> Collecting data in the "evaluate" function feels too hacky for
Doug> something we explicitly tell users is the published way to do this
Doug> kind of thing.

Can you explain what is hacky about it?  I'm not trying to be difficult,
I really do not understand.

I will explain why I think it is ok.

This patch addresses two bits of functionality we have needed in Python.

First, we need a clean way to run some code at a given breakpoint
location -- but with the caveats that the code always be run, regardless
of other breakpoints, and that the code not interfere with stepping.

By "clean" I just mean that we want it not to have user-visible effects
other than the effects we intend.  Yes, we can do it using a python
convenience function -- but the convenience function's name is visible
and in a flat namespace.

The use case for this is something like gdb-heap, where we want to
install a breakpoint that collects some information but doesn't
interfere with ordinary debugging.


Second, we want a way to stop the inferior when such the data collection
step decides.  Our use case here is a (to-be-written) mutex-tracking
extension, that collects information about pthread locking and
unlocking, and stops when a deadlock is detected.


I think Phil's patch accomplishes all of these goals.

Doug> And the name "evaluate" doesn't feel right either.

I agree.

Doug> I realize the _p convention mightn't be sufficiently common to use in
Doug> the Python API, but for example a better name might be stop_p.
Doug> And then one would have another method (I don't have a good name for
Doug> it ATM) to use to collect data.
Doug> Setting aside name choices, I like that API better.

Having two methods seems worse to me.  It is more complicated without
any added benefits.

First, both such methods will be called in the same place in gdb.  This
is necessary to implement the needed features.  But then .. why call two
methods when you can just call one?

Second, consider the mutex-watching use case.  To implement this in the
two-method case, you must have the data collector set a flag, which is
then returned by the condition method.  Or ... just do all the work in
the condition method -- which is where we are now.

Doug> OTOH, it seems like Python-based breakpoints have two conditions
Doug> now (or at least two kinds of conditions one has to think about).
Doug> One set with the "condition" command and one from the "evaluate"
Doug> API function.  IWBN to have only one, at least conceptually.
Doug> Maybe you could rename "evaluate" to "condition" (or some such, I
Doug> realize there's already a "condition"), and have the default be to
Doug> use the CLI condition.

This would be fine with me as long as it meets all the goals above.

Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]