[patch] [python] python/12686

[patch] [python] python/12686

[Phil Muldoon]

Kevin Pouget noticed that when GDB called the Python 'stop' callback,
the terminal stilled belonged to the inferior.  GDB intercepts the
signal generated from terminal input and it forces GDB into the
background.  While this patch is simple, I would like feedback.  I
believe the terminal is restored to the inferior when the inferior is
resumed (local tests seem to show this), but I am not totally sure.  Is
acquiring the terminal the right thing to do here? I was trying to think
of a scenario where the Python method would need the inferior terminal,
but could not.

Comments?

Cheers,

Phil

--

2011-04-20  Phil Muldoon  <pmuldoon@redhat.com>

        PR python/12686

	* python/py-breakpoint.c (gdbpy_should_stop): Acquire terminal
	before executing Python code.

2011-04-20  Phil Muldoon  <pmuldoon@redhat.com>

        PR python/12686

	* gdb.python/py-breakpoint.exp: Add terminal input 'stop' method
	test.

--

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 0c21bfc..3ee23be 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -729,7 +729,13 @@ gdbpy_should_stop (struct breakpoint_object *bp_obj)
 
   if (PyObject_HasAttrString (py_bp, stop_func))
     {
-      PyObject *result = PyObject_CallMethod (py_bp, stop_func, NULL);
+      PyObject *result;
+      
+      /* GDB acquires the terminal from the inferior before the
+	 call.  The Python 'stop' method might need to do
+	 input/output.  */
+      terminal_ours ();
+      result = PyObject_CallMethod (py_bp, stop_func, NULL);
 
       if (result)
 	{
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index f0a83f1..d46a37c 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -237,6 +237,15 @@ gdb_py_test_multiple "Sub-class a third breakpoint" \
   "   count = 0" "" \
   "end" ""
 
+gdb_py_test_multiple "Breakpoint that asks for input" \
+  "python" "" \
+  "class ask_bp_input (gdb.Breakpoint):" "" \
+  "    def stop (self):" "" \
+  "       inp = raw_input(\"Input\")" "" \
+  "       print inp" "" \
+  "       return True" "" \
+  "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
@@ -293,3 +302,39 @@ gdb_py_test_silent_cmd  "python wp1 = wp_eval (\"result\", type=gdb.BP_WATCHPOIN
 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."
+
+# Test breakpoints that require user input.
+# Start with a fresh gdb.
+delete_breakpoints
+clean_restart ${testfile}
+
+gdb_py_test_multiple "Breakpoint that asks for input" \
+  "python" "" \
+  "class ask_bp_input (gdb.Breakpoint):" "" \
+  "    def stop (self):" "" \
+  "       inp = raw_input(\"Input? \")" "" \
+  "       print \"Output =\", inp" "" \
+  "       return True" "" \
+  "end" ""
+
+set input_bp [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd  "python np_input = ask_bp_input(\"$input_bp\")" \
+    "Set breakpoint" 0
+send_gdb "run\n"
+gdb_expect {
+    -re "Input\?.*" {
+        pass "Input prompt has appeard and awaiting for input."
+    }
+    timeout {
+        fail "Input prompt has not appearead."
+    }
+}
+send_gdb "123\n"
+gdb_expect {
+    -re "Output = 123.*Breakpoint.*main.*py-breakpoint.*" {
+	pass "Output recieved from input is correct and printed."
+    }
+    timeout {
+        fail "Output not recieved from input prompt."
+    }
+}

Re: [patch] [python] python/12686

[Tom Tromey]

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

Phil> Kevin Pouget noticed that when GDB called the Python 'stop' callback,
Phil> the terminal stilled belonged to the inferior.  GDB intercepts the
Phil> signal generated from terminal input and it forces GDB into the
Phil> background.  While this patch is simple, I would like feedback.  I
Phil> believe the terminal is restored to the inferior when the inferior is
Phil> resumed (local tests seem to show this), but I am not totally sure.  Is
Phil> acquiring the terminal the right thing to do here? I was trying to think
Phil> of a scenario where the Python method would need the inferior terminal,
Phil> but could not.

Phil> Comments?

I don't think we want to do this here.

I'm not really sure of the implications, but it isn't something that
most implementations of the `stop' method are going to want.
If we really need to, we can provide some explicit functions to
manipulate this state from Python.  TBH I think asking the user
questions here is weird.

Tom

Re: [patch] [python] python/12686

[Kevin Pouget]

>  TBH I think asking the user questions here is weird.

I'm not sure about the right way to do, I haven't had time to consider
your suggestion on the main mailing list, but if you consider the
situation where you want to ask the user (from Python) if s/he wants
to stop the inferior.
Currently, one way would be to do it here, the other would be in the
"stop" event handler, with `gdb.execute("continue")', but I don't
really like this solution, being under a Python callstack during the
execution seems a bit strange, doesn't it ?

Cordially,

Kevin

Re: [patch] [python] python/12686

[Tom Tromey]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

Tom>  TBH I think asking the user questions here is weird.

Kevin> I'm not sure about the right way to do, I haven't had time to consider
Kevin> your suggestion on the main mailing list, but if you consider the
Kevin> situation where you want to ask the user (from Python) if s/he wants
Kevin> to stop the inferior.

I would suggest simply not asking.

Kevin> Currently, one way would be to do it here, the other would be in the
Kevin> "stop" event handler, with `gdb.execute("continue")', but I don't
Kevin> really like this solution, being under a Python callstack during the
Kevin> execution seems a bit strange, doesn't it ?

Yeah, if you are committed to asking, then I agree about the placement
of the query.

Tom

Re: [patch] [python] python/12686

[Kevin Pouget]

one thing I just noticed, python debugging ("import pdb ;
pdb.set_trace()") faces the same SIGTTIN problem ...

On Mon, Apr 25, 2011 at 3:28 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>
> Tom>  TBH I think asking the user questions here is weird.
>
> Kevin> I'm not sure about the right way to do, I haven't had time to consider
> Kevin> your suggestion on the main mailing list, but if you consider the
> Kevin> situation where you want to ask the user (from Python) if s/he wants
> Kevin> to stop the inferior.
>
> I would suggest simply not asking.
>
> Kevin> Currently, one way would be to do it here, the other would be in the
> Kevin> "stop" event handler, with `gdb.execute("continue")', but I don't
> Kevin> really like this solution, being under a Python callstack during the
> Kevin> execution seems a bit strange, doesn't it ?
>
> Yeah, if you are committed to asking, then I agree about the placement
> of the query.
>
> Tom
>