| Message ID | 1438013299-1449-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 71951 invoked by alias); 27 Jul 2015 16:08:31 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 71930 invoked by uid 89); 27 Jul 2015 16:08:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-qg0-f54.google.com Received: from mail-qg0-f54.google.com (HELO mail-qg0-f54.google.com) (209.85.192.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 27 Jul 2015 16:08:29 +0000 Received: by qged69 with SMTP id d69so55060362qge.0 for <gdb-patches@sourceware.org>; Mon, 27 Jul 2015 09:08:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=xHlgkv99haQLH/jImqfiPnLHzIlOJhYXje1nt3yf/54=; b=N4t/03SSx3ZP7L/J10gE0CjA2VnyvvoDH5/l3MRq/548pER5KNYaBzE/q9A6XcrOQl EO484Qy+6cx1xgEHQ57EfSQNtD0qfDVh8y32yAx0qfww2aadLqmZKiloUSDm1l7catDM MMIXIMDUMF/T0DKetU57BvndlaCC7VE9NVJeOX/VthedKTDty76XL+3CAjJKc7ngFE5Q D/fKRzoSYcHFX3AeEmshdL9CG5IU0q5s+cuNbEKtfKjqFFLozywy0uCG2Hu676mk9Sj2 nXutAiI5V7JVLZzICApnmE94oXkSivScRq3ztvXExbTmcuP23NjPTSAuJHhODHuO/+2U 3iBw== X-Gm-Message-State: ALoCoQk7fffQcSEV1+6WP0XPIDfx2vI8iqVGoLAVKTm5MS60y8ziAfwzr6Np83tiDVU3kQSnqrgw X-Received: by 10.140.147.196 with SMTP id 187mr31550791qht.59.1438013307612; Mon, 27 Jul 2015 09:08:27 -0700 (PDT) Received: from localhost.localdomain (ool-4353acd8.dyn.optonline.net. [67.83.172.216]) by smtp.gmail.com with ESMTPSA id e21sm9422402qhc.14.2015.07.27.09.08.26 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 27 Jul 2015 09:08:27 -0700 (PDT) From: Patrick Palka <patrick@parcs.ath.cx> To: gdb-patches@sourceware.org Cc: Patrick Palka <patrick@parcs.ath.cx> Subject: [PATCH] Call target_terminal_ours in quit_force Date: Mon, 27 Jul 2015 12:08:19 -0400 Message-Id: <1438013299-1449-1-git-send-email-patrick@parcs.ath.cx> |
Commit Message
Patrick Palka
July 27, 2015, 4:08 p.m. UTC
We should make sure our terminal settings are in effect before finally quitting GDB. Our terminal settings may not be in effect at this point if we are e.g. quitting due to a SIGTERM. gdb/ChangeLog: * top.c (quit_force): Call target_terminal_ours. --- gdb/top.c | 2 ++ 1 file changed, 2 insertions(+)
Comments
On Mon, Jul 27, 2015 at 12:08 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: > We should make sure our terminal settings are in effect before finally > quitting GDB. Our terminal settings may not be in effect at this point > if we are e.g. quitting due to a SIGTERM. I should add, "quitting due to a SIGTERM while an inferior an inferior is running in the foreground."
Patrick Palka <patrick@parcs.ath.cx> writes: > On Mon, Jul 27, 2015 at 12:08 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >> We should make sure our terminal settings are in effect before finally >> quitting GDB. Our terminal settings may not be in effect at this point >> if we are e.g. quitting due to a SIGTERM. > > I should add, "quitting due to a SIGTERM while an inferior an inferior s/an inferior an inferior/an inferior/ > is running in the foreground." Andreas.
On 07/27/2015 05:11 PM, Patrick Palka wrote: > On Mon, Jul 27, 2015 at 12:08 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >> We should make sure our terminal settings are in effect before finally >> quitting GDB. Our terminal settings may not be in effect at this point >> if we are e.g. quitting due to a SIGTERM. > > I should add, "quitting due to a SIGTERM while an inferior an inferior > is running in the foreground." Looks OK, though I notice that the settings are broken even if we we're not debugging anything: $ stty speed 38400 baud; line = 0; iutf8 $ ./gdb GNU gdb (GDB) 7.10.50.20150726-cvs (gdb) *sent SIGTERM from another terminal, gdb exits* $ $ stty (echo is off) speed 38400 baud; line = 0; lnext = <undef>; min = 1; time = 0; -icrnl iutf8 -icanon -echo $ Do you also see this? Can I convince you to add a test? :-) You wouldn't have to write it from scratch; we already have a test that checks that terminal settings are preserved: gdb.base/batch-preserve-term-settings.exp Thanks, Pedro Alves
On Mon, Jul 27, 2015 at 12:39 PM, Pedro Alves <palves@redhat.com> wrote: > On 07/27/2015 05:11 PM, Patrick Palka wrote: >> On Mon, Jul 27, 2015 at 12:08 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >>> We should make sure our terminal settings are in effect before finally >>> quitting GDB. Our terminal settings may not be in effect at this point >>> if we are e.g. quitting due to a SIGTERM. >> >> I should add, "quitting due to a SIGTERM while an inferior an inferior >> is running in the foreground." > > Looks OK, though I notice that the settings are broken even if we > we're not debugging anything: > > $ stty > speed 38400 baud; line = 0; > iutf8 > > $ ./gdb > GNU gdb (GDB) 7.10.50.20150726-cvs > (gdb) > *sent SIGTERM from another terminal, gdb exits* > $ > $ stty (echo is off) > speed 38400 baud; line = 0; > lnext = <undef>; min = 1; time = 0; > -icrnl iutf8 > -icanon -echo > $ > > Do you also see this? Yeah, even with this patch... $ stty speed 38400 baud; line = 0; -brkint -imaxbel iutf8 $ gdb -q (gdb) *SIGTERM* $ stty speed 38400 baud; line = 0; lnext = <undef>; -brkint -icrnl -imaxbel iutf8 Quitting via the "quit" command is OK though... strange.
On Mon, Jul 27, 2015 at 2:49 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: > On Mon, Jul 27, 2015 at 12:39 PM, Pedro Alves <palves@redhat.com> wrote: >> On 07/27/2015 05:11 PM, Patrick Palka wrote: >>> On Mon, Jul 27, 2015 at 12:08 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >>>> We should make sure our terminal settings are in effect before finally >>>> quitting GDB. Our terminal settings may not be in effect at this point >>>> if we are e.g. quitting due to a SIGTERM. >>> >>> I should add, "quitting due to a SIGTERM while an inferior an inferior >>> is running in the foreground." >> >> Looks OK, though I notice that the settings are broken even if we >> we're not debugging anything: >> >> $ stty >> speed 38400 baud; line = 0; >> iutf8 >> >> $ ./gdb >> GNU gdb (GDB) 7.10.50.20150726-cvs >> (gdb) >> *sent SIGTERM from another terminal, gdb exits* >> $ >> $ stty (echo is off) >> speed 38400 baud; line = 0; >> lnext = <undef>; min = 1; time = 0; >> -icrnl iutf8 >> -icanon -echo >> $ >> >> Do you also see this? > > Yeah, even with this patch... > > $ stty > speed 38400 baud; line = 0; > -brkint -imaxbel iutf8 > $ gdb -q > (gdb) *SIGTERM* > $ stty > speed 38400 baud; line = 0; > lnext = <undef>; > -brkint -icrnl -imaxbel iutf8 > > Quitting via the "quit" command is OK though... strange. This happens because when quitting via SIGTERM a readline callback handler remains installed which means that the terminal is still prepped by readline. The readline callback handler is temporarily removed during the execution of a command (thus deprepping the terminal) which is why quitting via "quit" does not leak our terminal settings.
On 07/27/2015 08:12 PM, Patrick Palka wrote: > On Mon, Jul 27, 2015 at 2:49 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >> On Mon, Jul 27, 2015 at 12:39 PM, Pedro Alves <palves@redhat.com> wrote: >>> On 07/27/2015 05:11 PM, Patrick Palka wrote: >>>> On Mon, Jul 27, 2015 at 12:08 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >>>>> We should make sure our terminal settings are in effect before finally >>>>> quitting GDB. Our terminal settings may not be in effect at this point >>>>> if we are e.g. quitting due to a SIGTERM. >>>> >>>> I should add, "quitting due to a SIGTERM while an inferior an inferior >>>> is running in the foreground." >>> >>> Looks OK, though I notice that the settings are broken even if we >>> we're not debugging anything: >>> >>> $ stty >>> speed 38400 baud; line = 0; >>> iutf8 >>> >>> $ ./gdb >>> GNU gdb (GDB) 7.10.50.20150726-cvs >>> (gdb) >>> *sent SIGTERM from another terminal, gdb exits* >>> $ >>> $ stty (echo is off) >>> speed 38400 baud; line = 0; >>> lnext = <undef>; min = 1; time = 0; >>> -icrnl iutf8 >>> -icanon -echo >>> $ >>> >>> Do you also see this? >> >> Yeah, even with this patch... >> >> $ stty >> speed 38400 baud; line = 0; >> -brkint -imaxbel iutf8 >> $ gdb -q >> (gdb) *SIGTERM* >> $ stty >> speed 38400 baud; line = 0; >> lnext = <undef>; >> -brkint -icrnl -imaxbel iutf8 >> >> Quitting via the "quit" command is OK though... strange. > > This happens because when quitting via SIGTERM a readline callback > handler remains installed which means that the terminal is still > prepped by readline. The readline callback handler is temporarily > removed during the execution of a command (thus deprepping the > terminal) which is why quitting via "quit" does not leak our terminal > settings. Yeah. Sounds like we should call gdb_rl_callback_handler_remove. And remove the input fd from the event loop too, otherwise pending input may end up waking some nested event loop and end up in readline with no callback installed, which aborts. Looks like gdb_disable_readline would do? Thanks, Pedro Alves
diff --git a/gdb/top.c b/gdb/top.c index 1e30b1c..1a31194 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1494,6 +1494,8 @@ quit_force (char *args, int from_tty) int exit_code = 0; struct qt_args qt; + target_terminal_ours (); + /* An optional expression may be used to cause gdb to terminate with the value of that expression. */ if (args)