From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31326 invoked by alias); 11 Mar 2011 11:39:26 -0000 Received: (qmail 31313 invoked by uid 22791); 11 Mar 2011 11:39:24 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 11 Mar 2011 11:39:19 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2BBdF07022136 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 11 Mar 2011 06:39:15 -0500 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p2BBdDdr006442; Fri, 11 Mar 2011 06:39:14 -0500 From: Phil Muldoon To: Tom Tromey Cc: Doug Evans , gdb-patches@sourceware.org Subject: Re: [patch] [python] Implement stop_p for gdb.Breakpoint References: Reply-to: pmuldoon@redhat.com X-URL: http://www.redhat.com Date: Fri, 11 Mar 2011 11:59:00 -0000 In-Reply-To: (Tom Tromey's message of "Thu, 10 Mar 2011 14:43:41 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg00658.txt.bz2 Tom Tromey writes: > Tom> My reason is that the Python method is an implementation detail of s= ome > Tom> kind of "stop point" provided by a Python script. =C2=A0It is not re= adily > Tom> mutable by the user. =C2=A0On the other hand, the condition is somet= hing > 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 =20 #include "mi/mi-common.h" +#include "python/python.h" =20 /* 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 =3D=3D 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 =3D 0; struct expression *cond; =20 + /* Evaluate Python breakpoints that have a "stop" + method implemented. */ + if (b->py_bp_object) + bs->stop =3D gdbpy_should_stop (b->py_bp_object); + if (is_watchpoint (b)) cond =3D 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 =20 +@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 =3D gdb.parse_and_eval("foo") + if inf_val =3D=3D 3: + return True + return False +@end smallexample +@end defop + The available watchpoint types represented by constants are defined in the @code{gdb} module: =20 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" =20 static PyTypeObject breakpoint_object_type; =20 @@ -38,6 +40,9 @@ static int bppy_live; constructor and the breakpoint-created hook function. */ static breakpoint_object *bppy_pending_object; =20 +/* Function that is called when a Python condition is evaluated. */ +char *stop_func =3D "stop"; + struct breakpoint_object { PyObject_HEAD @@ -586,10 +591,9 @@ bppy_get_ignore_count (PyObject *self, void *closure) } =20 /* 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[] =3D { "spec", "type", "wp_class", "internal", NU= LL }; char *spec; int type =3D bp_breakpoint; @@ -600,19 +604,16 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyOb= ject *kwargs) =20 if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiO", keywords, &spec, &type, &access_type, &internal)) - return NULL; + return -1; =20 if (internal) { internal_bp =3D PyObject_IsTrue (internal); if (internal_bp =3D=3D -1) - return NULL; + return -1; } =20 - result =3D subtype->tp_alloc (subtype, 0); - if (! result) - return NULL; - bppy_pending_object =3D (breakpoint_object *) result; + bppy_pending_object =3D (breakpoint_object *) self; bppy_pending_object->number =3D -1; bppy_pending_object->bp =3D NULL; =20=20=20 @@ -649,14 +650,14 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyOb= ject *kwargs) } if (except.reason < 0) { - subtype->tp_free (result); - return PyErr_Format (except.reason =3D=3D RETURN_QUIT - ? PyExc_KeyboardInterrupt : PyExc_RuntimeError, - "%s", except.message); + PyErr_Format (except.reason =3D=3D RETURN_QUIT + ? PyExc_KeyboardInterrupt : PyExc_RuntimeError, + "%s", except.message); + return -1; } =20 - BPPY_REQUIRE_VALID ((breakpoint_object *) result); - return result; + BPPY_SET_REQUIRE_VALID ((breakpoint_object *) self); + return 0; } =20 =0C @@ -707,6 +708,69 @@ gdbpy_breakpoints (PyObject *self, PyObject *args) return PyList_AsTuple (list); } =20 +/* 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 =3D 1; + + PyObject *py_bp =3D (PyObject *) bp_obj; + struct breakpoint *b =3D bp_obj->bp; + struct gdbarch *garch =3D b->gdbarch ? b->gdbarch : get_current_arch (); + struct cleanup *cleanup =3D ensure_python_env (garch, current_language); + + if (PyObject_HasAttrString (py_bp, stop_func)) + { + PyObject *result =3D PyObject_CallMethod (py_bp, stop_func, NULL); + + if (result) + { + int evaluate =3D PyObject_IsTrue (result); + + if (evaluate =3D=3D -1) + gdbpy_print_stack (); + + /* If the "stop" function returns False that means + the Python breakpoint wants GDB to continue. */ + if (! evaluate) + stop =3D 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 =3D 0; + PyObject *py_bp =3D (PyObject *) bp_obj; + struct gdbarch *garch =3D bp_obj->bp->gdbarch ? bp_obj->bp->gdbarch : + get_current_arch (); + struct cleanup *cleanup =3D ensure_python_env (garch, current_language); +=20=20 + if (py_bp =3D=3D NULL) + return 0; + + has_func =3D PyObject_HasAttrString (py_bp, stop_func); + do_cleanups (cleanup); + + return has_func; +} + =0C =20 /* Event callback functions. */ @@ -793,7 +857,6 @@ gdbpy_initialize_breakpoints (void) { int i; =20 - breakpoint_object_type.tp_new =3D bppy_new; if (PyType_Ready (&breakpoint_object_type) < 0) return; =20 @@ -828,6 +891,36 @@ gdbpy_initialize_breakpoints (void) =20 =0C =20 +/* Helper function that overrides this Python object's + PyObject_GenericSetAttr to allow extra validation of the attribute + being set. */ +static int=20 +local_setattro (PyObject *self, PyObject *name, PyObject *v) +{ + breakpoint_object *obj =3D (breakpoint_object *) self;=20=20 + char *attr =3D python_string_to_host_string (name); +=20=20 + if (attr =3D=3D NULL) + return -1; +=20=20 + /* 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) =3D=3D 0 && obj->bp->cond_string) + { + xfree (attr); + PyErr_SetString (PyExc_RuntimeError,=20 + _("Cannot set 'stop' method. There is an " \ + "existing GDB condition attached to the " \ + "breakpoint.")); + return -1; + } +=20=20 + xfree (attr); +=20=20 + return PyObject_GenericSetAttr ((PyObject *)self, name, v);=20=20 +} + static PyGetSetDef breakpoint_object_getset[] =3D { { "enabled", bppy_get_enabled, bppy_set_enabled, "Boolean telling whether the breakpoint is enabled.", NULL }, @@ -897,9 +990,9 @@ static PyTypeObject breakpoint_object_type =3D 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 =3D 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" =20 #include =20 @@ -864,6 +864,22 @@ source_python_script (FILE *stream, const char *file) _("Python scripting is not supported in this copy of GDB.")); } =20 +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 */ =20 =0C 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 @@ =20 #include "value.h" =20 +struct breakpoint_object; + extern int gdbpy_global_auto_load; =20 extern void finish_python_initialization (void); @@ -41,4 +43,8 @@ void preserve_python_values (struct objfile *objfile, hta= b_t copied_types); =20 void load_auto_scripts_for_objfile (struct objfile *objfile); =20 +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 =3D gdb.Breakpoin= t (\"result\", type=3Dgdb.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 in= fo breakpoints shows invisible breakpoints" gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value =3D 0.*New value= =3D 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 =3D 0" "" \ + " count =3D 0" "" \ + " def stop (self):" "" \ + " self.count =3D self.count + 1" "" \ + " self.inf_i =3D gdb.parse_and_eval(\"i\")" "" \ + " if self.inf_i =3D=3D 3:" "" \ + " return True" "" \ + " return False" "" \ + "end" "" + +gdb_py_test_multiple "Sub-class a second breakpoint" \ + "python" "" \ + "class bp_also_eval (gdb.Breakpoint):" "" \ + " count =3D 0" "" \ + " def stop (self):" "" \ + " self.count =3D self.count + 1" "" \ + " if self.count =3D=3D 9:" "" \ + " return True" "" \ + " return False" "" \ + "end" "" + +gdb_py_test_multiple "Sub-class a third breakpoint" \ + "python" "" \ + "class basic (gdb.Breakpoint):" "" \ + " count =3D 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 =3D bp_eval(\"$bp_location2\")" "= Set breakpoint" 0 +gdb_py_test_silent_cmd "python also_eval_bp1 =3D bp_also_eval(\"$bp_locat= ion2\")" "Set breakpoint" 0 +gdb_py_test_silent_cmd "python never_eval_bp1 =3D bp_also_eval(\"$end_loc= ation\")" "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 matche= s inferior" +gdb_test "python print also_eval_bp1.count" "4" \ + "Check non firing same-location breakpoint eval function was also call= ed at each stop." +gdb_test "python print eval_bp1.count" "4" \ + "Check non firing same-location breakpoint eval function was also call= ed at each stop." + +delete_breakpoints +set cond_bp [gdb_get_line_number "Break at multiply."] +gdb_py_test_silent_cmd "python eval_bp1 =3D bp_eval(\"$cond_bp\")" "Set b= reakpoint" 0 +set test_cond {cond $bpnum} +gdb_test "$test_cond \"foo=3D=3D3\"" "Cannot set a condition where a Pytho= n.*" \ + "Check you cannot add a CLI condition to a Python breakpoint that" \ + "has defined stop" +gdb_py_test_silent_cmd "python eval_bp2 =3D basic(\"$cond_bp\")" "Set bre= akpoint" 0 +gdb_py_test_silent_cmd "python eval_bp2.condition =3D \"1=3D=3D1\"" "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 =3D 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 =3D 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 =3D 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 =3D gdb.parse_and_eval(\"result\")" "" \ + " if self.result =3D=3D 788:" "" \ + " return True" "" \ + " return False" "" \ + "end" "" + +delete_breakpoints +gdb_py_test_silent_cmd "python wp1 =3D wp_eval (\"result\", type=3Dgdb.BP= _WATCHPOINT, wp_class=3Dgdb.WP_WRITE)" "Set watchpoint" 0 +gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value =3D.*New value = =3D 788.*" "Test watchpoint write" +gdb_test "python print never_eval_bp1.count" "0" \ + "Check that this unrelated breakpoints eval function was never called."