[RFC,2/2] Change SIGINT handler for extension languages only when target terminal is ours
Commit Message
On 12/21/2015 11:30 AM, Yao Qi wrote:
> I see a timeout in gdb.base/random-signal.exp,
>
> Continuing.^M
> PASS: gdb.base/random-signal.exp: continue
> ^CPython Exception <type 'exceptions.KeyboardInterrupt'> <type
> exceptions.KeyboardInterrupt'>: ^M
> FAIL: gdb.base/random-signal.exp: stop with control-c (timeout)
>
> it can be reproduced by running random-signal.exp with native-gdbserver
> in a loop, like this, and the fail will be shown in about 20 runs,
>
> $ (set -e; while true; do make check RUNTESTFLAGS="--target_board=native-gdbserver random-signal.exp"; done)
The reason this doesn't trigger with native debugging is that
in that case, gdb is sharing the terminal with gdb, and with job
control, then the ctrl-c always reaches the inferior instead.
We can trigger the issue with native debugging as well, if
we "attach" instead of "run", like in the patch at the bottom:
$ (set -e; while true; do make check RUNTESTFLAGS="random-signal.exp"; done)
...
FAIL: gdb.base/random-signal.exp: attach: stop with control-c (timeout)
FAIL: gdb.base/random-signal.exp: attach: p wait_gdb = 0 (timeout)
...
gdb.log:
(gdb) PASS: gdb.base/random-signal.exp: attach: watch v
continue
Continuing.
PASS: gdb.base/random-signal.exp: attach: continue
^CPython Exception <type 'exceptions.KeyboardInterrupt'> <type 'exceptions.KeyboardInterrupt'>:
FAIL: gdb.base/random-signal.exp: attach: stop with control-c (timeout)
p wait_gdb = 0
FAIL: gdb.base/random-signal.exp: attach: p wait_gdb = 0 (timeout)
>
> In the test, the program is being single-stepped for software watchpoint,
> and in each internal stop, python unwinder sniffer is used,
>
> #0 pyuw_sniffer (self=<optimised out>, this_frame=<optimised out>, cache_ptr=0xd554f8) at /home/yao/SourceCode/gnu/gdb/git/gdb/python/py-unwind.c:608
> #1 0x00000000006a10ae in frame_unwind_try_unwinder (this_frame=this_frame@entry=0xd554e0, this_cache=this_cache@entry=0xd554f8, unwinder=0xecd540)
> at /home/yao/SourceCode/gnu/gdb/git/gdb/frame-unwind.c:107
> #2 0x00000000006a143f in frame_unwind_find_by_frame (this_frame=this_frame@entry=0xd554e0, this_cache=this_cache@entry=0xd554f8)
> at /home/yao/SourceCode/gnu/gdb/git/gdb/frame-unwind.c:163
> #3 0x000000000069dc6b in compute_frame_id (fi=0xd554e0) at /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:454
> #4 get_prev_frame_if_no_cycle (this_frame=this_frame@entry=0xd55410) at /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:1781
> #5 0x000000000069fdb9 in get_prev_frame_always_1 (this_frame=0xd55410) at /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:1955
> #6 get_prev_frame_always (this_frame=this_frame@entry=0xd55410) at /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:1971
> #7 0x00000000006a04b1 in get_prev_frame (this_frame=this_frame@entry=0xd55410) at /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:2213
>
> when GDB goes to python extension, or other extension language, the
> SIGINT handler is changed, and is restored when GDB leaves extension
> language. GDB only stays in extension language for a very short period
> in this case, but if ctrl-c is pressed at that moment, python extension
> will handle the SIGINT, and exceptions.KeyboardInterrupt is shown.
>
> Language extension is used in GDB side rather than inferior side,
> so GDB should only change SIGINT handler for extension language when
> the terminal is ours (not inferior's). This is what this patch does.
> With this patch applied, I run random-signal.exp in a loop for 18
> hours, and no fail is shown.
>
> gdb:
>
> 2015-12-21 Yao Qi <yao.qi@linaro.org>
>
> * extension.c: Include target.h.
> (set_active_ext_lang): Only call install_gdb_sigint_handler,
> check_quit_flag, and set_quit_flag if target_terminal_is_inferior
> returns false.
> (restore_active_ext_lang): Likewise.
> ---
> gdb/extension.c | 51 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/gdb/extension.c b/gdb/extension.c
> index 1b5365a..1147df9 100644
> --- a/gdb/extension.c
> +++ b/gdb/extension.c
> @@ -22,6 +22,7 @@
>
> #include "defs.h"
> #include <signal.h>
> +#include "target.h"
> #include "auto-load.h"
> #include "breakpoint.h"
> #include "event-top.h"
> @@ -746,19 +747,24 @@ set_active_ext_lang (const struct extension_language_defn *now_active)
> = XCNEW (struct active_ext_lang_state);
>
> previous->ext_lang = active_ext_lang;
> + previous->sigint_handler.handler_saved = 0;
> active_ext_lang = now_active;
>
> - /* If the newly active extension language uses cooperative SIGINT handling
> - then ensure GDB's SIGINT handler is installed. */
> - if (now_active->language == EXT_LANG_GDB
> - || now_active->ops->check_quit_flag != NULL)
> - install_gdb_sigint_handler (&previous->sigint_handler);
> -
> - /* If there's a SIGINT recorded in the cooperative extension languages,
> - move it to the new language, or save it in GDB's global flag if the newly
> - active extension language doesn't use cooperative SIGINT handling. */
> - if (check_quit_flag ())
> - set_quit_flag ();
> + if (!target_terminal_is_inferior ())
I think this should check for terminal_is_ours instead. We also
don't want this with terminal_ours_for_output. In that case, _input_
is still sent to the inferior.
Here's the test tweak:
From 2cf334c489d6278ea73e1f82905b9726f7d502fc Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 4 Jan 2016 14:25:31 +0000
Subject: [PATCH] Test gdb.base/random-signal.exp with "attach"
This exposes the issued fixed by:
https://sourceware.org/ml/gdb-patches/2015-12/msg00423.html
to native debugging as well.
gdb/testsuite/ChangeLog:
2016-01-04 Pedro Alves <palves@redhat.com>
* gdb.base/random-signal.c (wait): New global.
(main): Use it.
* gdb.base/random-signal.exp (do_test): New procedure, with body
of testcase moved in.
(top level) Call it twice, once with "run" and once with "attach".
---
gdb/testsuite/gdb.base/random-signal.c | 7 +++-
gdb/testsuite/gdb.base/random-signal.exp | 58 ++++++++++++++++++++++++++------
2 files changed, 53 insertions(+), 12 deletions(-)
@@ -19,11 +19,16 @@
int v;
+/* GDB clears this. */
+volatile int wait_gdb = 1;
+
int main()
{
/* Don't let the test case run forever. */
alarm (60);
- for (;;)
+ while (wait_gdb)
;
+
+ return 0;
}
@@ -30,19 +30,55 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
return -1
}
-if {![runto_main]} {
- return -1
+# Set a software watchpoint, continue, wait a bit and stop the target
+# with ctrl-c. A software watchpoint forces the target to
+# single-step.
+proc do_test {} {
+ global binfile
+
+ gdb_test_no_output "set can-use-hw-watchpoints 0"
+ gdb_test "watch v" "Watchpoint .*"
+ gdb_test_multiple "continue" "continue" {
+ -re "Continuing" {
+ pass "continue"
+ }
+ }
+
+ # For this to work we must be sure to consume the "Continuing."
+ # message first, or GDB's signal handler may not be in place.
+ after 500 {send_gdb "\003"}
+ gdb_test "" "Program received signal SIGINT.*" "stop with control-c"
+
+ gdb_test "p wait_gdb = 0" " = 0"
}
-gdb_test_no_output "set can-use-hw-watchpoints 0"
-gdb_test "watch v" "Watchpoint .*"
-gdb_test_multiple "continue" "continue" {
- -re "Continuing" {
- pass "continue"
+# With native debugging and "run" (with job control), the ctrl-c
+# always reaches the inferior, not gdb, even if ctrl-c is pressed
+# while gdb is processing the internal software watchtpoint
+# single-step. With remote debugging, the ctrl-c reaches GDB first.
+with_test_prefix "run" {
+ clean_restart $binfile
+
+ if {![runto_main]} {
+ return -1
}
+
+ do_test
}
-# For this to work we must be sure to consume the "Continuing."
-# message first, or GDB's signal handler may not be in place.
-after 500 {send_gdb "\003"}
-gdb_test "" "Program received signal SIGINT.*" "stop with control-c"
+# With "attach" however, even with native debugging, the ctrl-c always
+# reaches GDB first. Test that as well.
+with_test_prefix "attach" {
+ if {[can_spawn_for_attach]} {
+ clean_restart $binfile
+
+ set test_spawn_id [spawn_wait_for_attach $binfile]
+ set testpid [spawn_id_get_pid $test_spawn_id]
+
+ gdb_test "attach $testpid" "Attaching to.*process $testpid.*libc.*" "attach"
+
+ do_test
+
+ kill_wait_spawned_process $test_spawn_id
+ }
+}