From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67292 invoked by alias); 3 Oct 2019 11:55:48 -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 67284 invoked by uid 89); 3 Oct 2019 11:55:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.1 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=arrive, HX-Received:c84f, deciding X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Oct 2019 11:55:45 +0000 Received: by mail-wm1-f66.google.com with SMTP id i16so2050310wmd.3 for ; Thu, 03 Oct 2019 04:55:45 -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=KsFcq+owRfN9oIJAm6luDs+G1UYHf0bGZE0S0DeJzmc=; b=a/TtnuGP2XAn4fCHgem7Uu/x1pOD5SnPOy1+o9fcz5/fG4984UY+Q00Ajx1NM7Os+h nMyPttgOiPvLwsbdi7YQoGeP3N88hVBWluB9dYzIm1x7TqSvpWfXWJ5rGHGt+uQ4rZg5 KIFsv/Oyucdlt5BbJWfam+h3lkh3B+I4wBDrx8NdBiBp/1u7tQVwnStriYDwm0opCEiX kx51b8uICU0bqHeHhlLyYyiTS3E9Jx/Bl2Kpnn5r99JW0yC+n4d0HBBVgBQkeGzOExMG EOuyb6KsmQFrv/OCyTkcP0Z6b55rB0aPuytr3xR70wKFwQ52zM3wCEBhkCDkbb0d0YlR CbIA== Return-Path: Received: from localhost (host86-128-12-122.range86-128.btcentralplus.com. [86.128.12.122]) by smtp.gmail.com with ESMTPSA id x5sm1863544wrt.75.2019.10.03.04.55.42 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 03 Oct 2019 04:55:42 -0700 (PDT) Date: Thu, 03 Oct 2019 11:55: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: <20191003115542.GG4962@embecosm.com> References: <20190910173757.3374-1-andrew.burgess@embecosm.com> <20190926230613.GW4962@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190926230613.GW4962@embecosm.com> X-Fortune: Losing your drivers' license is just God's way of saying "BOOGA, BOOGA!" 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/msg00106.txt.bz2 I plan to merge this patch tomorrow unless anyone complains before then. Thanks, Andrew * Andrew Burgess [2019-09-27 00:06:13 +0100]: > 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 > >