From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69882 invoked by alias); 3 Oct 2019 12:59:22 -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 69873 invoked by uid 89); 3 Oct 2019 12:59:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham version=3.3.1 spammy= X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Oct 2019 12:59:20 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1570107558; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gPDNxhdzOdsLwu7fer86YfOIdvij5r23IkiLAO6GnwA=; b=QzJC3F/WJMcjh4lvU/zkvdtXmoJe9mIgo/2yPLr2JGJmchnaQx8RYfwDA7KuSLJPMBRhZ6 zqC9/0IvJuz2C5FwIKBNjr1hbHlfMBifJpO3MZATP+bEKm0uCa5pYAel24xQMFt0pGjwh2 xk6L6E8x1qT/Z1orDNTOMCm8w/8dMtA= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-61-bABla6WINeahzHgF2FM1Ig-1; Thu, 03 Oct 2019 08:59:17 -0400 Received: by mail-wm1-f69.google.com with SMTP id g67so622822wmg.4 for ; Thu, 03 Oct 2019 05:59:16 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id t83sm3397381wmt.18.2019.10.03.05.59.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 03 Oct 2019 05:59:15 -0700 (PDT) Subject: Re: [PATCH] gdb: Don't ignore all SIGSTOP when the signal handler is set to pass To: Andrew Burgess , gdb-patches@sourceware.org References: <20190910173757.3374-1-andrew.burgess@embecosm.com> Cc: Richard Bunt From: Pedro Alves Message-ID: <87066547-f991-9597-0ead-7a52b05ee2c3@redhat.com> Date: Thu, 03 Oct 2019 12:59: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: <20190910173757.3374-1-andrew.burgess@embecosm.com> X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2019-10/txt/msg00109.txt.bz2 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. >=20 > 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. >=20 > GDB then waits for all of the other threads to stop. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > In this commit I extend this condition to such that: >=20 > 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. >=20 > With this change in place the test now passes. Regression tested on > x86-64 GNU/Linux with no regressions. Indeed, makes sense.=20=20 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 p= ass .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. Thanks, Pedro Alves