From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x733.google.com (mail-qk1-x733.google.com [IPv6:2607:f8b0:4864:20::733]) by sourceware.org (Postfix) with ESMTPS id B8724396E476 for ; Wed, 28 Oct 2020 11:53:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B8724396E476 Received: by mail-qk1-x733.google.com with SMTP id q199so4146845qke.10 for ; Wed, 28 Oct 2020 04:53:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:autocrypt:subject :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=h6vcidBlLiV3egaPJS9XRHiV+mRtI2F4n9eNIPawKFM=; b=FTH7RcBSTKmxW7/s52DWrX2p+7+zRzJxTFPZ73Acc4idgkB5J2umodnphfJlAWP4UA uDJTZ0B2WViZFPz+/vv30UJuOxD2oiHK6fDJ3KQda9XNdBVZvlAQAYCXCkV6bm/9nq0d QrMfCD22ShT8dpMTqBBHVpZDWz5QjrwLTsnbB8ypnbV3W17c40XCRuy0Lb6lqtF2zDFo hOxivfG0vKj6mkg1JsQ0e1yYFMJiPxU5ZlCkvItylJJOgcB1mxqx1CRal+OSeUSZ9GVq cWH+Ll55azwukayScz/EeIDe1bIxXFJBekPD2eZehKefbmNilqYxi5MGhx/p0OIBhb5B ERFQ== X-Gm-Message-State: AOAM533f9bDWHrmILSdTGpyfgw9pLc8caU9nSmGVNCN7v3tThvDCnvuq wB7k1UwIBydyHeaJbetfut9PWTETj+TyZA== X-Google-Smtp-Source: ABdhPJx0mLm2AXkqvqo6K99J+fxTTRETn3tNzkbeDiijM4Z9et91UibxbmjRs63RZgRDGrIdqGf6Uw== X-Received: by 2002:a37:9b4a:: with SMTP id d71mr1736952qke.142.1603885997173; Wed, 28 Oct 2020 04:53:17 -0700 (PDT) Received: from [192.168.1.4] ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id e188sm2698615qkf.128.2020.10.28.04.53.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 28 Oct 2020 04:53:16 -0700 (PDT) To: Tadeus Prastowo <0x66726565@gmail.com> Cc: "libc-help@sourceware.org" , Florian Weimer References: <25b5791b-5368-7a78-f80d-5ceb2b618d72@linaro.org> From: Adhemerval Zanella Autocrypt: addr=adhemerval.zanella@linaro.org; prefer-encrypt=mutual; keydata= mQINBFcVGkoBEADiQU2x/cBBmAVf5C2d1xgz6zCnlCefbqaflUBw4hB/bEME40QsrVzWZ5Nq 8kxkEczZzAOKkkvv4pRVLlLn/zDtFXhlcvQRJ3yFMGqzBjofucOrmdYkOGo0uCaoJKPT186L NWp53SACXguFJpnw4ODI64ziInzXQs/rUJqrFoVIlrPDmNv/LUv1OVPKz20ETjgfpg8MNwG6 iMizMefCl+RbtXbIEZ3TE/IaDT/jcOirjv96lBKrc/pAL0h/O71Kwbbp43fimW80GhjiaN2y WGByepnkAVP7FyNarhdDpJhoDmUk9yfwNuIuESaCQtfd3vgKKuo6grcKZ8bHy7IXX1XJj2X/ BgRVhVgMHAnDPFIkXtP+SiarkUaLjGzCz7XkUn4XAGDskBNfbizFqYUQCaL2FdbW3DeZqNIa nSzKAZK7Dm9+0VVSRZXP89w71Y7JUV56xL/PlOE+YKKFdEw+gQjQi0e+DZILAtFjJLoCrkEX w4LluMhYX/X8XP6/C3xW0yOZhvHYyn72sV4yJ1uyc/qz3OY32CRy+bwPzAMAkhdwcORA3JPb kPTlimhQqVgvca8m+MQ/JFZ6D+K7QPyvEv7bQ7M+IzFmTkOCwCJ3xqOD6GjX3aphk8Sr0dq3 4Awlf5xFDAG8dn8Uuutb7naGBd/fEv6t8dfkNyzj6yvc4jpVxwARAQABtElBZGhlbWVydmFs IFphbmVsbGEgTmV0dG8gKExpbmFybyBWUE4gS2V5KSA8YWRoZW1lcnZhbC56YW5lbGxhQGxp bmFyby5vcmc+iQI3BBMBCAAhBQJXFRpKAhsDBQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJ EKqx7BSnlIjv0e8P/1YOYoNkvJ+AJcNUaM5a2SA9oAKjSJ/M/EN4Id5Ow41ZJS4lUA0apSXW NjQg3VeVc2RiHab2LIB4MxdJhaWTuzfLkYnBeoy4u6njYcaoSwf3g9dSsvsl3mhtuzm6aXFH /Qsauav77enJh99tI4T+58rp0EuLhDsQbnBic/ukYNv7sQV8dy9KxA54yLnYUFqH6pfH8Lly sTVAMyi5Fg5O5/hVV+Z0Kpr+ZocC1YFJkTsNLAW5EIYSP9ftniqaVsim7MNmodv/zqK0IyDB GLLH1kjhvb5+6ySGlWbMTomt/or/uvMgulz0bRS+LUyOmlfXDdT+t38VPKBBVwFMarNuREU2 69M3a3jdTfScboDd2ck1u7l+QbaGoHZQ8ZNUrzgObltjohiIsazqkgYDQzXIMrD9H19E+8fw kCNUlXxjEgH/Kg8DlpoYJXSJCX0fjMWfXywL6ZXc2xyG/hbl5hvsLNmqDpLpc1CfKcA0BkK+ k8R57fr91mTCppSwwKJYO9T+8J+o4ho/CJnK/jBy1pWKMYJPvvrpdBCWq3MfzVpXYdahRKHI ypk8m4QlRlbOXWJ3TDd/SKNfSSrWgwRSg7XCjSlR7PNzNFXTULLB34sZhjrN6Q8NQZsZnMNs TX8nlGOVrKolnQPjKCLwCyu8PhllU8OwbSMKskcD1PSkG6h3r0AquQINBFcVGkoBEACgAdbR Ck+fsfOVwT8zowMiL3l9a2DP3Eeak23ifdZG+8Avb/SImpv0UMSbRfnw/N81IWwlbjkjbGTu oT37iZHLRwYUFmA8fZX0wNDNKQUUTjN6XalJmvhdz9l71H3WnE0wneEM5ahu5V1L1utUWTyh VUwzX1lwJeV3vyrNgI1kYOaeuNVvq7npNR6t6XxEpqPsNc6O77I12XELic2+36YibyqlTJIQ V1SZEbIy26AbC2zH9WqaKyGyQnr/IPbTJ2Lv0dM3RaXoVf+CeK7gB2B+w1hZummD21c1Laua +VIMPCUQ+EM8W9EtX+0iJXxI+wsztLT6vltQcm+5Q7tY+HFUucizJkAOAz98YFucwKefbkTp eKvCfCwiM1bGatZEFFKIlvJ2QNMQNiUrqJBlW9nZp/k7pbG3oStOjvawD9ZbP9e0fnlWJIsj 6c7pX354Yi7kxIk/6gREidHLLqEb/otuwt1aoMPg97iUgDV5mlNef77lWE8vxmlY0FBWIXuZ yv0XYxf1WF6dRizwFFbxvUZzIJp3spAao7jLsQj1DbD2s5+S1BW09A0mI/1DjB6EhNN+4bDB SJCOv/ReK3tFJXuj/HbyDrOdoMt8aIFbe7YFLEExHpSk+HgN05Lg5TyTro8oW7TSMTk+8a5M kzaH4UGXTTBDP/g5cfL3RFPl79ubXwARAQABiQIfBBgBCAAJBQJXFRpKAhsMAAoJEKqx7BSn lIjvI/8P/jg0jl4Tbvg3B5kT6PxJOXHYu9OoyaHLcay6Cd+ZrOd1VQQCbOcgLFbf4Yr+rE9l mYsY67AUgq2QKmVVbn9pjvGsEaz8UmfDnz5epUhDxC6yRRvY4hreMXZhPZ1pbMa6A0a/WOSt AgFj5V6Z4dXGTM/lNManr0HjXxbUYv2WfbNt3/07Db9T+GZkpUotC6iknsTA4rJi6u2ls0W9 1UIvW4o01vb4nZRCj4rni0g6eWoQCGoVDk/xFfy7ZliR5B+3Z3EWRJcQskip/QAHjbLa3pml xAZ484fVxgeESOoaeC9TiBIp0NfH8akWOI0HpBCiBD5xaCTvR7ujUWMvhsX2n881r/hNlR9g fcE6q00qHSPAEgGr1bnFv74/1vbKtjeXLCcRKk3Ulw0bY1OoDxWQr86T2fZGJ/HIZuVVBf3+ gaYJF92GXFynHnea14nFFuFgOni0Mi1zDxYH/8yGGBXvo14KWd8JOW0NJPaCDFJkdS5hu0VY 7vJwKcyHJGxsCLU+Et0mryX8qZwqibJIzu7kUJQdQDljbRPDFd/xmGUFCQiQAncSilYOcxNU EMVCXPAQTteqkvA+gNqSaK1NM9tY0eQ4iJpo+aoX8HAcn4sZzt2pfUB9vQMTBJ2d4+m/qO6+ cFTAceXmIoFsN8+gFN3i8Is3u12u8xGudcBPvpoy4OoG Subject: Re: raise() marked __leaf__ is not C-compliant? Message-ID: <518e2a7c-74a3-d5dc-52c3-3fdddd5f7fa4@linaro.org> Date: Wed, 28 Oct 2020 08:53:13 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URI_DOTEDU autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-help@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-help mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Oct 2020 11:53:19 -0000 On 28/10/2020 04:33, Tadeus Prastowo wrote: > Allow me to fix some mistakes in my previous e-mails: > > On Wed, Oct 28, 2020 at 7:13 AM Tadeus Prastowo <0x66726565@gmail.com> wrote: >> >> Sorry that I closed this thread too soon and reopen it again now after >> re-reading GCC documentation because the GCC documentation thinks that >> raise() should not be marked __leaf__. >> >> On Wed, Oct 28, 2020 at 6:47 AM Tadeus Prastowo <0x66726565@gmail.com> wrote: >>> >>> On Tue, Oct 27, 2020 at 7:50 PM Adhemerval Zanella >>> wrote: >>>> >>>> On 27/10/2020 13:57, Tadeus Prastowo via Libc-help wrote: >>>>> >>>>> My understanding is that if my single-threaded program installs a >>>>> signal handler using signal() and the handler is executed as a result >>>>> of calling raise(), then the handler has a defined behavior when the >>>>> handler accesses any object with static storage duration even though >>>>> the object is not qualified using volatile and not of type >>>>> sig_atomic_t. >>>>> >>>>> If my understanding is incorrect, I would like to have some pointer to >>>>> parts of the C standard that say so. >>>> >>>> Unfortunately this is not fully correct, specially if you accessing the >>>> object outside the signal handler. As you have noticed in your example, >>>> compiler can assume the variable 'terminated' won't be modified outside >>>> the function and apply optimization that avoid read its value from >>>> memory (most likely GCC will optimize out the '!terminated'). >>> >>> However, the compiler cannot assume that the variable `terminated' >>> won't be modified outside the loop if the loop calls a function whose >>> definition the compiler does not see (e.g., the raise() function >>> defined by glibc). Otherwise, the compiler is wrong if the loop calls >>> a function defined in another translation unit because the function >>> may modify `terminated'. >>> >>>> One of the best didactic explanation I have for this is from [1]. >>>> >>>> [1] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers >>> >>> Thank you for the reference. >>> >>> I have studied it but found no pointer to parts of the C standard that >>> says that it is an undefined behavior to call a signal handler >>> synchronously using raise() on a normal execution path (i.e., not from >>> within another signal handler, which certainly is undefined behavior >>> as noted in "undefined behavior 131"). > > I meant to say: I have studied it but found no pointer to parts of the > C standard that says that it is an undefined behavior to access a > non-volatile object with static storage duration from within a signal > handler that is called synchronously using raise() on a normal > execution path (i.e., the note "undefined behavior 131" only talks > about an undefined behavior that results from calling raise() from > within a signal handler). > >>> I know that the safest thing to do is always to qualify such a >>> variable with `volatile', but I think it is valid to not do so for the >>> cases that are not stated as >>> undefined/unspecified/implementation-defined behavior by the C >>> standard. > >>> That is why I ask for a pointer to parts of the C standard >>> if indeed calling a signal handler synchronously using raise() on a >>> normal execution path is an undefined behavior. > > I meant to say: That is why I ask for a pointer to parts of the C > standard if indeed accessing a non-volatile object with static storage > duration from within a signal handler that is called synchronously > using raise() on a normal execution path is an undefined behavior. > >>> After further thought, I found out that the problem is not glibc-2.30 >>> marking raise() with __leaf__ but the file-scope limitation of >>> `terminated'. As I already mentioned, since `terminated' has a >>> file-scope, the compiler can assume that `terminated' will not be >>> modified by a function defined in another translation unit. And >>> hence, the compiler is correct in optimizing out the `!terminated'. >>> Once `terminated' has a global scope, the compiler correctly does not >>> optimize it out as exemplified by the attached program. So, marking >>> raise() with __leaf__ does not make raise() non-compliant with the C >>> standard. >> >> After re-reading the GCC documentation, I think marking raise() with >> __leaf__ makes raise() non-compliant with the C standard, unless the C >> standard says that calling a signal handler synchronously using >> raise() on a normal execution path is an undefined behavior. > > I meant to say: After re-reading the GCC documentation, I think > marking raise() with __leaf__ makes raise() non-compliant with the C > standard, unless the C standard says that accessing a non-volatile > object with static storage duration from within a signal handler that > is called synchronously using raise() on a normal execution path is an > undefined behavior. > >> The GCC documentation on __leaf__ >> (https://gcc.gnu.org/onlinedocs/gcc-9.3.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes) >> says (words in square brackets are added to fit our context): >> >> Note that leaf functions might indirectly run a signal handler defined >> in the current compilation unit that uses static variables. [...] >> There is no standard-compliant way to write such [an] implementation >> function [, for example, glibc-2.30 raise()], and the best that you >> can do is to remove the leaf attribute or mark all such static >> variables volatile. >> >> End quote. >> >> Glibc-2.30 raise() definitely runs a signal handler, and the signal >> handler can be defined in the current compilation unit using static >> variables. So, unless the C standard says that calling a signal >> handler synchronously using raise() on a normal execution path is an >> undefined behavior, the marking of raise() with __leaf__ makes raise() >> non-compliant with the C standard. > > I meant to say: Glibc-2.30 raise() definitely runs a signal handler, > and the signal handler can be defined in the current compilation unit > to use static variables. So, unless the C standard says that > accessing a non-volatile object with static storage duration from > within a signal handler that is called synchronously using raise() on > a normal execution path is an undefined behavior, the marking of > raise() with __leaf__ makes raise() non-compliant with the C standard. > >> May I know your opinion, please? The sentence "raise() definitely runs a signal handler" is not really valid in a portable sense. Afaik neither C nor POSIX states which signals should be delivered synchronously or asynchronously (although some do only make sense to be delivered synchronously such as SIGSEGV). However, Linux does ran some signals synchronously and I agree that using leaf attribute is incorrect and lead to this kind of problems. My point is to be fully portable, you need to assume any signal might be delivered asynchronously (and C standard specifies the volatile sig_atomic_t for such cases). Florian, what other leaf functions do you think are problematic? I am wondering if you would be better to just avoid this micro-optimization altogether.