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] [python] Implement stop_p for gdb.Breakpoint


Tom Tromey <tromey@redhat.com> writes:

> Tom> My reason is that the Python method is an implementation detail of some
> Tom> kind of "stop point" provided by a Python script. ÂIt is not readily
> Tom> mutable by the user. ÂOn the other hand, the condition is something
> Tom> explicitly under the user's control.
>
> Doug> That sounds a bit weird.
> Doug> The python method is part of an API.
> Doug> APIs are not implementation details.
>
> I think we are using the same words to mean different things.


Here is the latest patch.  I've added the mutual exclusion to setting
the CLI condition and the Python method.  I renamed the method to
'stop'.  Plus all the other review comments regarding the #HAVE Python
defines . I included some changes from the branch that alter how
breakpoints are initialized (the tests require those changes).

I think I got everything.

WDYT?

Cheers

Phil

--


diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 5bcab87..9736b12 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -72,6 +72,7 @@
 #undef savestring
 
 #include "mi/mi-common.h"
+#include "python/python.h"
 
 /* Arguments to pass as context to some catch command handlers.  */
 #define CATCH_PERMANENT ((void *) (uintptr_t) 0)
@@ -643,6 +644,14 @@ condition_command (char *arg, int from_tty)
   ALL_BREAKPOINTS (b)
     if (b->number == bnum)
       {
+	/* Check if this breakpoint has a Python object assigned to
+	   it, and if it has a definition of the "stop"
+	   method.  This method and conditions entered into GDB from
+	   the CLI are mutually exclusive.  */
+	if (b->py_bp_object
+	    && gdbpy_breakpoint_has_py_cond (b->py_bp_object))
+	  error (_("Cannot set a condition where a Python 'stop' " \
+		   "method has been defined in the breakpoint."));
 	set_breakpoint_condition (b, p, from_tty);
 	return;
       }
@@ -4070,6 +4079,11 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
       int value_is_zero = 0;
       struct expression *cond;
 
+      /* Evaluate Python breakpoints that have a "stop"
+	 method implemented.  */
+      if (b->py_bp_object)
+	bs->stop = gdbpy_should_stop (b->py_bp_object);
+
       if (is_watchpoint (b))
 	cond = b->cond_exp;
       else
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a5eaa72..9ca6c09 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23099,6 +23099,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} stop (self)
+The @code{gdb.Breakpoint} class can be sub-classed and, in
+particular, you may choose to implement the @code{stop} 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{stop} method, each one will be called regardless of the
+return status of the previous.  This ensures that all @code{stop}
+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{stop} implementation:
+
+@smallexample
+class MyBreakpoint (gdb.Breakpoint):
+      def stop (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 bfad002..6e5db8f 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"
 
 static PyTypeObject breakpoint_object_type;
 
@@ -38,6 +40,9 @@ static int bppy_live;
    constructor and the breakpoint-created hook function.  */
 static breakpoint_object *bppy_pending_object;
 
+/* Function that is called when a Python condition is evaluated.  */
+char *stop_func = "stop";
+
 struct breakpoint_object
 {
   PyObject_HEAD
@@ -586,10 +591,9 @@ bppy_get_ignore_count (PyObject *self, void *closure)
 }
 
 /* Python function to create a new breakpoint.  */
-static PyObject *
-bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
+static int
+bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 {
-  PyObject *result;
   static char *keywords[] = { "spec", "type", "wp_class", "internal", NULL };
   char *spec;
   int type = bp_breakpoint;
@@ -600,19 +604,16 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
 
   if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiO", keywords,
 				     &spec, &type, &access_type, &internal))
-    return NULL;
+    return -1;
 
   if (internal)
     {
       internal_bp = PyObject_IsTrue (internal);
       if (internal_bp == -1)
-	return NULL;
+	return -1;
     }
 
