From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128371 invoked by alias); 26 Sep 2019 23:06:21 -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 127716 invoked by uid 89); 26 Sep 2019 23:06:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=lp, HX-Received:sk:n22mr50 X-HELO: mail-wm1-f68.google.com Received: from mail-wm1-f68.google.com (HELO mail-wm1-f68.google.com) (209.85.128.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Sep 2019 23:06:18 +0000 Received: by mail-wm1-f68.google.com with SMTP id r19so4511565wmh.2 for ; Thu, 26 Sep 2019 16:06:18 -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=T8EHkgsx69v52aCOnUPbQ+xfkS1IUGK17pCtuXLYdkw=; b=VdiTLQZhpPwQRqx5lgfZSrqLoimw7Q1V0VsabvSg/pTIm0JAFdMOBMoO+PEU8F9PfD SWqRstDyNwIrZWoFhX2gEfGD4vmpZrNAltRJxdGLlbDJJRzcTgOjt5POB22M2QdhgjmU 0lKwekTWZOCh1HWwUaucB50KP29FnPC1Jlc6w2jsoVn+sivbL2l2FBkJbLdtlgFj4FE/ XFuvisdiGMk94zAuWfj/2qsMV43/9dil/QaCv5crnzVCTfDKGPY7ZUIAbgTKO4db7qMy ymKqulxF6MjaMWjq1ZPaAvogFttu15QQw19SWJnaXPR4lO6unsRGkdmCet9iuQ0fiWXP WqjA== Return-Path: Received: from localhost (host86-128-12-122.range86-128.btcentralplus.com. [86.128.12.122]) by smtp.gmail.com with ESMTPSA id i1sm8573859wmb.19.2019.09.26.16.06.15 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 26 Sep 2019 16:06:15 -0700 (PDT) Date: Thu, 26 Sep 2019 23:06:00 -0000 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Richard Bunt Subject: Re: [PATCH] gdb: Don't ignore all SIGSTOP when the signal handler is set to pass Message-ID: <20190926230613.GW4962@embecosm.com> References: <20190910173757.3374-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190910173757.3374-1-andrew.burgess@embecosm.com> X-Fortune: Quark! Quark! Beware the quantum duck! 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-09/txt/msg00542.txt.bz2 Ping! Thanks, Andrew * Andrew Burgess [2019-09-10 18:37:57 +0100]: > 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. > > gdb/ChangeLog: > > * linux-nat.c (linux_nat_filter_event): Don't ignore SIGSTOP if we > have just sent the thread a SIGSTOP and are waiting for it to > arrive. > > gdb/testsuite/ChangeLog: > > * gdb.threads/stop-with-handle.c: New file. > * gdb.threads/stop-with-handle.exp: New file. > --- > gdb/ChangeLog | 6 +++ > gdb/linux-nat.c | 5 +- > gdb/testsuite/ChangeLog | 5 ++ > gdb/testsuite/gdb.threads/stop-with-handle.c | 70 ++++++++++++++++++++++++++ > gdb/testsuite/gdb.threads/stop-with-handle.exp | 52 +++++++++++++++++++ > 5 files changed, 137 insertions(+), 1 deletion(-) > create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.c > create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.exp > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index 945c19f6668..5cf5c57c043 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -3150,9 +3150,12 @@ linux_nat_filter_event (int lwpid, int status) > > /* When using hardware single-step, we need to report every signal. > Otherwise, signals in pass_mask may be short-circuited > - except signals that might be caused by a breakpoint. */ > + except signals that might be caused by a breakpoint, or SIGSTOP > + if we sent the SIGSTOP and are waiting for it to arrive. */ > if (!lp->step > && WSTOPSIG (status) && sigismember (&pass_mask, WSTOPSIG (status)) > + && (WSTOPSIG (status) != SIGSTOP > + || !find_thread_ptid (lp->ptid)->stop_requested) > && !linux_wstatus_maybe_breakpoint (status)) > { > linux_resume_one_lwp (lp, lp->step, signo); > diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.c b/gdb/testsuite/gdb.threads/stop-with-handle.c > new file mode 100644 > index 00000000000..da1b3d45498 > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/stop-with-handle.c > @@ -0,0 +1,70 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2019 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include > +#include > +#include > + > +/* A place to record the thread. */ > +pthread_t the_thread; > + > +/* The worker thread just spins forever. */ > +void* > +thread_worker (void *payload) > +{ > + while (1) > + { > + sleep (1); > + } > + > + return NULL; > +} > + > +/* Create a worker thread. */ > +int > +spawn_thread () > +{ > + if (pthread_create (&the_thread, NULL, thread_worker, NULL)) > + { > + fprintf (stderr, "Unable to create thread.\n"); > + return 0; > + } > + return 1; > +} > + > +/* A place for GDB to place a breakpoint. */ > +void > +breakpt () > +{ > + asm ("" ::: "memory"); > +} > + > +/* Create a worker thread that just spins forever, then enter a loop > + periodically calling the BREAKPT function. */ > +int main() { > + spawn_thread (); > + > + while (1) > + { > + sleep (1); > + > + breakpt (); > + } > + > + return 0; > +} > + > diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.exp b/gdb/testsuite/gdb.threads/stop-with-handle.exp > new file mode 100644 > index 00000000000..bab58a6129d > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/stop-with-handle.exp > @@ -0,0 +1,52 @@ > +# Copyright 2019 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# This test covers a case where SIGSTOP has been configured to be > +# passed to the target with GDB's 'handle' command, and then a > +# multi-threaded inferior encounters an event that causes all theads > +# to be stopped. > +# > +# The problem that (used) to exist was that GDB would see the SIGSTOP, > +# but decide to ignore the signal based on the handle table. > + > +standard_testfile > + > +if {[prepare_for_testing "failed to prepare" \ > + "${testfile}" "${srcfile}" {debug pthreads}]} { > + return -1 > +} > + > +if ![runto_main] then { > + fail "can't run to main" > + return 0 > +} > + > +# Have SIGSTOP sent to the inferior. > +gdb_test "handle SIGSTOP nostop noprint pass" \ > + [multi_line \ > + "Signal\[ \t\]+Stop\[ \t\]+Print\[ \t\]+Pass to program\[ \t\]+Description" \ > + "SIGSTOP\[ \t\]+No\[ \t\]+No\[ \t\]+Yes\[ \t\]+Stopped \\(signal\\)"] > + > +# Create a breakpoint, and when we hit it automatically finish the > +# current frame. > +gdb_breakpoint breakpt > + > +# When the bug triggers this continue never completes. GDB hits the > +# breakpoint in thread 1, and then tries to stop the second thread by > +# sending it SIGSTOP. GDB sees the SIGSTOP arrive in thread 2 but > +# incorrect decides to pass the SIGSTOP to the thread rather than > +# bringing the thread to a stop. > +gdb_continue_to_breakpoint breakpt > + > -- > 2.14.5 >