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

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

[Phil Muldoon]

This patch allows Python breakpoints to be sub-classed and implements
the "evaluate" function.  If the user defines an "evaluate" function in
the Python breakpoint it will be called when GDB evaluates any
conditions assigned to the breakpoint. This allows the user to write
conditional breakpoints entirely in Python, and also allows the user to
collect data at each breakpoint.  If the function returns True, GDB will
stop the inferior at the breakpoint; if the function returns False
the inferior will continue.

Tested on x86-64, Fedora 14 with no regressions.

Comments?

Cheers,

Phil

2010-12-13  Phil Muldoon  <pmuldoon@redhat.com>

	* breakpoint.c (bpstat_check_breakpoint_conditions): Add Python
	evaluation code.

2010-12-13  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/py-breakpoint.exp: Add Python evaluation tests.

2010-12-13  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.texinfo (Breakpoints In Python): Add description and example
	of Python evaluation function.

--

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6a51a3b..b9c3d01 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -72,6 +72,11 @@
 
 #include "mi/mi-common.h"
 
+#if HAVE_PYTHON
+#include "python/python.h"
+#include "python/python-internal.h"
+#endif
+
 /* Arguments to pass as context to some catch command handlers.  */
 #define CATCH_PERMANENT ((void *) (uintptr_t) 0)
 #define CATCH_TEMPORARY ((void *) (uintptr_t) 1)
@@ -3964,6 +3969,41 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
       int value_is_zero = 0;
       struct expression *cond;
 
+      /* Evaluate Python breakpoints that have an "evaluate"
+	 function implemented.  */
+#if HAVE_PYTHON
+      if (b->py_bp_object)
+	{
+	  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
+						       current_language);
+	  PyObject *gdbpy_bp_eval = PyString_FromString ("evaluate");
+	  PyObject *py_bp = (PyObject *) b->py_bp_object;
+
+	  if (PyObject_HasAttr (py_bp, gdbpy_bp_eval))
+	    {
+	      PyObject *result = PyObject_CallMethodObjArgs (py_bp,
+							     gdbpy_bp_eval,
+							     NULL);
+
+	      if (result)
+		{
+		  int evaluate = PyObject_IsTrue (result);
+
+		  if (evaluate == -1)
+		    gdbpy_print_stack ();
+
+		  /* If the evaluate function returns False that means the
+		     Python breakpoint wants GDB to continue.  */
+		  if (!evaluate)
+		    bs->stop = 0;
+		}
+	      else
+		gdbpy_print_stack ();
+	    }
+	  do_cleanups (cleanup);
+	}
+#endif
+
       if (is_watchpoint (b))
 	cond = b->cond_exp;
       else
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dc9630a..09a02b0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22872,7 +22872,33 @@ Return the symbol table's source absolute file name.
 @tindex gdb.Breakpoint
 
 Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
-class.
+class.  The @code{gdb.Breakpoint} class can be sub-classed and, in
+particular, you may choose to implement the @code{evaluate} function.
+If this function is defined it will be called when the inferior reaches
+that breakpoint.  If the @code{evaluate} function returns
+@code{True}, the inferior will be stopped at the location of the
+breakpoint.  Otherwise if the function returns @code{False} the
+inferior will continue.  The @code{evaluate} function should not
+attempt to influence the state of the inferior (e.g., step) or
+otherwise alter its data.  
+
+If there are multiple breakpoints at the same location with an
+@code{evaluate} function, each one will be called regardless of the
+return status of the previous.  This ensures that all @code{evaluate}
+functions have a chance to execute at that location.  In this scenario
+if one of the functions returns @code{True} but the others return
+@code{False}, the inferior will still be stopped.
+
+Example @code{evaluate} implementation:
+
+@smallexample
+class MyBreakpoint (gdb.Breakpoint):
+      def evaluate (self):
+        inf_val = gdb.parse_and_eval("foo")
+        if inf_val == 3:
+          return True
+        return False
+@end smallexample
 
 @defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]} @r{[}internal@r{]}
 Create a new breakpoint.  @var{spec} is a string naming the
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 88d9930..6752d83 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -887,7 +887,7 @@ static PyTypeObject breakpoint_object_type =
   0,				  /*tp_getattro*/
   0,				  /*tp_setattro*/
   0,				  /*tp_as_buffer*/
-  Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
+  Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,  /*tp_flags*/
   "GDB breakpoint object",	  /* tp_doc */
   0,				  /* tp_traverse */
   0,				  /* tp_clear */
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 34a64a3..0867acd 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -198,3 +198,67 @@ gdb_py_test_silent_cmd  "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WA
 gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
 gdb_test "maint info breakpoints" ".*hw watchpoint.*result.*" "Check maint info breakpoints shows invisible breakpoints"
 gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value = 0.*New value = 25.*" "Test watchpoint write"
+
+# Breakpoints that have an evaluation function.
+
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+    fail "Cannot run to main."
+    return 0
+}
+delete_breakpoints
+
+gdb_py_test_multiple "Sub-class a breakpoint" \
+  "python" "" \
+  "class bp_eval (gdb.Breakpoint):" "" \
+  "   inf_i = 0" "" \
+  "   count = 0" "" \
+  "   def evaluate (self):" "" \
+  "      self.count = self.count + 1" "" \
+  "      self.inf_i = gdb.parse_and_eval(\"i\")" "" \
+  "      if self.inf_i == 3:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+gdb_py_test_multiple "Sub-class a second breakpoint" \
+  "python" "" \
+  "class bp_also_eval (gdb.Breakpoint):" "" \
+  "   count = 0" "" \
+  "   def evaluate (self):" "" \
+  "      self.count = self.count + 1" "" \
+  "      if self.count == 9:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+set bp_location2 [gdb_get_line_number "Break at multiply."]
+set end_location [gdb_get_line_number "Break at end."]
+gdb_py_test_silent_cmd  "python eval_bp1 = bp_eval(\"$bp_location2\")" "Set breakpoint" 0
+gdb_py_test_silent_cmd  "python also_eval_bp1 = bp_also_eval(\"$bp_location2\")" "Set breakpoint" 0
+gdb_py_test_silent_cmd  "python never_eval_bp1 = bp_also_eval(\"$end_location\")" "Set breakpoint" 0
+gdb_continue_to_breakpoint "Break at multiply." ".*/$srcfile:$bp_location2.*"
+gdb_test "print i" "3" "Check inferior value matches python accounting"
+gdb_test "python print eval_bp1.inf_i" "3" "Check python accounting matches inferior"
+gdb_test "python print also_eval_bp1.count" "4" \
+    "Check non firing same-location breakpoint eval function was also called at each stop."
+gdb_test "python print eval_bp1.count" "4" \
+    "Check non firing same-location breakpoint eval function was also called at each stop."
+
+gdb_py_test_multiple "Sub-class a watchpoint" \
+  "python" "" \
+  "class wp_eval (gdb.Breakpoint):" "" \
+  "   def evaluate (self):" "" \
+  "      self.result = gdb.parse_and_eval(\"result\")" "" \
+  "      if self.result == 788:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+delete_breakpoints
+gdb_py_test_silent_cmd  "python wp1 = wp_eval (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE)" "Set watchpoint" 0
+gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value =.*New value = 788.*" "Test watchpoint write"
+gdb_test "python print never_eval_bp1.count" "0" \
+    "Check that this unrelated breakpoints eval function was never called."

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

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Date: Mon, 13 Dec 2010 13:50:36 +0000
> 
> 
> This patch allows Python breakpoints to be sub-classed and implements
> the "evaluate" function.  If the user defines an "evaluate" function in
> the Python breakpoint it will be called when GDB evaluates any
> conditions assigned to the breakpoint. This allows the user to write
> conditional breakpoints entirely in Python, and also allows the user to
> collect data at each breakpoint.  If the function returns True, GDB will
> stop the inferior at the breakpoint; if the function returns False
> the inferior will continue.

