From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117617 invoked by alias); 3 Oct 2019 15:14:40 -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 117601 invoked by uid 89); 3 Oct 2019 15:14:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= 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, 03 Oct 2019 15:14:38 +0000 Received: by mail-wr1-f65.google.com with SMTP id b9so3261209wrs.0 for ; Thu, 03 Oct 2019 08:14:38 -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=Q71v1/DO62cHBElLSCMW9l3Pme0DuirKOguwcbLFwLw=; b=GiAhEfBqvlhCbYx7mylTamrE5ZZMHUjREF/f7O7QKocDsY1Sxiba8SRCs9AyOlvsv/ 77K6qUP2jsqsAMyA+aTlTyrxCYhoAQ9QK1YE0JrRO5WpOEj/EiGj4TIOj8OazSVdmGvF XOIaW+DVuHusqYfGv24xkVlT9l3Jlf69nhLOHZDxGtPdZAWGBL7BTToOYTYLCOvj1ieH 8CU3DuH0Mc1UysogjTjJQa0YzYLa2n5WE6yUkGub9fE2LS+YNnvdVeCoEKOoKLTIwPYH GGABoGT5zZ8FXeb1YC9OwyHJstjqLxNlaftt+PlECASjU+H/5teBlDFEtmWyijj+Yhil 58qA== Return-Path: Received: from localhost (host86-128-12-122.range86-128.btcentralplus.com. [86.128.12.122]) by smtp.gmail.com with ESMTPSA id f9sm2246896wre.74.2019.10.03.08.14.35 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 03 Oct 2019 08:14:35 -0700 (PDT) Date: Thu, 03 Oct 2019 15:14:00 -0000 From: Andrew Burgess To: Pedro Alves Cc: gdb-patches@sourceware.org, Richard Bunt Subject: Re: [PATCH] gdb: Don't ignore all SIGSTOP when the signal handler is set to pass Message-ID: <20191003151434.GI4962@embecosm.com> References: <20190910173757.3374-1-andrew.burgess@embecosm.com> <87066547-f991-9597-0ead-7a52b05ee2c3@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87066547-f991-9597-0ead-7a52b05ee2c3@redhat.com> X-Fortune: descramble code needed from software company 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-10/txt/msg00115.txt.bz2 * Pedro Alves [2019-10-03 13:59:14 +0100]: > On 9/10/19 6:37 PM, Andrew Burgess wrote: > > It was observed that in a multi-threaded application on GNU/Linux, > > that if the user has set the SIGSTOP to be pass (using GDB's handle > > command) then the inferior would hang upon hitting a breakpoint. > > > > What happens is that when a thread hits the breakpoint GDB tries to > > stop all of the other threads by sending them a SIGSTOP and setting > > the stop_requested flag in the target_ops structure - this can be seen > > in infrun.c:stop_all_threads. > > > > GDB then waits for all of the other threads to stop. > > > > When the SIGSTOP event arrives we eventually end up in > > linux-nat.c:linux_nat_filter_event, which has the job of deciding if > > the event we're looking at (the SIGSTOP arriving in this case) is > > something that should be reported back to the core of GDB. > > > > One of the final actions of this function is to check if we stopped > > due to a signal, and if we did, and the signal has been set to 'pass' > > by the user then we ignore the event and resume the thread. > > > > This code already has some conditions in place that mean the event is > > reported to GDB even if the signal is in the set of signals to be > > passed to the inferior. > > > > In this commit I extend this condition to such that: > > > > If the signal is a SIGSTOP, and the thread's stop_requested flag is > > set (indicating we're waiting for the thread to stop with a SIGSTOP) > > then we should report this SIGSTOP to GDB and not pass it to the > > inferior. > > > > With this change in place the test now passes. Regression tested on > > x86-64 GNU/Linux with no regressions. > > Indeed, makes sense. > > I checked that gdbserver passes the new test too. (It knows to always > report an event when gdb tries to stop a thread with 'vCont;t'.) > > > +/* The worker thread just spins forever. */ > > +void* > > +thread_worker (void *payload) > > +{ > > + while (1) > > + { > > + sleep (1); > > + } > > + > > + return NULL; > > +} > > + > > > +/* A place for GDB to place a breakpoint. */ > > +void > > +breakpt () > > +{ > > + asm ("" ::: "memory"); > > I wonder whether this barrier is a good idea, given > that it limits portability to gcc and clang. > > I checked that both clang and gcc still manage to > eliminate the function with "-flto -O2", the barrier > didn't help then. '__attribute__((used))' instead > of the barrier works though. > > > +} > > > + > > +/* Create a worker thread that just spins forever, then enter a loop > > + periodically calling the BREAKPT function. */ > > +int main() { > > GNU formatting. > > > + spawn_thread (); > > + > > + while (1) > > + { > > + sleep (1); > > + > > + breakpt (); > > + } > > + > > + return 0; > > +} > > It's better to avoid that the testcase really runs forever, in case GDB > crashes and the system doesn't kill the process. Easily fixed with > an "alarm" call at the top of main. > > LGTM otherwise. > > BTW, FYI, I saw this: > > $ git am /tmp/sigstop.mbox > Applying: gdb: Don't ignore all SIGSTOP when the signal handler is set to pass > .git/rebase-apply/patch:104: new blank line at EOF. > + > .git/rebase-apply/patch:162: new blank line at EOF. > + > warning: 2 lines add whitespace errors. > I pushed this patch with the fixes you identified above. Thanks, Andrew