From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 8nkfOxmYy2BoMQAAWB0awg (envelope-from ) for ; Thu, 17 Jun 2021 14:44:41 -0400 Received: by simark.ca (Postfix, from userid 112) id E2E071F163; Thu, 17 Jun 2021 14:44:41 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.6 required=5.0 tests=MAILING_LIST_MULTI, RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A6B601E813 for ; Thu, 17 Jun 2021 14:44:40 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 388963844062 for ; Thu, 17 Jun 2021 18:44:40 +0000 (GMT) Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) by sourceware.org (Postfix) with ESMTPS id A5C00385782A for ; Thu, 17 Jun 2021 18:44:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A5C00385782A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f50.google.com with SMTP id h22-20020a05600c3516b02901a826f84095so4242810wmq.5 for ; Thu, 17 Jun 2021 11:44:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=rVB6Vl27OK3LZPNFhmmQOjZ2l35x9JqzUCeIxP0zB5M=; b=rPeLOIuDnamwpA6hfPONR5QqsQ1fFmF7qAEwASg+QEJ1TDczQd7mFlwIjyj6b1iPws 5lDXOizcCRKlJ2qYQGC7Er6EnPwNCP2kdaUiZc+jSoMcCcZZ9cSQE/6rTEB8jDChkRwC J8P/c41pbKluTwLR5+6tuRjkIfC1aTQ8sBye9S72xkWCeDicP0mFpHqz7ZFuQdPkkCX/ xt1U4wR1NK6SSrfWirc9z535C7NhYY56Wetr1J+0rqdfjXo5jOQRNeMvD7AIXeHzNqfU 68UyQU5CTXt3RFvlkcH3HDbJArbYwXapnmLibnfYCLGrKY+H9EWkEhtkwtOeDJODoxlA d7yQ== X-Gm-Message-State: AOAM531DJnFI+2EFNuFVH5//lmPYpYGWKWCTCSmla4lBB6xoN32o2IOG PRNmerIMR92a3UWOGgYo1YYzngAHIMd6SmZY X-Google-Smtp-Source: ABdhPJyLULxX2HESge8z5vXaB33FDQN2HjuuXTL1O4vrO+kdWHytxuE5rpdT2F9COIneiz1I0IJ6NA== X-Received: by 2002:a1c:a484:: with SMTP id n126mr6912888wme.34.1623955467039; Thu, 17 Jun 2021 11:44:27 -0700 (PDT) Received: from ?IPv6:2001:8a0:f932:6a00:46bc:d03b:7b3a:2227? ([2001:8a0:f932:6a00:46bc:d03b:7b3a:2227]) by smtp.gmail.com with ESMTPSA id f14sm5962272wri.16.2021.06.17.11.44.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 17 Jun 2021 11:44:25 -0700 (PDT) Subject: [pushed] Don't call sigtimedwait for scoped_ignore_sigttou From: Pedro Alves To: John Baldwin , gdb-patches@sourceware.org References: <20210615111429.1879286-1-pedro@palves.net> <7f678d4a-b015-655f-f79a-36daccab5062@palves.net> <18c7433b-464d-b274-88a2-5495aab407ba@palves.net> Message-ID: <7f6edc2b-b684-05bc-aa47-400f8c12616f@palves.net> Date: Thu, 17 Jun 2021 19:44:24 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2021-06-17 5:45 p.m., John Baldwin wrote: > On 6/17/21 8:36 AM, Pedro Alves wrote: >> On 2021-06-17 3:38 p.m., Pedro Alves wrote: >>> On 2021-06-15 6:04 p.m., John Baldwin wrote: >> >>> If SIGTTOU ends up raised by some other thread while the main thread has it blocked >>> in scoped_ignore_sigttou, and if it is sent to the whole process, there's a small time window >>> where the sigtimedwait in the main thread can eat the signal.  That still seems better than >>> the current status where the end result can be that we can end up with the signal enabled >>> instead of ignored and then gdb backgrounds itself.  This is of course highly theoretical since I >>> expect that gdb threads run with signals blocked from the get go.  But you never know what >>> Python code does. >>> >> >> Hmm, I just thought of an easy fix for that.  Just don't consume the pending >> signal in SIGTTOU's case.  I wonder how portable is this?  WDYT? >> >> I'll see if I can try this on Solaris. > > I think this is ok.  I suspect that some of the rules around SIGTTOU are in POSIX as I > think job control is in POSIX, so (hopefully) the behavior there is fairly uniform. > I strongly suspect the other BSD's all follow FreeBSD at least, so if Solaris is the > same I'd feel pretty good about this behavior being consistent. > I tested this on the Solaris 11.3 SPARC machine on the compile farm (gcc211). I did not get any spurious SIGTTOU so it must mean the signal wasn't sent at all there either. I tried to test on AIX (gcc119 on the compile farm), but the build is broken there due to PR 27123, and even with an evil hack to get past that, for some reason I couldn't get GDB to build against the GMP on the system. So I gave up... I did notice that the "maint selftest scoped_ignore_sigpipe" test fails on Solaris, bitten by BSD signal semantics, which I forgot to handle... I'll fix that in a bit. Here's the patch that I merged. Thanks for the help John. >From 336b30e58ae98fe66862ab8480d3f7bb885fef23 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 17 Jun 2021 16:23:03 +0100 Subject: [PATCH] Don't call sigtimedwait for scoped_ignore_sigttou Because SIGTTOU is sent to the whole process instead of to a specific thread, consuming a pending SIGTTOU in the destructor of scoped_ignore_sigttou could consume a SIGTTOU signal raised due to actions done by some other thread. Simply avoid sigtimedwait in scoped_ignore_sigttou, thus plugging the race. This works because we know that when the thread writes to the terminal and the signal is blocked, the kernel does not raise the signal at all. Tested on GNU/Linux, Solaris 11 and FreeBSD. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * scoped_ignore_signal.h (scoped_ignore_signal): Add ConsumePending template parameter. (scoped_ignore_signal::~scoped_ignore_signal): Skip calling sigtimedwait if ConsumePending is false. (scoped_ignore_sigpipe): Initialize with ConsumePending=true. * scoped_ignore_sigttou.h (scoped_ignore_sigttou) : Initialize with ConsumePending=false. Change-Id: I92f754dbc45c45819dce2ce68b8c067d8d5c61b1 --- gdb/ChangeLog | 10 ++++++++++ gdbsupport/scoped_ignore_signal.h | 18 +++++++++++++----- gdbsupport/scoped_ignore_sigttou.h | 2 +- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index e4e58173ee6..c70f6ef5329 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2021-06-17 Pedro Alves + + * scoped_ignore_signal.h (scoped_ignore_signal): Add + ConsumePending template parameter. + (scoped_ignore_signal::~scoped_ignore_signal): Skip calling + sigtimedwait if ConsumePending is false. + (scoped_ignore_sigpipe): Initialize with ConsumePending=true. + * scoped_ignore_sigttou.h (scoped_ignore_sigttou) + : Initialize with ConsumePending=false. + 2021-06-17 Pedro Alves * Makefile.in (SELFTESTS_SRCS): Add diff --git a/gdbsupport/scoped_ignore_signal.h b/gdbsupport/scoped_ignore_signal.h index 55a921cb332..a14c96779bf 100644 --- a/gdbsupport/scoped_ignore_signal.h +++ b/gdbsupport/scoped_ignore_signal.h @@ -25,9 +25,16 @@ /* RAII class used to ignore a signal in a scope. If sigprocmask is supported, then the signal is only ignored by the calling thread. Otherwise, the signal disposition is set to SIG_IGN, which affects - the whole process. */ - -template + the whole process. If ConsumePending is true, the destructor + consumes a pending Sig. SIGPIPE for example is queued on the + thread even if blocked at the time the pipe is written to. SIGTTOU + OTOH is not raised at all if the thread writing to the terminal has + it blocked. Because SIGTTOU is sent to the whole process instead + of to a specific thread, consuming a pending SIGTTOU in the + destructor could consume a signal raised due to actions done by + some other thread. */ + +template class scoped_ignore_signal { public: @@ -58,7 +65,8 @@ class scoped_ignore_signal /* If we got a pending Sig signal, consume it before unblocking. */ - sigtimedwait (&set, nullptr, &zero_timeout); + if (ConsumePending) + sigtimedwait (&set, nullptr, &zero_timeout); sigprocmask (SIG_UNBLOCK, &set, nullptr); } @@ -89,7 +97,7 @@ struct scoped_ignore_signal_nop }; #ifdef SIGPIPE -using scoped_ignore_sigpipe = scoped_ignore_signal; +using scoped_ignore_sigpipe = scoped_ignore_signal; #else using scoped_ignore_sigpipe = scoped_ignore_signal_nop; #endif diff --git a/gdbsupport/scoped_ignore_sigttou.h b/gdbsupport/scoped_ignore_sigttou.h index 1fc8f80d7fd..5695c5db905 100644 --- a/gdbsupport/scoped_ignore_sigttou.h +++ b/gdbsupport/scoped_ignore_sigttou.h @@ -75,7 +75,7 @@ class scoped_ignore_sigttou DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigttou); private: - lazy_init> m_ignore_signal; + lazy_init> m_ignore_signal; }; #else base-commit: e013d20dc73b40d4b70c4a366c9adc619503e66b -- 2.26.2