Thanks.

> +class.  The @code{gdb.Breakpoint} class can be sub-classed and, in
> +particular, you may choose to implement the @code{evaluate} function.
> +If this function is defined it will be called when the inferior reaches
> +that breakpoint.

Which "that breakpoint" are you talking about here?  Perhaps you meant
this:

  If this function is defined for a breakpoint, it will be called when
  the inferior reaches that breakpoint.

?  If so, the text is clear, but then this interpretation doesn't seem
to be consistent with the example you give.

> +@code{True}, the inferior will be stopped at the location of the
> +breakpoint.  Otherwise if the function returns @code{False} the
> +inferior will continue.

Either "otherwise", or "if it returns False"; using both is redundant.

> +                         The @code{evaluate} function should not
> +attempt to influence the state of the inferior (e.g., step) or
> +otherwise alter its data.  

Sounds like a non-trivial limitation; is it really necessary?

> +If there are multiple breakpoints at the same location with an
> +@code{evaluate} function, each one will be called regardless of the
> +return status of the previous.

Do we have anything to say regarding the order of the calls?

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

[Pedro Alves]

On Monday 13 December 2010 13:50:36, Phil Muldoon wrote:

> +      /* Evaluate Python breakpoints that have an "evaluate"
> +        function implemented.  */
> +#if HAVE_PYTHON
> +      if (b->py_bp_object)
> +       {
> +         struct cleanup *cleanup = ensure_python_env (get_current_arch (),
> +                                                      current_language);
> +         PyObject *gdbpy_bp_eval = PyString_FromString ("evaluate");
> +         PyObject *py_bp = (PyObject *) b->py_bp_object;
> +
> +         if (PyObject_HasAttr (py_bp, gdbpy_bp_eval))
> +           {
> +             PyObject *result = PyObject_CallMethodObjArgs (py_bp,
> +                                                            gdbpy_bp_eval,
> +                                                            NULL);
> +
> +             if (result)
> +               {
> +                 int evaluate = PyObject_IsTrue (result);
> +
> +                 if (evaluate == -1)
> +                   gdbpy_print_stack ();
> +
> +                 /* If the evaluate function returns False that means the
> +                    Python breakpoint wants GDB to continue.  */
> +                 if (!evaluate)
> +                   bs->stop = 0;
> +               }
> +             else
> +               gdbpy_print_stack ();
> +           }
> +         do_cleanups (cleanup);
> +       }
> +#endif
> +

Can you factor out the PyObject manipulations and the actual evaluation
of the condition to pythong/py-breakpoint.c?  Say, to a
new "py_breakpoint_evaluate (struct breakpoint_object *, ...)" function.
The driving idea being to get rid of the need to now include
python-internal.h.

My first reaction was 'why not call the field "condition"?  "evaluate"
sounds like it's about watchpoint evaluation or some such to me.  Point
being, that there are several different things that are evaluated, and
so it kind of sounds ambiguous.  OTOH, there's chance of confusion with
the condition expression set with the "condition" command.  Is that one
exposed to python?  It may be worth it to think a bit about that, so to
make sure the docs and api doesn't end up confusing when you end up
exposing that condition too.  I'm okay with whatever you guys come up
with, just pointing it out.

-- 
Pedro Alves

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

[Phil Muldoon]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Phil Muldoon <pmuldoon@redhat.com>
>> Date: Mon, 13 Dec 2010 13:50:36 +0000

>> +class.  The @code{gdb.Breakpoint} class can be sub-classed and, in
>> +particular, you may choose to implement the @code{evaluate} function.
>> +If this function is defined it will be called when the inferior reaches
>> +that breakpoint.
>
> Which "that breakpoint" are you talking about here?  Perhaps you meant
> this:
>
>   If this function is defined for a breakpoint, it will be called when
>   the inferior reaches that breakpoint.
>
> ?  If so, the text is clear, but then this interpretation doesn't seem
> to be consistent with the example you give.


I'm not sure what you mean here.  Given the example in the patch, you
can instantiate any number of those breakpoints.

So

+class MyBreakpoint (gdb.Breakpoint):
+      def evaluate (self):
+        inf_val = gdb.parse_and_eval("foo")
+        if inf_val == 3:
+          return True
+        return False

There are two scenarios where the breakpoints would be executed.  The first
is where you would end up instantiating two instances of the above
breakpoint at the same location:

python bp1 = MyBreakPoint("myfile.c:42")
python bp2 = MyBreakpoint("myfile.c:42", internal=True)

And the other where you would have two dissimilar breakpoints at the same
location:

e.g,

+class MyBreakpoint2 (gdb.Breakpoint):
+      def evaluate (self):
+        inf_val = gdb.parse_and_eval("bar")
+        if inf_val == 5:
+          return True
+        return False

python bp1 = MyBreakPoint("myfile.c:42")
python bp2 = MyBreakpoint2("myfile.c:42")

Does that help explain? I'm opening to wording it however makes the most
sense.


>> +@code{True}, the inferior will be stopped at the location of the
>> +breakpoint.  Otherwise if the function returns @code{False} the
>> +inferior will continue.
>
> Either "otherwise", or "if it returns False"; using both is redundant.

Ok, thanks.

>> +                         The @code{evaluate} function should not
>> +attempt to influence the state of the inferior (e.g., step) or
>> +otherwise alter its data.  
>
> Sounds like a non-trivial limitation; is it really necessary?

I mentioned it as it was a non-trivial limitation? It is something the user
should not do in the evaluate function.  The most pressing reason is
that there may be other breakpoints to be checked at that location so
the state of the inferior should remain.  The other is it is just bad
business influencing the inferior position (i.e., stepping) when
breakpoint evaluation is being performed.  At least to me.  This is all
well and good, but we cannot prevent the user from doing it; just tell
them it is very bad idea. ;)

>> +If there are multiple breakpoints at the same location with an
>> +@code{evaluate} function, each one will be called regardless of the
>> +return status of the previous.
>
> Do we have anything to say regarding the order of the calls?

I'm not sure, as there is no mention I could find in the manual
regarding the conditional evaluation order of breakpoints.  I believe
they are called in the order of creation.  Maybe Pedro knows.  But as
breakpoint ordering is not mentioned anywhere else, and as all the
evaluation functions at that location will be called regardless, I
decided not to mention it.

Cheers,

Phil

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

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:

