[PATCH v1 05/13] script language API for GDB: breakpoint changes

[PATCH v1 05/13] script language API for GDB: breakpoint changes

[xdje42]

This patch updates breakpoint.c to call the API functions in scripting.h.
It also updates py-breakpoint.c, the functions that implement the
script_language_ops "methods" are all named ${lang}_method_name.

The script condition test function, gdbpy_breakpoint_cond_says_stop,
has an enum result instead of just a boolean so that the caller can
know if there is a condition.  One could call the "breakpoint_has_cond"
"method" except that python "finish breakpoints" are implemented on
top of normal breakpoints and require calling "breakpoint_cond_says_stop"
even if there is no stop method.  I can imagine reworking this,
but there's no need to at the moment.

It also updates some expected output in py-breakpoint.exp.

2013-12-05  Doug Evans  <xdje42@gmail.com>

	* breakpoint.c (condition_command): Replace call to
	gdbpy_breakpoint_has_py_cond with call to breakpoint_has_script_cond.
	(bpstat_check_breakpoint_conditions): Replace call to gdbpy_should_stop
	with call to breakpoint_script_cond_says_stop.
	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Renamed
	from gdbpy_should_stop.  Change result type to enum scr_bp_stop.
	New arg slang.  Return SCR_BP_STOP_UNSET if py_bp_object is NULL.
	(gdbpy_breakpoint_has_cond): Renamed from gdbpy_breakpoint_has_py_cond.
	New arg slang.
	(local_setattro): Call breakpoint_has_script_cond.

	testsuite/
	* gdb.python/py-breakpoint.exp (test_bkpt_eval_funcs): Update expected
	output.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 111660f..4064e7b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1052,10 +1052,11 @@ condition_command (char *arg, int from_tty)
 	   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."));
+	if (breakpoint_has_script_cond (b, SCRIPT_LANG_NONE))
+	  {
+	    error (_("Cannot set a condition where a scripting language"
+		     " 'stop' method has already been defined."));
+	  }
 	set_breakpoint_condition (b, p, from_tty);
 
 	if (is_breakpoint (b))
@@ -5140,9 +5141,9 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
       return;
     }
 