-  result = subtype->tp_alloc (subtype, 0);
-  if (! result)
-    return NULL;
-  bppy_pending_object = (breakpoint_object *) result;
+  bppy_pending_object = (breakpoint_object *) self;
   bppy_pending_object->number = -1;
   bppy_pending_object->bp = NULL;
   
@@ -649,14 +650,14 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
     }
   if (except.reason < 0)
     {
-      subtype->tp_free (result);
-      return PyErr_Format (except.reason == RETURN_QUIT
-			   ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
-			   "%s", except.message);
+      PyErr_Format (except.reason == RETURN_QUIT
+		    ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
+		    "%s", except.message);
+      return -1;
     }
 
-  BPPY_REQUIRE_VALID ((breakpoint_object *) result);
-  return result;
+  BPPY_SET_REQUIRE_VALID ((breakpoint_object *) self);
+  return 0;
 }
 
 
@@ -707,6 +708,69 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
   return PyList_AsTuple (list);
 }
 
+/* Call the "stop" 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_should_stop (struct breakpoint_object *bp_obj)
+{
+  int stop = 1;
+
+  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);
+
+  if (PyObject_HasAttrString (py_bp, stop_func))
+    {
+      PyObject *result = PyObject_CallMethod (py_bp, stop_func, NULL);
+
+      if (result)
+	{
+	  int evaluate = PyObject_IsTrue (result);
+
+	  if (evaluate == -1)
+	    gdbpy_print_stack ();
+
+	  /* If the "stop" function returns False that means
+	     the Python breakpoint wants GDB to continue.  */
+	  if (! evaluate)
+	    stop = 0;
+
+	  Py_DECREF (result);
+	}
+      else
+	gdbpy_print_stack ();
+    }
+  do_cleanups (cleanup);
+
+  return stop;
+}
+
+/* Checks if the  "stop" method exists in this breakpoint.
+   Used by condition_command to ensure mutual exclusion of breakpoint
+   conditions.  */
+
+int
+gdbpy_breakpoint_has_py_cond (struct breakpoint_object *bp_obj)
+{
+  int has_func = 0;
+  PyObject *py_bp = (PyObject *) bp_obj;
+  struct gdbarch *garch = bp_obj->bp->gdbarch ? bp_obj->bp->gdbarch :
+    get_current_arch ();
+  struct cleanup *cleanup = ensure_python_env (garch, current_language);
+  
+  if (py_bp == NULL)
+    return 0;
+
+  has_func = PyObject_HasAttrString (py_bp, stop_func);
+  do_cleanups (cleanup);
+
+  return has_func;
+}
+
 
 
 /* Event callback functions.  */
@@ -793,7 +857,6 @@ gdbpy_initialize_breakpoints (void)
 {
   int i;
 
-  breakpoint_object_type.tp_new = bppy_new;
   if (PyType_Ready (&breakpoint_object_type) < 0)
     return;
 
@@ -828,6 +891,36 @@ gdbpy_initialize_breakpoints (void)
 
 
 
+/* Helper function that overrides this Python object's
+   PyObject_GenericSetAttr to allow extra validation of the attribute
+   being set.  */
+static int 
+local_setattro (PyObject *self, PyObject *name, PyObject *v)
+{
+  breakpoint_object *obj = (breakpoint_object *) self;  
+  char *attr = python_string_to_host_string (name);
+  
+  if (attr == NULL)
+    return -1;
+  
+  /* If the attribute trying to be set is the "stop" method,
+     but we already have a condition set in the CLI, disallow this
+     operation.  */
+  if (strcmp (attr, stop_func) == 0 && obj->bp->cond_string)
+    {
+      xfree (attr);
+      PyErr_SetString (PyExc_RuntimeError, 
+		       _("Cannot set 'stop' method.  There is an " \
+			 "existing GDB condition attached to the " \
+			 "breakpoint."));
+      return -1;
+    }
+  
+  xfree (attr);
+  
+  return PyObject_GenericSetAttr ((PyObject *)self, name, v);  
+}
+
 static PyGetSetDef breakpoint_object_getset[] = {
   { "enabled", bppy_get_enabled, bppy_set_enabled,
     "Boolean telling whether the breakpoint is enabled.", NULL },
@@ -897,9 +990,9 @@ static PyTypeObject breakpoint_object_type =
   0,				  /*tp_call*/
   0,				  /*tp_str*/
   0,				  /*tp_getattro*/
-  0,				  /*tp_setattro*/
+  (setattrofunc)local_setattro,   /*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 */
@@ -909,5 +1002,13 @@ static PyTypeObject breakpoint_object_type =
   0,				  /* tp_iternext */
   breakpoint_object_methods,	  /* tp_methods */
   0,				  /* tp_members */
-  breakpoint_object_getset	  /* tp_getset */
+  breakpoint_object_getset,	  /* tp_getset */
+  0,				  /* tp_base */
+  0,				  /* tp_dict */
+  0,				  /* tp_descr_get */
+  0,				  /* tp_descr_set */
+  0,				  /* tp_dictoffset */
+  bppy_init,			  /* tp_init */
+  0,				  /* tp_alloc */
+  PyType_GenericNew		  /* tp_new */
 };
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 3b10d8c..2d7213a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -30,6 +30,7 @@
 #include "exceptions.h"
 #include "event-loop.h"
 #include "serial.h"
+#include "python.h"
 
 #include <ctype.h>
 
@@ -864,6 +864,22 @@ source_python_script (FILE *stream, const char *file)
 	       _("Python scripting is not supported in this copy of GDB."));
 }
 