> Can you factor out the PyObject manipulations and the actual evaluation
> of the condition to pythong/py-breakpoint.c?  Say, to a
> new "py_breakpoint_evaluate (struct breakpoint_object *, ...)" function.
> The driving idea being to get rid of the need to now include
> python-internal.h.

Sure I've no problem with that. I think we would still have to include
python.h (but not python-internal.h) inside a HAVE_PYTHON conditional, as
the function would be exposed from py_breakpoint.c via python.h.  Is
that ok? Or maybe I misread your intentions.

> My first reaction was 'why not call the field "condition"?  "evaluate"
> sounds like it's about watchpoint evaluation or some such to me.  Point
> being, that there are several different things that are evaluated, and
> so it kind of sounds ambiguous.  OTOH, there's chance of confusion with
> the condition expression set with the "condition" command.  Is that one
> exposed to python?  It may be worth it to think a bit about that, so to
> make sure the docs and api doesn't end up confusing when you end up
> exposing that condition too.  I'm okay with whatever you guys come up
> with, just pointing it out.

We do expose the "condition" API currently, but it is a read-only
attribute that exposes any conditions set via the CLI.  I've no opinion
what we should call the Python conditional evaluation.  I picked
"evaluate" for no particular reasons (and we are evaluating code).  I'm
open to ideas; changing the name is pretty trivial.

Cheers,

Phil

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

[Pedro Alves]

On Monday 13 December 2010 14:56:10, Phil Muldoon wrote:
> Pedro Alves <pedro@codesourcery.com> writes:
> 
> > Can you factor out the PyObject manipulations and the actual evaluation
> > of the condition to pythong/py-breakpoint.c?  Say, to a
> > new "py_breakpoint_evaluate (struct breakpoint_object *, ...)" function.
> > The driving idea being to get rid of the need to now include
> > python-internal.h.
> 
> Sure I've no problem with that. I think we would still have to include
> python.h (but not python-internal.h) inside a HAVE_PYTHON conditional, as
> the function would be exposed from py_breakpoint.c via python.h.

Right.  That's okay.

> Is that ok? Or maybe I misread your intentions.

You got it.

-- 
Pedro Alves

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

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 13 Dec 2010 14:47:33 +0000
> 
> +class MyBreakpoint (gdb.Breakpoint):
> +      def evaluate (self):
> +        inf_val = gdb.parse_and_eval("foo")
> +        if inf_val == 3:
> +          return True
> +        return False
> 
> There are two scenarios where the breakpoints would be executed.  The first
> is where you would end up instantiating two instances of the above
> breakpoint at the same location:
> 
> python bp1 = MyBreakPoint("myfile.c:42")
> python bp2 = MyBreakpoint("myfile.c:42", internal=True)

So I would rephrase the text in question like this:

  If this function is defined in a sub-class of @code{gdb.Breakpoint},
  it will be called when the inferior reaches any breakpoint which
  instantiates that sub-class.

Does this describe correctly what will happen?

> >> +                         The @code{evaluate} function should not
> >> +attempt to influence the state of the inferior (e.g., step) or
> >> +otherwise alter its data.  
> >
> > Sounds like a non-trivial limitation; is it really necessary?
> 
> I mentioned it as it was a non-trivial limitation? It is something the user
> should not do in the evaluate function.  The most pressing reason is
> that there may be other breakpoints to be checked at that location so
> the state of the inferior should remain.  The other is it is just bad
> business influencing the inferior position (i.e., stepping) when
> breakpoint evaluation is being performed.

We allow that from the CLI, FWIW.

> This is all well and good, but we cannot prevent the user from doing
> it; just tell them it is very bad idea. ;)

But if there's only one breakpoint at that location, there doesn't
seem to be any reasons to disallow this, right?

Anyway, this is not really related to the manual; if the modified text
above is okay with you, the patch for the manual is approved.

Thanks.

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

[Phil Muldoon]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Phil Muldoon <pmuldoon@redhat.com>
>> Cc: gdb-patches@sourceware.org
>> Date: Mon, 13 Dec 2010 14:47:33 +0000
>> 
>> +class MyBreakpoint (gdb.Breakpoint):
>> +      def evaluate (self):
>> +        inf_val = gdb.parse_and_eval("foo")
>> +        if inf_val == 3:
>> +          return True
>> +        return False
>> 
>> There are two scenarios where the breakpoints would be executed.  The first
>> is where you would end up instantiating two instances of the above
>> breakpoint at the same location:
>> 
>> python bp1 = MyBreakPoint("myfile.c:42")
>> python bp2 = MyBreakpoint("myfile.c:42", internal=True)
>
> So I would rephrase the text in question like this:
>
>   If this function is defined in a sub-class of @code{gdb.Breakpoint},
>   it will be called when the inferior reaches any breakpoint which
>   instantiates that sub-class.

But only if the inferior has stopped at the location of that
breakpoint.  So given this case:

python bp1 = MyBreakPoint("myfile.c:42")
python bp2 = MyBreakpoint("myfile.c:45")

Those two are using the same sub-classed breakpoint but are at different
locations.  So if the inferior reached line 42, then only the evaluate
function of bp1 would be called.  The above scenario would happen, for
example, if the user was checking the state of a global variable.

In the previous example:

python bp1 = MyBreakPoint("myfile.c:42")
python bp2 = MyBreakpoint("myfile.c:42", internal=True)

The evaluate function would be called for both bp1 an bp2 as they are
both at the same location, and they both have evaluate functions
defined.  

Make sense?


>> > Sounds like a non-trivial limitation; is it really necessary?
>> 
>> I mentioned it as it was a non-trivial limitation? It is something the user
>> should not do in the evaluate function.  The most pressing reason is
>> that there may be other breakpoints to be checked at that location so
>> the state of the inferior should remain.  The other is it is just bad
>> business influencing the inferior position (i.e., stepping) when
>> breakpoint evaluation is being performed.
>
> We allow that from the CLI, FWIW.


>> This is all well and good, but we cannot prevent the user from doing
>> it; just tell them it is very bad idea. ;)
>
> But if there's only one breakpoint at that location, there doesn't
> seem to be any reasons to disallow this, right?

Aha, I did not know about the CLI version, (or more specifically the
fact that we do not make a mention that it may be unwise to start
tinkering with the state of the inferior.)  In retrospect I think you
are right; adding this text would confuse matters.  I have removed it
from the patch.

Cheers

Phil

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

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 13 Dec 2010 17:20:57 +0000
> 
> > So I would rephrase the text in question like this:
> >
> >   If this function is defined in a sub-class of @code{gdb.Breakpoint},
> >   it will be called when the inferior reaches any breakpoint which
> >   instantiates that sub-class.
> 
> But only if the inferior has stopped at the location of that
> breakpoint.

That's why my suggested text says "when the inferior reaches any
breakpoint".  I thought it was clear that "reaching a breakpoint"
means to get to the breakpoint's location.  And when the inferior
reaches the breakpoint's location, it inevitably stops, right?

I don't mind making this more explicit, either:

  If this function is defined in a sub-class of @code{gdb.Breakpoint},
  it will be called when the inferior stops at any location of a
  breakpoint which instantiates that sub-class.

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

[Doug Evans]

