From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69555 invoked by alias); 23 May 2019 20:32:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 69547 invoked by uid 89); 23 May 2019 20:32:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,UNSUBSCRIBE_BODY autolearn=no version=3.3.1 spammy=claiming, pumps, foreground, wonders X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 May 2019 20:32:47 +0000 Received: by mail-wr1-f65.google.com with SMTP id w8so7671536wrl.6 for ; Thu, 23 May 2019 13:32:47 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4eeb:42ff:feef:f164? ([2001:8a0:f913:f700:4eeb:42ff:feef:f164]) by smtp.gmail.com with ESMTPSA id x22sm764307wmi.4.2019.05.23.13.32.44 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Thu, 23 May 2019 13:32:44 -0700 (PDT) Subject: Re: [PATCH] Supress SIGTTOU when handling errors To: Andrew Burgess , Alan Hayward References: <20190516155150.71826-1-alan.hayward@arm.com> <20190516180640.GS2568@embecosm.com> Cc: "gdb-patches@sourceware.org" , nd From: Pedro Alves Message-ID: Date: Thu, 23 May 2019 20:32:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190516180640.GS2568@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-05/txt/msg00548.txt.bz2 On 5/16/19 7:06 PM, Andrew Burgess wrote: > As I understand it the problem you're seeing could be resolved by > making sure that the terminal is correctly claimed by GDB at the point > tcdrain is called. This would require a call to either > 'target_terminal::ours ()' or 'target_terminal::ours_for_output ()'. > > I've made several attempts to fix this in the past (non posted > though), one problem I've previously run into that I've not yet > understood is that calling ::ours_for_output doesn't seem to be enough > to prevent the SIGTTOU (I assume from the tcdrain) and only calling > ::ours is enough. > > What I've been trying to figure out is what the intended difference > between ::ours_for_output and ::ours actually is, The point of ours_for_output is that while the inferior is still running, leave it in the foreground, such that gdb doesn't interfere with the terminal mode, Ctrl-C reaches the inferior directly, or such that any keyboard/stdin input typed by the user goes to the inferior directly, not to gdb. The latter is particularly important for a follow up series I was working on but never got a chance to submit, here: https://github.com/palves/gdb/commits/palves/tty-always-separate-session With that branch, gdb always puts the inferior in a separate session, and then pumps stdin/stdout between the inferior's tty and gdb's tty at the right times. That branch solves problems like not being able to Ctrl-C while the inferior is blocked in sigwait with SIGINT blocked (gdb/14559, gdb/9425). That's the fix I mentioned in the commit log of e671cd59d74c. Anyway, with that branch, if we switch to ours() while the inferior is running in the foreground, then we miss forwarding the input to the inferior. See: https://github.com/palves/gdb/blob/d3392b83086b0a541aa31fcff301281da7a73a0e/gdb/inflow.c#L762 Also, if we miss calling ours_for_output(), then we print output to the terminal in raw mode. What I'm saying is that that branch/fix exposes more latent bugs around incorrect ours()/ours_for_output()/inferior() handling, and the branch includes fixes in that direction, e.g.: the "target_terminal::ours_for_output(): received SIGINT" patch. This is not unlike what old comment in remote.c suggests we could do, but for local debugging: static void remote_terminal_inferior (struct target_ops *self) { /* NOTE: At this point we could also register our selves as the recipient of all input. Any characters typed could then be passed on down to the target. */ } > if we can't always > write output after calling ::ours_for_output. Part of me wonders if > we should just switch to using ::ours in all cases.... I'm thinking that Alan's original patch to disable SIGTTOU is correct. When we're in ours_for_output mode, we should be able to write to the terminal, and we can. But, there are a couple functions that raise a SIGTTOU if you write to the controlling terminal while in the background, and your terminal has the TOSTOP attribute set, and tcdrain is one of them. That isn't to say that your patch _isn't_ also correct. We have many latent bugs around this area. Let me take a better look at that one too. I think that even if we get something like your patch in, then Alan's is still correct because we can have places doing try/catch to swallow an error but still print it with exception_print, all while the inferior is running. Of course such spots should call ours_for_output(), but that will run into the tcdrain issue. > > Anyway, I think most of the problems you're seeing should be fixed by > claiming the terminal either permanently (calling ::ours or > ::ours_for_output) or temporarily using > target_terminal::scoped_restore_terminal_state. > > Like I said, I'm not an expert on this code, so maybe I've > misunderstood the problem you're solving. > Thanks, Pedro Alves