[RFA,v3,07/13] Throw a "quit" on a KeyboardException in py-framefilter.c
Commit Message
If a C-c comes while the Python code for a frame filter is running, it
will be turned into a Python KeyboardException. It seems good for
this to be treated like a GDB quit, so this patch changes
py-framefilter.c to notice this situation and call throw_quit in this
case.
gdb/ChangeLog
2018-03-23 Tom Tromey <tom@tromey.com>
* python/py-framefilter.c (throw_quit_or_print_exception): New
function.
(gdbpy_apply_frame_filter): Use it.
gdb/testsuite/ChangeLog
2018-03-23 Tom Tromey <tom@tromey.com>
* gdb.python/py-framefilter.exp: Add test for KeyboardInterrupt.
* gdb.python/py-framefilter.py (name_error): New global.
(ErrorInName.function): Use name_error.
---
gdb/ChangeLog | 6 ++++++
gdb/python/py-framefilter.c | 21 ++++++++++++++++++---
gdb/testsuite/ChangeLog | 6 ++++++
gdb/testsuite/gdb.python/py-framefilter.exp | 10 ++++++++++
gdb/testsuite/gdb.python/py-framefilter.py | 6 +++++-
5 files changed, 45 insertions(+), 4 deletions(-)
Comments
On 03/23/2018 08:55 PM, Tom Tromey wrote:
> +set test "bt 1 with KeyboardInterrupt"
> +gdb_test_multiple "bt 1" $test {
> + -re "Quit" {
> + pass $test
> + }
> +}
What's the gdb output in gdb.log in this case? Is there a GDB prompt
involved?
I'm wondering whether this is racy as is. Does e.g., the test pass
with "make check-read1"?
Or, are we leaving a gdb prompt in the expect buffer unprocessed?
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> On 03/23/2018 08:55 PM, Tom Tromey wrote:
>> +set test "bt 1 with KeyboardInterrupt"
>> +gdb_test_multiple "bt 1" $test {
>> + -re "Quit" {
>> + pass $test
>> + }
>> +}
Pedro> What's the gdb output in gdb.log in this case? Is there a GDB prompt
Pedro> involved?
Here's what gdb.log says:
bt 1
#0 Quit
(gdb) PASS: gdb.python/py-framefilter.exp: bt 1 with KeyboardInterrupt
The test is making a frame filter that throws a KeyboardInterrupt when
called, to simulate the user interrupting the backtrace.
Pedro> I'm wondering whether this is racy as is. Does e.g., the test pass
Pedro> with "make check-read1"?
This works fine.
Pedro> Or, are we leaving a gdb prompt in the expect buffer unprocessed?
I don't think so but I guess I am not really sure.
Tom
On 03/25/2018 05:37 PM, Tom Tromey wrote:
> Pedro> Or, are we leaving a gdb prompt in the expect buffer unprocessed?
>
> I don't think so but I guess I am not really sure.
We are then, since there's a prompt after the Quit that your
gdb_test_multiple is not consuming. That is a recipe for making the
following gdb_test/gdb_test_multiple confused and hit the internal default
prompt/FAIL match in gdb_test_multiple:
...
-re "\r\n$gdb_prompt $" {
if ![string match "" $message] then {
fail "$message"
}
set result 1
}
...
Given the "$", it's going to be racy, though make check-read1
is more likely to catch it.
Sounds like that doesn't happen currently because your test is
the last one before restarting gdb. So if you add more tests after
yours, it's likely you'll see a problem. Or even without more tests,
it can still happen with board files that issue gdb commands when
tearing down the connection or gdb.
So you need to consume the prompt, like:
gdb_test_multiple "bt 1" $test {
- -re "Quit" {
+ -re "Quit\r\n$gdb_prompt $" {
pass $test
}
}
Or simply instead:
gdb_test "bt 1" "Quit" "bt 1 with KeyboardInterrupt"
since gdb_test adds the prompt to the expected string for you.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Or simply instead:
Pedro> gdb_test "bt 1" "Quit" "bt 1 with KeyboardInterrupt"
Pedro> since gdb_test adds the prompt to the expected string for you.
Thanks for the analysis. I've made this change.
Tom
@@ -1305,6 +1305,21 @@ bootstrap_python_frame_filters (struct frame_info *frame,
return iterable.release ();
}
+/* A helper function that will either print an exception or, if it is
+ a KeyboardException, throw a quit. This can only be called when
+ the Python exception is set. */
+
+static void
+throw_quit_or_print_exception ()
+{
+ if (PyErr_ExceptionMatches (PyExc_KeyboardInterrupt))
+ {
+ PyErr_Clear ();
+ throw_quit ("Quit");
+ }
+ gdbpy_print_stack ();
+}
+
/* This is the only publicly exported function in this file. FRAME
is the source frame to start frame-filter invocation. FLAGS is an
integer holding the flags for printing. The following elements of
@@ -1375,7 +1390,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
initialization error. This return code will trigger a
default backtrace. */
- gdbpy_print_stack ();
+ throw_quit_or_print_exception ();
return EXT_LANG_BT_NO_FILTERS;
}
@@ -1398,7 +1413,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
{
if (PyErr_Occurred ())
{
- gdbpy_print_stack ();
+ throw_quit_or_print_exception ();
return EXT_LANG_BT_ERROR;
}
break;
@@ -1423,7 +1438,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
/* Do not exit on error printing a single frame. Print the
error and continue with other frames. */
if (success == EXT_LANG_BT_ERROR)
- gdbpy_print_stack ();
+ throw_quit_or_print_exception ();
}
return success;
@@ -213,6 +213,16 @@ gdb_test_multiple "bt 1" $test {
}
}
+# Now verify that we can see a quit.
+gdb_test_no_output "python name_error = KeyboardInterrupt" \
+ "Change ErrorFilter to throw KeyboardInterrupt"
+set test "bt 1 with KeyboardInterrupt"
+gdb_test_multiple "bt 1" $test {
+ -re "Quit" {
+ pass $test
+ }
+}
+
# Test with no debuginfo
# We cannot use prepare_for_testing as we have to set the safe-patch
@@ -134,13 +134,17 @@ class FrameElider ():
def filter (self, frame_iter):
return ElidingIterator (frame_iter)
+# This is here so the test can change the kind of error that is
+# thrown.
+name_error = RuntimeError
+
# A simple decorator that gives an error when computing the function.
class ErrorInName(FrameDecorator):
def __init__(self, frame):
FrameDecorator.__init__(self, frame)
def function(self):
- raise RuntimeError('whoops')
+ raise name_error('whoops')
# A filter that supplies buggy frames. Disabled by default.
class ErrorFilter():