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

[Phil Muldoon <pmuldoon@redhat.com>]

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