From patchwork Tue Jan 5 16:52:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 10228 Received: (qmail 37109 invoked by alias); 5 Jan 2016 16:52:38 -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 37091 invoked by uid 89); 5 Jan 2016 16:52:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=UD:target.c, targetc, target.c X-HELO: mail-pa0-f41.google.com Received: from mail-pa0-f41.google.com (HELO mail-pa0-f41.google.com) (209.85.220.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 05 Jan 2016 16:52:35 +0000 Received: by mail-pa0-f41.google.com with SMTP id yy13so125266308pab.3 for ; Tue, 05 Jan 2016 08:52:34 -0800 (PST) X-Received: by 10.66.177.193 with SMTP id cs1mr77419723pac.132.1452012753215; Tue, 05 Jan 2016 08:52:33 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id g68sm129364582pfg.9.2016.01.05.08.52.31 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 05 Jan 2016 08:52:32 -0800 (PST) From: Yao Qi To: Pedro Alves Cc: 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> <568A99E3.7020703@redhat.com> Date: Tue, 05 Jan 2016 16:52:29 +0000 In-Reply-To: <568A99E3.7020703@redhat.com> (Pedro Alves's message of "Mon, 04 Jan 2016 16:12:19 +0000") Message-ID: <86h9isx92q.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Pedro Alves writes: > The reason this doesn't trigger with native debugging is that > in that case, gdb is sharing the terminal with gdb, and with job I think you meant "gdb is sharing the terminal with the inferior". > 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 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) > > Yes, it is a good test case. >> + 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. OK, target_terminal_is_ours is added and is used here. Patch is updated as below, > > 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". Your patch looks good to me. diff --git a/gdb/extension.c b/gdb/extension.c index 5c23bcc..d2c5669 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_ours ()) + { + /* 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 (); + } return previous; } @@ -772,16 +778,19 @@ restore_active_ext_lang (struct active_ext_lang_state *previous) active_ext_lang = previous->ext_lang; - /* Restore the previous SIGINT handler if one was saved. */ - if (previous->sigint_handler.handler_saved) - install_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_ours ()) + { + /* Restore the previous SIGINT handler if one was saved. */ + if (previous->sigint_handler.handler_saved) + install_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 (); + } xfree (previous); } diff --git a/gdb/target.c b/gdb/target.c index d331fe4..e88d60c 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -466,6 +466,14 @@ target_terminal_is_inferior (void) /* See target.h. */ +int +target_terminal_is_ours (void) +{ + return (terminal_state == terminal_is_ours); +} + +/* See target.h. */ + void target_terminal_inferior (void) { diff --git a/gdb/target.h b/gdb/target.h index dd2896f..e1419a9 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1503,6 +1503,10 @@ extern int target_remove_breakpoint (struct gdbarch *gdbarch, extern int target_terminal_is_inferior (void); +/* Returns true if our terminal settings are in effect. */ + +extern int target_terminal_is_ours (void); + /* Initialize the terminal settings we record for the inferior, before we actually run the inferior. */