-  /* Evaluate Python breakpoints that have a "stop" method implemented.  */
-  if (b->py_bp_object)
-    bs->stop = gdbpy_should_stop (b->py_bp_object);
+  /* Evaluate scripting language breakpoints that have a "stop" method
+     implemented.  */
+  bs->stop = breakpoint_script_cond_says_stop (b);
 
   if (is_watchpoint (b))
     {
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 1085588..882ac7f 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -750,15 +750,22 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
    stopped at the breakpoint.  Otherwise the inferior will be
    allowed to continue.  */
 
-int
-gdbpy_should_stop (struct gdbpy_breakpoint_object *bp_obj)
+enum scr_bp_stop
+gdbpy_breakpoint_cond_says_stop (const struct script_language_defn *slang,
+				 struct breakpoint *b)
 {
-  int stop = 1;
-
+  int stop;
+  struct gdbpy_breakpoint_object *bp_obj = b->py_bp_object;
   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);
+  struct gdbarch *garch;
+  struct cleanup *cleanup;
+
+  if (bp_obj == NULL)
+    return SCR_BP_STOP_UNSET;
+
+  stop = -1;
+  garch = b->gdbarch ? b->gdbarch : get_current_arch ();
+  cleanup = ensure_python_env (garch, current_language);
 
   if (bp_obj->is_finish_bp)
     bpfinishpy_pre_stop_hook (bp_obj);
@@ -767,6 +774,7 @@ gdbpy_should_stop (struct gdbpy_breakpoint_object *bp_obj)
     {
       PyObject *result = PyObject_CallMethod (py_bp, stop_func, NULL);
 
+      stop = 1;
       if (result)
 	{
 	  int evaluate = PyObject_IsTrue (result);
@@ -790,7 +798,9 @@ gdbpy_should_stop (struct gdbpy_breakpoint_object *bp_obj)
 
   do_cleanups (cleanup);
 
-  return stop;
+  if (stop < 0)
+    return SCR_BP_STOP_UNSET;
+  return stop ? SCR_BP_STOP_YES : SCR_BP_STOP_NO;
 }
 
 /* Checks if the  "stop" method exists in this breakpoint.
@@ -798,17 +808,21 @@ gdbpy_should_stop (struct gdbpy_breakpoint_object *bp_obj)
    conditions.  */
 
 int
-gdbpy_breakpoint_has_py_cond (struct gdbpy_breakpoint_object *bp_obj)
+gdbpy_breakpoint_has_cond (const struct script_language_defn *slang,
+			   struct breakpoint *b)
 {
-  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)
-    has_func = PyObject_HasAttrString (py_bp, stop_func);
-
+  int has_func;
+  PyObject *py_bp;
+  struct gdbarch *garch;
+  struct cleanup *cleanup;
+
+  if (b->py_bp_object == NULL)
+    return 0;
+
+  py_bp = (PyObject *) b->py_bp_object;
+  garch = b->gdbarch ? b->gdbarch : get_current_arch ();
+  cleanup = ensure_python_env (garch, current_language);
+  has_func = PyObject_HasAttrString (py_bp, stop_func);
   do_cleanups (cleanup);
 
   return has_func;
@@ -949,7 +963,9 @@ local_setattro (PyObject *self, PyObject *name, PyObject *v)
   /* 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)
+  if (strcmp (attr, stop_func) == 0
+      && (obj->bp->cond_string != NULL
+	  || breakpoint_has_script_cond (obj->bp, SCRIPT_LANG_PYTHON)))
     {
       xfree (attr);
       PyErr_SetString (PyExc_RuntimeError,
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 107b026..356ef30 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -372,7 +372,7 @@ proc test_bkpt_eval_funcs { } {
 	    "Set breakpoint" 0
 	set test_cond {cond $bpnum}
 	gdb_test "$test_cond \"foo==3\"" \
-	    "Cannot set a condition where a Python.*" \
+	    "Cannot set a condition where a scripting language.*" \
 	    "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

Re: [PATCH v1 05/13] script language API for GDB: breakpoint changes

[Phil Muldoon]

On 06/12/13 05:52, xdje42@gmail.com wrote:

> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 111660f..4064e7b 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1052,10 +1052,11 @@ condition_command (char *arg, int from_tty)
>  	   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."));
> +	if (breakpoint_has_script_cond (b, SCRIPT_LANG_NONE))
> +	  {
> +	    error (_("Cannot set a condition where a scripting language"
> +		     " 'stop' method has already been defined."));
> +	  }
>  	set_breakpoint_condition (b, p, from_tty);

As you moved the entire check into the Python ops structure, it occurs
to me that we might have to tweak struct breakpoint.  Instead of
having each language attach its object to struct breakpoint, we just
make the pointer to the scripting object void * (and rename it to
bp->script_object).  I think this would be OK as only one scripting
language may attach a condition call-back to a breakpoint? What do you
think?

Cheers,

Phil

Re: [PATCH v1 05/13] script language API for GDB: breakpoint changes

[Doug Evans]

Phil Muldoon <pmuldoon@redhat.com> writes:

> On 06/12/13 05:52, xdje42@gmail.com wrote:
>
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index 111660f..4064e7b 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -1052,10 +1052,11 @@ condition_command (char *arg, int from_tty)
>>  	   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."));
>> +	if (breakpoint_has_script_cond (b, SCRIPT_LANG_NONE))
>> +	  {
>> +	    error (_("Cannot set a condition where a scripting language"
>> +		     " 'stop' method has already been defined."));
>> +	  }
>>  	set_breakpoint_condition (b, p, from_tty);
>
> As you moved the entire check into the Python ops structure, it occurs
> to me that we might have to tweak struct breakpoint.  Instead of
> having each language attach its object to struct breakpoint, we just
> make the pointer to the scripting object void * (and rename it to
> bp->script_object).  I think this would be OK as only one scripting
> language may attach a condition call-back to a breakpoint? What do you
> think?

That's certainly a possibility (as long as we keep the restriction in
place, and it's what's there now).

One could still be type-safe and have a union,
plus an enum to say which one is in use, if any.

Thanks!
[For all the reviews, not just this one.]