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

[Phil Muldoon <pmuldoon at redhat dot com>]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Sorry for the delay in replying.  I just noticed this email has been
pending for awhile. 

> Phil> +  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
> Phil> +					       current_language);
>
> I didn't notice this before.  I think this should use the breakpoint's
> arch and language.


Two odd things I noticed here.  ensure_python_env requires:

const struct language_defn *language

But struct breakpoint only stores:

language_defn->la_language

So we only have the enum la_language available when we have the
breakpoint.  Not sure if there is a way to backtrack back to
language_defn, and if not, we are stuck with current_language.

Also the b->gdbarch member is NULL with watchpoints, so we have to do:

  PyObject *py_bp = (PyObject *) bp_obj;
  struct breakpoint *b = bp_obj->bp;
  struct gdbarch *garch = b->gdbarch ? b->gdbarch : get_current_arch ();
  struct cleanup *cleanup = ensure_python_env (garch, current_language);

I'm not sure why watchpoints have a NULL gdbarch, but if you grep
watch_command_1, the watchpoint is eventually created with:

  /* Now set up the breakpoint.  */
  b = set_raw_breakpoint_without_location (NULL, bp_type);

Where that NULL is the architecture.  It never seems to be updated.
Anyway we need the architecture later when we call
convert_value_from_python. Without it we get:

Program received signal SIGSEGV, Segmentation fault.
0x00000000005a99ab in gdbarch_data (gdbarch=0x0, data=0xbb1820) at ../../archer/gdb/gdbarch.c:3924
3924	  gdb_assert (data->index < gdbarch->nr_data);

(gdb) bt
#0  0x00000000005a99ab in gdbarch_data (gdbarch=0x0, data=0xbb1820) at ../../archer/gdb/gdbarch.c:3924
#1  0x00000000005b3a74 in builtin_type (gdbarch=0x0) at ../../archer/gdb/gdbtypes.c:3650
#2  0x000000000050ef84 in convert_value_from_python (obj=0xc93ff0) at ../../archer/gdb/python/py-value.c:1116
#3  0x000000000050e86a in valpy_richcompare (self=0x7ffff202ee30, other=0xc93ff0, op=2) at ../../archer/gdb/python/py-value.c:907

> Phil> +  if (PyObject_HasAttrString (py_bp, method))
> Phil> +    {
> Phil> +      PyObject *result = PyObject_CallMethod (py_bp, method, NULL);
>
> Since we're now overloading the "condition" member, what happens if the
> user sets a condition on the breakpoint?
>
> I think we may need an additional check here, to see if the "condition"
> member is a callable object.

I cannot find a way to test if an attribute is callable or
not. There is:  PyCallable_Check but that only refers to the object
itself.  I think there is only a check to see if the attribute exists
(which we do), but as you correctly point out, as we are overloading
"condition" that will always return True.  For now, as we are waiting
for other bits of the breakpoint puzzle to fall in place, I've returned
the name to "eval".  It is trivial to change it in the code and tests at
a later date.

Cheers

Phil