On Mon, Dec 13, 2010 at 5:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> This patch allows Python breakpoints to be sub-classed and implements
> the "evaluate" function.  If the user defines an "evaluate" function in
> the Python breakpoint it will be called when GDB evaluates any
> conditions assigned to the breakpoint. This allows the user to write
> conditional breakpoints entirely in Python, and also allows the user to
> collect data at each breakpoint.  If the function returns True, GDB will
> stop the inferior at the breakpoint; if the function returns False
> the inferior will continue.

Hi.
My $0.02 cents.

Collecting data in the "evaluate" function feels too hacky for
something we explicitly tell users is the published way to do this
kind of thing.
And the name "evaluate" doesn't feel right either.  What does it mean
to evaluate a breakpoint? [One might think it means to evaluate the
location of the breakpoint, e.g., since gdb tries to re-evaluate the
location when the binary changes.]
I realize the _p convention mightn't be sufficiently common to use in
the Python API, but for example a better name might be stop_p.
And then one would have another method (I don't have a good name for
it ATM) to use to collect data.
Setting aside name choices, I like that API better.

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

Not that this has to be done now, or ever, but I wonder how useful it
would be to allow CLI breakpoints to have commands that are executed
prior to evaluating the condition.  "continue", etc. would not be
allowed here, so it may be too awkward to implement.

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

[Phil Muldoon]

Doug Evans <dje@google.com> writes:

> On Mon, Dec 13, 2010 at 5:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>> This patch allows Python breakpoints to be sub-classed and implements
>> the "evaluate" function.  If the user defines an "evaluate" function in
>> the Python breakpoint it will be called when GDB evaluates any
>> conditions assigned to the breakpoint. This allows the user to write
>> conditional breakpoints entirely in Python, and also allows the user to
>> collect data at each breakpoint.  If the function returns True, GDB will
>> stop the inferior at the breakpoint; if the function returns False
>> the inferior will continue.
>
> Hi.
> My $0.02 cents.
>
> Collecting data in the "evaluate" function feels too hacky for
> something we explicitly tell users is the published way to do this
> kind of thing.

It's nothing more than a hook for the user to accomplish some truth
finding mission at breakpoint evaluation time.  If the user wants to
collect data, then I think they can do that -- that is we should not
attempt to stop them.  I probably worded my email hastily; FWIW I think
this a place where we can write conditional breakpoints that can be
written entirely in Python.


> And the name "evaluate" doesn't feel right either.  What does it mean
> to evaluate a breakpoint? [One might think it means to evaluate the
> location of the breakpoint, e.g., since gdb tries to re-evaluate the
> location when the binary changes.]
> I realize the _p convention mightn't be sufficiently common to use in
> the Python API, but for example a better name might be stop_p.
> And then one would have another method (I don't have a good name for
> it ATM) to use to collect data.
> Setting aside name choices, I like that API better.

I'm happy with any name.  Lets just call it what the majority are
comfortable with.  I used "evaluate" purely as a descriptive term to what
we are doing.  Whatever else fits will work with me.

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


This is something that is always a ponder-point for Python API hackers.
There are lots of places where "things" have worked in CLI for many years,
and we (well, I) do not want to deprecate existing CLI functionality
while expanding other areas.  I'm not sure what the path is from here.
Overloading the condition API seems like a pain for the script-writer to
figure out if it is a Python scripted condition, or a CLI scripted
condition.  That is why I ended up choosing a different name.  

What do you think?

Cheers,

Phil

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

[Doug Evans]

On Mon, Dec 13, 2010 at 1:02 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Doug Evans <dje@google.com> writes:
>
>> On Mon, Dec 13, 2010 at 5:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>>> This patch allows Python breakpoints to be sub-classed and implements
>>> the "evaluate" function.  If the user defines an "evaluate" function in
>>> the Python breakpoint it will be called when GDB evaluates any
>>> conditions assigned to the breakpoint. This allows the user to write
>>> conditional breakpoints entirely in Python, and also allows the user to
>>> collect data at each breakpoint.  If the function returns True, GDB will
>>> stop the inferior at the breakpoint; if the function returns False
>>> the inferior will continue.
>>
>> Hi.
>> My $0.02 cents.
>>
>> Collecting data in the "evaluate" function feels too hacky for
>> something we explicitly tell users is the published way to do this
>> kind of thing.
>
> It's nothing more than a hook for the user to accomplish some truth
> finding mission at breakpoint evaluation time.  If the user wants to
> collect data, then I think they can do that -- that is we should not
> attempt to stop them.  I probably worded my email hastily; FWIW I think
> this a place where we can write conditional breakpoints that can be
> written entirely in Python.

I'm not suggesting we stop users from doing quick hacks. :-)
[Heck, can they already write a breakpoint condition in Python?  E.g.
with a python convenience function?]

But there is an existing need to collect data at a breakpoint in a
stable way (i.e. collecting data in the CLI commands section is
problematic).  I think such a facility is rather useful,  and we
should provide it.  But I'd rather make it explicit, instead of a
hacky use of the gdb breakpoint condition.

It's an orthogonal feature to allowing one to write the breakpoint
condition in Python via a subclass of gdb.Breakpoint, so it *could* be
a separate patch.
I just don't want the breakpoint condition being *the* way to collect data.

>> And the name "evaluate" doesn't feel right either.  What does it mean
>> to evaluate a breakpoint? [One might think it means to evaluate the
>> location of the breakpoint, e.g., since gdb tries to re-evaluate the
>> location when the binary changes.]
>> I realize the _p convention mightn't be sufficiently common to use in
>> the Python API, but for example a better name might be stop_p.
>> And then one would have another method (I don't have a good name for
>> it ATM) to use to collect data.
>> Setting aside name choices, I like that API better.
>
> I'm happy with any name.  Lets just call it what the majority are
> comfortable with.  I used "evaluate" purely as a descriptive term to what
> we are doing.  Whatever else fits will work with me.
>
>> OTOH, it seems like Python-based breakpoints have two conditions now
>> (or at least two kinds of conditions one has to think about).
>> One set with the "condition" command and one from the "evaluate" API function.
>> IWBN to have only one, at least conceptually.  Maybe you could rename
>> "evaluate" to "condition" (or some such, I realize there's already a
>> "condition"), and have the default be to use the CLI condition.
>
>
> This is something that is always a ponder-point for Python API hackers.
> There are lots of places where "things" have worked in CLI for many years,
> and we (well, I) do not want to deprecate existing CLI functionality
> while expanding other areas.  I'm not sure what the path is from here.
> Overloading the condition API seems like a pain for the script-writer to
> figure out if it is a Python scripted condition, or a CLI scripted
> condition.  That is why I ended up choosing a different name.

Yeah, a simple overloading of the condition API seems problematic.
Though conceptually having the value either be a string of text or a
method/closure doesn't seem entirely clumsy.
If it's a string it's a standard CLI condition, and if it's a
procedure(function/method/closure) it's Python.
[A simple test might be whether the condition has the __call__ attribute.]
From a u/i perspective is that better?  I don't know but it feels so.
[E.g., one doesn't have to deal with possible confusion over
gdb.Breakpoint having two conditions.]
There may be some implementation details to work out of course.

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

[Tom Tromey]

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

Phil> +	  PyObject *gdbpy_bp_eval = PyString_FromString ("evaluate");

