From patchwork Mon Jan 4 16:12:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 10215 Received: (qmail 30728 invoked by alias); 4 Jan 2016 16:12:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 30707 invoked by uid 89); 4 Jan 2016 16:12:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL, BAYES_50, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=pressed, 608, 1781, 2213 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 04 Jan 2016 16:12:21 +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 (Postfix) with ESMTPS id A43494BC5A; Mon, 4 Jan 2016 16:12:20 +0000 (UTC) Received: from [127.0.0.1] (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 u04GCJnb000913; Mon, 4 Jan 2016 11:12:20 -0500 Message-ID: <568A99E3.7020703@redhat.com> Date: Mon, 04 Jan 2016 16:12:19 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Yao Qi , gdb-patches@sourceware.org Subject: Re: [RFC 2/2] Change SIGINT handler for extension languages only when target terminal is ours References: <1450697443-29067-1-git-send-email-yao.qi@linaro.org> <1450697443-29067-3-git-send-email-yao.qi@linaro.org> In-Reply-To: <1450697443-29067-3-git-send-email-yao.qi@linaro.org> 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 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 : 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=, this_frame=, 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 > > * 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 > +#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 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 * 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(-) diff --git a/gdb/testsuite/gdb.base/random-signal.c b/gdb/testsuite/gdb.base/random-signal.c index f4c4b9f..45adbf0 100644 --- a/gdb/testsuite/gdb.base/random-signal.c +++ b/gdb/testsuite/gdb.base/random-signal.c @@ -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; } diff --git a/gdb/testsuite/gdb.base/random-signal.exp b/gdb/testsuite/gdb.base/random-signal.exp index a6170cc..9aa5843 100644 --- a/gdb/testsuite/gdb.base/random-signal.exp +++ b/gdb/testsuite/gdb.base/random-signal.exp @@ -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 + } +}