+int
+gdbpy_should_stop (struct breakpoint_object *bp_obj)
+{
+  internal_error (__FILE__, __LINE__,
+		  _("gdbpy_should_stop called when Python scripting is  " \
+		    "not supported."));
+}
+
+int
+gdbpy_breakpoint_has_py_cond (struct breakpoint_object *bp_obj)
+{
+  internal_error (__FILE__, __LINE__,
+		  _("gdbpy_breakpoint_has_py_cond called when Python " \
+		    "scripting is not supported."));
+}
+
 #endif /* HAVE_PYTHON */
 
 
diff --git a/gdb/python/python.h b/gdb/python/python.h
index 58d97fc..ce0eb35 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,8 @@ void preserve_python_values (struct objfile *objfile, htab_t copied_types);
 
 void load_auto_scripts_for_objfile (struct objfile *objfile);
 
+int gdbpy_should_stop (struct breakpoint_object *bp_obj);
+
+int gdbpy_breakpoint_has_py_cond (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 8f58181..f0a83f1 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -195,3 +195,101 @@ 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" ".*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 stop (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 stop (self):" "" \
+  "      self.count = self.count + 1" "" \
+  "      if self.count == 9:" "" \
+  "        return True" "" \
+  "      return False" "" \
+  "end" ""
+
+gdb_py_test_multiple "Sub-class a third breakpoint" \
+  "python" "" \
+  "class basic (gdb.Breakpoint):" "" \
+  "   count = 0" "" \
+  "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
+set cond_bp [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd  "python eval_bp1 = bp_eval(\"$cond_bp\")" "Set breakpoint" 0
+set test_cond {cond $bpnum}
+gdb_test "$test_cond \"foo==3\"" "Cannot set a condition where a Python.*" \
+    "Check you cannot add a CLI condition to a Python breakpoint that" \
+    "has defined stop"
+gdb_py_test_silent_cmd  "python eval_bp2 = basic(\"$cond_bp\")" "Set breakpoint" 0
+gdb_py_test_silent_cmd  "python eval_bp2.condition = \"1==1\"" "Set a condition" 0
+gdb_py_test_multiple "Construct an eval function" \
+  "python" "" \
+  "def stop_func ():" "" \
+  "   return True" "" \
+  "end" ""
+
+gdb_test "python eval_bp2.stop = stop_func"  \
+    "RuntimeError: Cannot set 'stop' method.*" \
+    "Assign stop function to a breakpoint that has a condition"
+
+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 stop (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."


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