This is leaked.  However...

Phil> +	  if (PyObject_HasAttr (py_bp, gdbpy_bp_eval))
Phil> +	    {
Phil> +	      PyObject *result = PyObject_CallMethodObjArgs (py_bp,
Phil> +							     gdbpy_bp_eval,
Phil> +							     NULL);

You can just use PyObject_HasAttrString and PyObject_CallMethod instead.

Phil> +gdb_py_test_silent_cmd  "python eval_bp1 = bp_eval(\"$bp_location2\")" "Set breakpoint" 0
Phil> +gdb_py_test_silent_cmd  "python also_eval_bp1 = bp_also_eval(\"$bp_location2\")" "Set breakpoint" 0

I think there should also be a test for the case where there is an
ordinary user breakpoint at the same location as the Python breakpoints.
In this case, the user breakpoint should stop, but all the Python
methods should be invoked first.

Tom

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

[Tom Tromey]

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

Phil> +class.  The @code{gdb.Breakpoint} class can be sub-classed and, in
Phil> +particular, you may choose to implement the @code{evaluate} function.

I think this should say "method" and not "function".

Also I think the new method should be defined using @defop, like we do
for pretty-printer methods.  I suggest moving this paragraph down below
the @defmethod for __init__, but before the watchpoint types table, and
putting the @defop there as well.

Tom

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

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> +	  PyObject *gdbpy_bp_eval = PyString_FromString ("evaluate");
>
> This is leaked.  However...
>
> Phil> +	  if (PyObject_HasAttr (py_bp, gdbpy_bp_eval))
> Phil> +	    {
> Phil> +	      PyObject *result = PyObject_CallMethodObjArgs (py_bp,
> Phil> +							     gdbpy_bp_eval,
> Phil> +							     NULL);
>
> You can just use PyObject_HasAttrString and PyObject_CallMethod instead.

Ok, thanks.

> Phil> +gdb_py_test_silent_cmd  "python eval_bp1 = bp_eval(\"$bp_location2\")" "Set breakpoint" 0
> Phil> +gdb_py_test_silent_cmd  "python also_eval_bp1 = bp_also_eval(\"$bp_location2\")" "Set breakpoint" 0
>
> I think there should also be a test for the case where there is an
> ordinary user breakpoint at the same location as the Python breakpoints.
> In this case, the user breakpoint should stop, but all the Python
> methods should be invoked first.

All the breakpoints at a location will be evaluated, but does the order
matter? I can add the test to prove the point, but the Python breakpoints will
have the evaluation function called regardless if the user breakpoint
was first. 

FWIW in bpstop_stop_status a bpstat chain is built from the location
where the inferior stopped.  Then later, we iterate through that chain and
eventually call bpstat_check_breakpoint_conditions.  Then finally in
that condition call we evaluate the Python function.  This loop does not
exit until the all the bpstats in the chain have been iterated.

Cheers

Phil

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

[Phil Muldoon]

Doug Evans <dje@google.com> writes:

>> It's nothing more than a hook for the user to accomplish some truth
>> finding mission at breakpoint evaluation time.  If the user wants to
>> collect data, then I think they can do that -- that is we should not
>> attempt to stop them.  I probably worded my email hastily; FWIW I think
>> this a place where we can write conditional breakpoints that can be
>> written entirely in Python.
>
> I'm not suggesting we stop users from doing quick hacks. :-)
> [Heck, can they already write a breakpoint condition in Python?  E.g.
> with a python convenience function?]

We could call the function "collect" (or whatever we want) and allow the
user to collect data and document it/bless it as a way to do it.  And if
on that data they wanted to issue a stop, they could return True.  We
could remove any mention of it being a replacement for conditional
breakpoints. If that satisfies your requirements, then it is simply a name
change/messaging/documentation tweak.  

If this is just to be a pure data collecting hook, I suspect we would
have to move the code up from bpstat_check_breakpoint_conditions to the
calling function bpstop_stop_status. Then we would not be able to give
the user the functionality to stop.  But it would be the same code in a
different place

I like the first option, and there is no performance loss for
providing the option to stop.

What do you think?

Cheers,
Phil

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

[Tom Tromey]

>>>>> "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

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

[Pedro Alves]

On Monday 13 December 2010 20:45:38, Doug Evans wrote:
> Collecting data in the "evaluate" function feels too hacky for
> something we explicitly tell users is the published way to do this
> kind of thing.

FWIW, I think that's a common idiom, and I'm cool with it.
I recently saw a presentation of a proprietary (quite featureful)
debugger where the equivalent callback was shown as a means of collecting
data as well ("it just always returns FALSE, thus never causes a stop",
or some such).

-- 
Pedro Alves

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

[Tom Tromey]

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

Tom> I think there should also be a test for the case where there is an
Tom> ordinary user breakpoint at the same location as the Python breakpoints.
Tom> In this case, the user breakpoint should stop, but all the Python
Tom> methods should be invoked first.

Phil> All the breakpoints at a location will be evaluated, but does the order
Phil> matter?

The order matters in the sense that GDB can't stop and give the user a
prompt before evaluating all the conditions.

I don't think we should make any promises about the relative order in
which breakpoint conditions will be evaluated.

Phil> I can add the test to prove the point, but the Python breakpoints
Phil> will have the evaluation function called regardless if the user
Phil> breakpoint was first.

Yes, that is what we want to test -- that the condition methods are
evaluated regardless of whether a user breakpoint exists.  This is the
"no interference" case.

Tom

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

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:

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

It's no problem to allow the implemented function to be called
"condition" or anything else.

> 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.

So let me be sure I understand correctly:

class MyBreakpoint (gdb.Breakpoint):
      def condition (self):
        inf_val = gdb.parse_and_eval("i")
        if inf_val == 3:
          return True
        return False

foo = MyBreakpoint ("myfile.c:42")

Would be how we work with the new Python evaluation breakpoints.

And

bar = gdb.Breakpoint("myfile.c:42)

bar.condition = "i == 2"

Would be the old style. In this case "foo" would have had the "condition"
method executed three times (up until i == 2), always returning "False"
(IE, don't stop) before the "bar" condition of the other breakpoint
stopped the inferior with its condition.

In the case of a new style breakpoint having both a CLI .condition() AND
an implemented method (though I could never see why you would want to do
this), whichever of those methods told GDB not to stop would trump the
other. (GDB assumes in bpstat_stop_status that the breakpoint will stop
the inferior (other than special cases like moribund breakpoints) and it
is up to the breakpoint evaluation to say, "hey don't stop".  So the
stop-bit is never set by the conditional stuff, just unset).

How does this sound?

Cheers

Phil

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

[Phil Muldoon]

Phil Muldoon <pmuldoon@redhat.com> writes:

> In the case of a new style breakpoint having both a CLI .condition() AND
> an implemented method (though I could never see why you would want to do
> this), whichever of those methods told GDB not to stop would trump the
> other. (GDB assumes in bpstat_stop_status that the breakpoint will stop
> the inferior (other than special cases like moribund breakpoints) and it
> is up to the breakpoint evaluation to say, "hey don't stop".  So the
> stop-bit is never set by the conditional stuff, just unset).

Reading back this sounds bogus.  We've never had a situation where you
can have two conditions determining the stop status of one breakpoint.  In
that case I think the inverse is true, if one says "stop" it should
trump the other.  In that case, and if that is the chosen path, I'll
have to rewire a few extra things.

Cheers

Phil

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

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

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

> 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

I've appended the latest version of the patch.  What do you think?

Cheers

Phil

--

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6a51a3b..51251cf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -72,6 +72,10 @@
 
 #include "mi/mi-common.h"
 
+#if HAVE_PYTHON
+#include "python/python.h"
+#endif
+
 /* Arguments to pass as context to some catch command handlers.  */
 #define CATCH_PERMANENT ((void *) (uintptr_t) 0)
 #define CATCH_TEMPORARY ((void *) (uintptr_t) 1)
@@ -3964,6 +3968,12 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
       int value_is_zero = 0;
       struct expression *cond;
 
+#if HAVE_PYTHON
+      /* Evaluate Python breakpoints that have a "condition"
+	 method implemented.  */
+      if (b->py_bp_object)
+	bs->stop = gdbpy_cond_evaluate (b->py_bp_object);
+#endif
       if (is_watchpoint (b))
 	cond = b->cond_exp;
       else
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dc9630a..82c1539 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22892,6 +22892,34 @@ argument defines the class of watchpoint to create, if @var{type} is
 assumed to be a @var{WP_WRITE} class.
 @end defmethod
 
+@defop Operation {gdb.Breakpoint} condition (self)
+The @code{gdb.Breakpoint} class can be sub-classed and, in
+particular, you may choose to implement the @code{condition} method.
+If this method is defined as a sub-class of @code{gdb.Breakpoint},
+it will be called when the inferior stops at any location of a
+breakpoint which instantiates that sub-class.  If the method returns
+@code{True}, the inferior will be stopped at the location of the
+breakpoint, otherwise the inferior will continue.
+
+If there are multiple breakpoints at the same location with a
+@code{condition} method, each one will be called regardless of the
+return status of the previous.  This ensures that all @code{condition}
+methods have a chance to execute at that location.  In this scenario
+if one of the methods returns @code{True} but the others return
+@code{False}, the inferior will still be stopped.
+
+Example @code{condition} implementation:
+
+@smallexample
+class MyBreakpoint (gdb.Breakpoint):
+      def condition (self):
+        inf_val = gdb.parse_and_eval("foo")
+        if inf_val == 3:
+          return True
+        return False
+@end smallexample
+@end defop
+
 The available watchpoint types represented by constants are defined in the
 @code{gdb} module:
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 88d9930..57e4555 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -28,6 +28,8 @@
 #include "observer.h"
 #include "cli/cli-script.h"
 #include "ada-lang.h"
+#include "arch-utils.h"
+#include "language.h"
 
 /* From breakpoint.c.  */
 typedef struct breakpoint_object breakpoint_object;
@@ -695,6 +697,45 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
   return PyList_AsTuple (list);
 }
 
+/* Evaluate the "condition" method (if implemented) in
+   the breakpoint class.  If the method returns True, the inferior
+   will be stopped at the breakpoint.  Otherwise the inferior will be
+   allowed to continue.  */
+int
+gdbpy_cond_evaluate (struct breakpoint_object *bp_obj)
+{
+  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
+					       current_language);
+  PyObject *py_bp = (PyObject *) bp_obj;
+  int should_stop = 1;
+  char *method = "condition";
+
+  if (PyObject_HasAttrString (py_bp, method))
+    {
+      PyObject *result = PyObject_CallMethod (py_bp, method, NULL);
+
+      if (result)
+	{
+	  int evaluate = PyObject_IsTrue (result);
+
+	  if (evaluate == -1)
+	    gdbpy_print_stack ();
+
+	  /* If the evaluate function returns False that means the
+	     Python breakpoint wants GDB to continue.  */
+	  if (!evaluate)
+	    should_stop = 0;
+	  
+	  Py_DECREF (result);
+	}
+      else
+	gdbpy_print_stack ();
+    }
+  do_cleanups (cleanup);
+
+  return should_stop;
+}
+
 \f
 
 /* Event callback functions.  */
@@ -887,7 +928,7 @@ static PyTypeObject breakpoint_object_type =
   0,				  /*tp_getattro*/
   0,				  /*tp_setattro*/
   0,				  /*tp_as_buffer*/
-  Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
+  Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,  /*tp_flags*/
   "GDB breakpoint object",	  /* tp_doc */
   0,				  /* tp_traverse */
   0,				  /* tp_clear */
diff --git a/gdb/python/python.h b/gdb/python/python.h
index 04d5c28..53552c6 100644
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -22,6 +22,8 @@
 
 #include "value.h"
 
+struct breakpoint_object;
+
 extern int gdbpy_global_auto_load;
 
 extern void finish_python_initialization (void);
@@ -41,4 +43,6 @@ void preserve_python_values (struct objfile *objfile, htab_t copied_types);
 
 void load_auto_scripts_for_objfile (struct objfile *objfile);
 
+int gdbpy_cond_evaluate (struct breakpoint_object *bp_obj);
+
 #endif /* GDB_PYTHON_H */
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 34a64a3..ce323f8 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -198,3 +198,76 @@ gdb_py_test_silent_cmd  "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WA
 gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
 gdb_test "maint info breakpoints" ".*hw watchpoint.*result.*" "Check maint info breakpoints shows invisible breakpoints"
 gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value = 0.*New value = 25.*" "Test watchpoint write"
+
+# Breakpoints that have an evaluation function.
+
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+    fail "Cannot run to main."
+    return 0
+}
+delete_breakpoints
+
+gdb_py_test_multiple "Sub-class a breakpoint" \
+  "python" "" \
+  "class bp_eval (gdb.Breakpoint):" "" \
+  "   inf_i = 0" "" \
+  "   count = 0" "" \
+  "   def condition (self):" "" \
+  "      self.count = self.count + 1" "" \
+  "      self.inf_i = gdb.parse_and_eval(\"i\")" "" \
+  "      if self.inf_i == 3:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+gdb_py_test_multiple "Sub-class a second breakpoint" \
+  "python" "" \
+  "class bp_also_eval (gdb.Breakpoint):" "" \
+  "   count = 0" "" \
+  "   def condition (self):" "" \
+  "      self.count = self.count + 1" "" \
+  "      if self.count == 9:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+set bp_location2 [gdb_get_line_number "Break at multiply."]
+set end_location [gdb_get_line_number "Break at end."]
+gdb_py_test_silent_cmd  "python eval_bp1 = bp_eval(\"$bp_location2\")" "Set breakpoint" 0
+gdb_py_test_silent_cmd  "python also_eval_bp1 = bp_also_eval(\"$bp_location2\")" "Set breakpoint" 0
+gdb_py_test_silent_cmd  "python never_eval_bp1 = bp_also_eval(\"$end_location\")" "Set breakpoint" 0
+gdb_continue_to_breakpoint "Break at multiply." ".*/$srcfile:$bp_location2.*"
+gdb_test "print i" "3" "Check inferior value matches python accounting"
+gdb_test "python print eval_bp1.inf_i" "3" "Check python accounting matches inferior"
+gdb_test "python print also_eval_bp1.count" "4" \
+    "Check non firing same-location breakpoint eval function was also called at each stop."
+gdb_test "python print eval_bp1.count" "4" \
+    "Check non firing same-location breakpoint eval function was also called at each stop."
+
+delete_breakpoints
+gdb_breakpoint [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd  "python check_eval = bp_eval(\"$bp_location2\")" "Set breakpoint" 0
+gdb_test "python print check_eval.count" "0" \
+    "Test that evaluate function has not been yet executed (ie count = 0)"
+gdb_continue_to_breakpoint "Break at multiply." ".*/$srcfile:$bp_location2.*"
+gdb_test "python print check_eval.count" "1" \
+    "Test that evaluate function is run when location also has normal bp"
+
+gdb_py_test_multiple "Sub-class a watchpoint" \
+  "python" "" \
+  "class wp_eval (gdb.Breakpoint):" "" \
+  "   def condition (self):" "" \
+  "      self.result = gdb.parse_and_eval(\"result\")" "" \
+  "      if self.result == 788:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+delete_breakpoints
+gdb_py_test_silent_cmd  "python wp1 = wp_eval (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE)" "Set watchpoint" 0
+gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value =.*New value = 788.*" "Test watchpoint write"
+gdb_test "python print never_eval_bp1.count" "0" \
+    "Check that this unrelated breakpoints eval function was never called."

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

[Doug Evans]

On Tue, Dec 14, 2010 at 9:28 AM, Tom Tromey <tromey@redhat.com> wrote:
> 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 reference to convenience functions was *not* a serious attempt at
arguing against doing something.
It was just an off the cuff comment regarding hacky solutions.

> 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.

Right.  These are the "existing needs" I was referring to.

> 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.

One thought I had was that maybe one might have a situation where it'd
be useful to run all the collect methods first, and then run all the
stop_p methods.
Plus I still like the API design of keeping data collection separate
from stop/don't-stop.

> 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.

Setting a flag seems hardly onerous.

But don't let my $0.02 cents get in the way ... btw ...
If you *really* want to go this route, go for it.

Btw, if some mutex-checker or whatever detected a condition that it
wanted to report to the user, IWBN to be able to throw an error that
some higher level construct could recognize and deal with.
Is a simple boolean result from stop_p (I'm only calling that for
clarity's sake) going to be sufficient?
[Heh.  And if one bkpt did throw an error in stop_p, one might have
wished one could have collected one's own data first. 1/2 :-)]

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

[Tom Tromey]

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

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.

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.

Tom

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

[Tom Tromey]

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

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

Doug> The reference to convenience functions was *not* a serious attempt at
Doug> arguing against doing something.
Doug> It was just an off the cuff comment regarding hacky solutions.

Sorry, I did not mean to imply that you meant this.
I was just listing our criteria for a solution.

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

Doug> One thought I had was that maybe one might have a situation where it'd
Doug> be useful to run all the collect methods first, and then run all the
Doug> stop_p methods.

Could you give an example?

Doug> Plus I still like the API design of keeping data collection separate
Doug> from stop/don't-stop.

Can you say why?

I like the other way because it is simpler.  Also, I can't think of a
reason I would ever need two methods.

Doug> But don't let my $0.02 cents get in the way ... btw ...
Doug> If you *really* want to go this route, go for it.

Ok.  We can always add a second method if we find a need.

Doug> Btw, if some mutex-checker or whatever detected a condition that it
Doug> wanted to report to the user, IWBN to be able to throw an error that
Doug> some higher level construct could recognize and deal with.
Doug> Is a simple boolean result from stop_p (I'm only calling that for
Doug> clarity's sake) going to be sufficient?

I don't understand.  What higher level are you thinking of?
Could you give an example of when we would want to throw an exception
and what that would mean?

Tom

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

[Doug Evans]

On Wed, Dec 15, 2010 at 12:57 PM, Tom Tromey <tromey@redhat.com> wrote:
> Tom> Having two methods seems worse to me.  It is more complicated without
> Tom> any added benefits.
>
> Doug> One thought I had was that maybe one might have a situation where it'd
> Doug> be useful to run all the collect methods first, and then run all the
> Doug> stop_p methods.
>
> Could you give an example?

Nothing specific, it was just a thought.

> Doug> Plus I still like the API design of keeping data collection separate
> Doug> from stop/don't-stop.
>
> Can you say why?

They're orthogonal requests, and I've had better success with keeping
such things separate.
[But as I say, since they are orthogonal requests, one could add a
data-collection API in a separate patch.]

> Doug> Btw, if some mutex-checker or whatever detected a condition that it
> Doug> wanted to report to the user, IWBN to be able to throw an error that
> Doug> some higher level construct could recognize and deal with.
> Doug> Is a simple boolean result from stop_p (I'm only calling that for
> Doug> clarity's sake) going to be sufficient?
>
> I don't understand.  What higher level are you thinking of?
> Could you give an example of when we would want to throw an exception
> and what that would mean?

[For reference sake, this subthread isn't about an either/or choice of
stop_p or something else.  stop_p is sufficient for some uses.]

For checkers for rule violations (and such) it's nice to stop the
program and notify the user (obviously).  It's also nice for a script
driving the program (say in automated tests) to know why the program
stopped (e.g. you might want to take different actions, e.g. to
further isolate the cause).

For example, what happens after a checker employing stop_p says
"stop"?  There's nothing more in the API that helps the user know
*why* the checker said "stop".  At the moment the API doesn't provide
anything.  Checkers will have to print an error message, but I have a
feeling something more will be useful.
[Did you have something in mind, or am I missing something?]

One thought, and it's just a thought, is to treat rule violations like
segv's.  One could do that in various ways, I'm not suggesting a
particular implementation.  But some of the aspects of getting a segv
are useful here.  E.g., the program can't be resumed until the rule
violation is fixed, and the reason why the program has stopped is
encoded in the program's state.

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

[Tom Tromey]

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

Doug> For example, what happens after a checker employing stop_p says
Doug> "stop"?  There's nothing more in the API that helps the user know
Doug> *why* the checker said "stop".  At the moment the API doesn't provide
Doug> anything.  Checkers will have to print an error message, but I have a
Doug> feeling something more will be useful.
Doug> [Did you have something in mind, or am I missing something?]

The method in question is attached to a particular breakpoint.  So, if
it requests a stop, it will be reported as coming from that breakpoint.

This falls down a little if the breakpoint is internal.  For this I
think we will need to export a breakpoint_ops method to python.

Tom

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

[Doug Evans]

On Tue, Dec 21, 2010 at 12:01 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> For example, what happens after a checker employing stop_p says
> Doug> "stop"?  There's nothing more in the API that helps the user know
> Doug> *why* the checker said "stop".  At the moment the API doesn't provide
> Doug> anything.  Checkers will have to print an error message, but I have a
> Doug> feeling something more will be useful.
> Doug> [Did you have something in mind, or am I missing something?]
>
> The method in question is attached to a particular breakpoint.  So, if
> it requests a stop, it will be reported as coming from that breakpoint.

That feels a bit subtle (and easy for the user to confuse with a
normal breakpoint).
[e.g. as compared to getting a SIGSEGV]

Plus will it also breakdown if a developer sets his/her own breakpoint
at that location?

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

[Tom Tromey]

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

>> The method in question is attached to a particular breakpoint.  So, if
>> it requests a stop, it will be reported as coming from that breakpoint.

Doug> That feels a bit subtle (and easy for the user to confuse with a
Doug> normal breakpoint).

But it will actually be a normal breakpoint.

Doug> Plus will it also breakdown if a developer sets his/her own breakpoint
Doug> at that location?

No, it will continue to do the right thing.  In fact that is one of the
major points of this patch, see the requirements upthread.

Tom

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

[Doug Evans]

On Wed, Dec 22, 2010 at 8:19 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
>>> The method in question is attached to a particular breakpoint.  So, if
>>> it requests a stop, it will be reported as coming from that breakpoint.
>
> Doug> That feels a bit subtle (and easy for the user to confuse with a
> Doug> normal breakpoint).
>
> But it will actually be a normal breakpoint.

Right, that's my point.
The user will see the normal breakpoint output, and then be expected
to know that the breakpoint is special (e.g., a rule was violated and
that s/he needs to pay attention to what the rule was).  Is there a
plan to have something more?  [Or am I missing something?]

> Doug> Plus will it also breakdown if a developer sets his/her own breakpoint
> Doug> at that location?
>
> No, it will continue to do the right thing.  In fact that is one of the
> major points of this patch, see the requirements upthread.

Sorry, maybe I was too subtle.
If one is using this facility to implement a rule checker that stops
when the rule is violated, and one sets another breakpoint at the same
location, when the program stops how does one know whether the rule
was violated or not?
[Especially if all that is reported is the normal breakpoint output.]

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

[Tom Tromey]

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

Doug> The user will see the normal breakpoint output, and then be expected
Doug> to know that the breakpoint is special (e.g., a rule was violated and
Doug> that s/he needs to pay attention to what the rule was).  Is there a
Doug> plan to have something more?  [Or am I missing something?]

Yeah, we want to expose some of the breakpoint_ops stuff to python.

Doug> Sorry, maybe I was too subtle.
Doug> If one is using this facility to implement a rule checker that stops
Doug> when the rule is violated, and one sets another breakpoint at the same
Doug> location, when the program stops how does one know whether the rule
Doug> was violated or not?

One does not.  This is a pre-existing problem in gdb.


Phil, let's withdraw this patch until some later time when we have
worked out the issues.  We can continue to discuss on the archer list.

I think we should at least deal with exposing to Python the bits needed
to customize breakpoint notifications.

Also we should write a couple real-life uses of this to make sure it is
working ok.  At least we should update the glibc heap stuff to use it;
and if other bits are done in time we can either look at the mutex
tracker (requires Sergio's work) or return breakpoints (I think this
requires Sami's work).

Tom

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

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

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

I just came back from Christmas vacation today, so apologies for this
thread time-gap.

> Doug> The user will see the normal breakpoint output, and then be expected
> Doug> to know that the breakpoint is special (e.g., a rule was violated and
> Doug> that s/he needs to pay attention to what the rule was).  Is there a
> Doug> plan to have something more?  [Or am I missing something?]
>
> Yeah, we want to expose some of the breakpoint_ops stuff to python.

Yeah ultimately this was going to be series of patches (the first being
this, the ability to write conditions entirely within Python), but the
whole-story (from these threads) has probably shown we need to think
about the solution in totality, not incrementally. 

> Doug> Sorry, maybe I was too subtle.
> Doug> If one is using this facility to implement a rule checker that stops
> Doug> when the rule is violated, and one sets another breakpoint at the same
> Doug> location, when the program stops how does one know whether the rule
> Doug> was violated or not?
>
> One does not.  This is a pre-existing problem in gdb.
>
>
> Phil, let's withdraw this patch until some later time when we have
> worked out the issues.  We can continue to discuss on the archer list.
>
> I think we should at least deal with exposing to Python the bits needed
> to customize breakpoint notifications.
>
> Also we should write a couple real-life uses of this to make sure it is
> working ok.  

That's ok, no worries.  On that note, I keep a separate branch of my
Python breakpoints improvements patches over on archer within the
archer-pmuldoon-python-breakpoints branch.  That is always open for
folks to pull and comment on.  

We need to determine a concrete API plan though.  What do we need these
improvements to do? The only concrete action I saw was the ability to
override breakpoint operations.  (And probably a callback into the
python api to return the appropriate message). 

What else?

Cheers

Phil

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

[Doug Evans]

On Wed, Jan 5, 2011 at 12:23 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Yeah ultimately this was going to be series of patches (the first being
> this, the ability to write conditions entirely within Python), but the
> whole-story (from these threads) has probably shown we need to think
> about the solution in totality, not incrementally.

I don't mind the incremental approach (fwiw).
There is value in having the stop condition being computed in python.
[As long as we have a clear approach to handling old-style condition
(e.g. break foo if bar) vs new python-style condition.]

It breaks down, I think, in using it for some of the things we've
discussed, but that doesn't diminish the utility of stop_p (or however
it's named).

One thing that may require a non-incremental approach is what kind of
class hierarchy (if any) we want for breakpoints.

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

[Phil Muldoon]

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

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

[Phil Muldoon]

Doug Evans <dje@google.com> writes:

> On Thu, Jan 27, 2011 at 1:41 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>
>     > 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.
>
> Hi.  I'd like to understand this better.
> I'm not advocating any particular position, I just want to make sure we don't make a mistake - it is not trivial to change the published API (if it were
> we'd get rid of SENTINEL_FRAME 1/2 :-)).

I totally agree, but this is still just work-in-progress.  This only exists in
my archer branch.  I apologies for waiting so long to reply, I thought I
had already! 

>
> AIUI, the suggestion is to have "condition" be the cli breakpoint condition and "eval" be another condition available in the python breakpoint API.  Am I
> correct?
> [Apologies, it's been awhile since I looked at this.]

This was purely a collision on naming.  There is the *attribute*
"condition" which has been in the API from the very start.  You could
just set this (IE if foo == 12) and it would be analogous to the user
typing in a condition in the GDB CLI. It accepted a string, and it was
purely a textual interface to the existing GDB CLI conditions framework.   

The new stuff was the Python written condition.  This is where you could
write your condition in Python by implementing a method  called
"condition". Tom noted that if you had an old type condition, the
current code would try to execute it (so it was faulty), as the hasAttr
function will report it exists. My note was that I could not determine
if hasAttr was reporting the condition method, or the the condition
attribute.  So I renamed it back to something I used before.  My trivial
note text just indicated  that we can call it what we like when we agree
on the breakpoint work, it is a 5 minute task to do that.

Cheers

Phil

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

[Tom Tromey]

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

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

You can call `language_def' to get it.

Phil> Also the b->gdbarch member is NULL with watchpoints, so we have to do:
[...]
Phil> I'm not sure why watchpoints have a NULL gdbarch, but if you grep
Phil> watch_command_1, the watchpoint is eventually created with:

I don't know the reason either.  It seems like the watchpoint's
expression will have an arch, so the watchpoint might as well have the
same one.

Tom