From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91675 invoked by alias); 16 May 2019 18:06:46 -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 91667 invoked by uid 89); 16 May 2019 18:06:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=claiming, solving, permanently 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, 16 May 2019 18:06:44 +0000 Received: by mail-wr1-f65.google.com with SMTP id h4so4404157wre.7 for ; Thu, 16 May 2019 11:06:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=N0NJ785yJNQ233L56hP9rP9Wp+DpeLOD6jDl3wgu85k=; b=dRtEyOWukHYlrLnhEpKPGsm5tcgV8XTlx/51ZAN/vz37yK8tPpN26W4iVkGCdTRn/p tuhfeYv/M3xqsVtvzbog+LmqPVaNuQzmS7xvFRrtEp2OcEVx0ed7f27rTa8e1aJxE+XQ aWXEurxGfQ/QsIqqiXa9Ac2pV+zLLzfPElGNk9ij5aK9oPAJgBEgfZ0Z9jOWAPdDeHVh TBhkzXBL6wJmeVVrF14HL5KDqfGdE6hxyBH7is8IMg4M0RuNX8tpyPoFQvLmGXeDDGl4 BxXI1Ix/cQDgSAzD73DM5Au2X3HlwKeUiSOk/zcC6rsU6TcpJVEK/xa7sBLBk0bMUVKu 3Kgw== Return-Path: Received: from localhost (host86-180-62-212.range86-180.btcentralplus.com. [86.180.62.212]) by smtp.gmail.com with ESMTPSA id v17sm4119995wmc.30.2019.05.16.11.06.41 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 16 May 2019 11:06:41 -0700 (PDT) Date: Thu, 16 May 2019 18:06:00 -0000 From: Andrew Burgess To: Alan Hayward Cc: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH] Supress SIGTTOU when handling errors Message-ID: <20190516180640.GS2568@embecosm.com> References: <20190516155150.71826-1-alan.hayward@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190516155150.71826-1-alan.hayward@arm.com> X-Fortune: Some bachelors want a meaningful overnight relationship. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg00383.txt.bz2 * Alan Hayward [2019-05-16 15:51:53 +0000]: > [I've seen this on and off over many months on AArch64 and Arm, and am > assuming it isn't the intended behaviour. Not sure if this should be at > tcdrain or it should be done at a higher level - eg in the terminal > handling code] > > Calls to error () can cause SIGTTOU to send gdb to the background. > > For example, on an Arm build: > (gdb) b main > Breakpoint 1 at 0x10774: file /build/gdb/testsuite/../../../src/binutils-gdb/gdb/testsuite/gdb.base/watchpoint.c, line 174. > (gdb) r > Starting program: /build/gdb/testsuite/outputs/gdb.base/watchpoint/watchpoint > > [1]+ Stopped ../gdb ./outputs/gdb.base/watchpoint/watchpoint > localhost$ fg > ../gdb ./outputs/gdb.base/watchpoint/watchpoint > Cannot parse expression `.L1199 4@r4'. > warning: Probes-based dynamic linker interface failed. > Reverting to original interface. > > The SIGTTOU is raised whilst inside a syscall during the call to tcdrain. > Fix is to use scoped_ignore_sigttou to ensure SIGTTOU is blocked. > > In addition fix include comments - job_control is not included via > terminal.h Maybe someone else knows this better, but this feels like the wrong solution to me. 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, 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.... 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, Andrew > > gdb/ChangeLog: > > 2019-05-15 Alan Hayward > > * event-top.c: Remove include comment. > * inflow.c (class scoped_ignore_sigttou): Move from here... > * inflow.h (class scoped_ignore_sigttou): ...to here. > * ser-unix.c (hardwire_drain_output): Block SIGTTOU during drain. > * top.c: Remove include comment. > --- > gdb/event-top.c | 2 +- > gdb/inflow.c | 29 ----------------------------- > gdb/inflow.h | 31 +++++++++++++++++++++++++++++++ > gdb/ser-unix.c | 4 ++++ > gdb/top.c | 2 +- > 5 files changed, 37 insertions(+), 31 deletions(-) > > diff --git a/gdb/event-top.c b/gdb/event-top.c > index 3ccf136ff1..93b7d2d28b 100644 > --- a/gdb/event-top.c > +++ b/gdb/event-top.c > @@ -24,7 +24,7 @@ > #include "inferior.h" > #include "infrun.h" > #include "target.h" > -#include "terminal.h" /* for job_control */ > +#include "terminal.h" > #include "event-loop.h" > #include "event-top.h" > #include "interps.h" > diff --git a/gdb/inflow.c b/gdb/inflow.c > index 339b55c0bc..eba7a931f4 100644 > --- a/gdb/inflow.c > +++ b/gdb/inflow.c > @@ -106,35 +106,6 @@ static serial_ttystate initial_gdb_ttystate; > > static struct terminal_info *get_inflow_inferior_data (struct inferior *); > > -/* RAII class used to ignore SIGTTOU in a scope. */ > - > -class scoped_ignore_sigttou > -{ > -public: > - scoped_ignore_sigttou () > - { > -#ifdef SIGTTOU > - if (job_control) > - m_osigttou = signal (SIGTTOU, SIG_IGN); > -#endif > - } > - > - ~scoped_ignore_sigttou () > - { > -#ifdef SIGTTOU > - if (job_control) > - signal (SIGTTOU, m_osigttou); > -#endif > - } > - > - DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigttou); > - > -private: > -#ifdef SIGTTOU > - sighandler_t m_osigttou = NULL; > -#endif > -}; > - > /* While the inferior is running, we want SIGINT and SIGQUIT to go to the > inferior only. If we have job control, that takes care of it. If not, > we save our handlers in these two variables and set SIGINT and SIGQUIT > diff --git a/gdb/inflow.h b/gdb/inflow.h > index c32aa14433..5dd5c37bd2 100644 > --- a/gdb/inflow.h > +++ b/gdb/inflow.h > @@ -21,5 +21,36 @@ > #define INFLOW_H > > #include > +#include > +#include "common/job-control.h" > + > +/* RAII class used to ignore SIGTTOU in a scope. */ > + > +class scoped_ignore_sigttou > +{ > +public: > + scoped_ignore_sigttou () > + { > +#ifdef SIGTTOU > + if (job_control) > + m_osigttou = signal (SIGTTOU, SIG_IGN); > +#endif > + } > + > + ~scoped_ignore_sigttou () > + { > +#ifdef SIGTTOU > + if (job_control) > + signal (SIGTTOU, m_osigttou); > +#endif > + } > + > + DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigttou); > + > +private: > +#ifdef SIGTTOU > + sighandler_t m_osigttou = NULL; > +#endif > +}; > > #endif /* inflow.h */ > diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c > index 5a9965bf74..3492619f2d 100644 > --- a/gdb/ser-unix.c > +++ b/gdb/ser-unix.c > @@ -32,6 +32,7 @@ > #include "gdbcmd.h" > #include "common/filestuff.h" > #include > +#include "inflow.h" > > struct hardwire_ttystate > { > @@ -164,6 +165,9 @@ hardwire_print_tty_state (struct serial *scb, > static int > hardwire_drain_output (struct serial *scb) > { > + /* Ignore SIGTTOU which may occur during the drain. */ > + scoped_ignore_sigttou ignore_sigttou; > + > return tcdrain (scb->fd); > } > > diff --git a/gdb/top.c b/gdb/top.c > index bacd684dba..1e17ebee87 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -34,7 +34,7 @@ > #include "expression.h" > #include "value.h" > #include "language.h" > -#include "terminal.h" /* For job_control. */ > +#include "terminal.h" > #include "common/job-control.h" > #include "annotate.h" > #include "completer.h" > -- > 2.20.1 (Apple Git-117) >