From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc29.google.com (mail-oo1-xc29.google.com [IPv6:2607:f8b0:4864:20::c29]) by sourceware.org (Postfix) with ESMTPS id EF47A3851C0C for ; Tue, 20 Jul 2021 19:47:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EF47A3851C0C Received: by mail-oo1-xc29.google.com with SMTP id i11-20020a4adf0b0000b0290263e1ba7ff9so55336oou.2 for ; Tue, 20 Jul 2021 12:47:03 -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:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=t7SCw5XCGczdLtUQVPLjk5VKGUwey+D6b5idtzlkmjo=; b=LCnkdHS3CHHgbPsP9m2XhgLJaOUQUpzGRDy5zFbzqN81KMM2HQ81n1xb+lV4an7gtG mXVtslDwYKz01h1ZUE8Ui/HiLMsrCcFsyFS8bhqGvjJpsedbciGbG4ZuzFKSxynumXG8 zGY2q1jmeWaXkdCD+PWBKWJmdnl1KT9XZKiqet+pTkoBamk30I+ukLo3NiQdk2f729G6 lGs1ydyIJBvanjyVYchl5ruBK15QmA9Xk19mUUTPUF9lSVVR9pnVKwrIkHCmu6CYuuVF l5n19xn/d/uRVHBJXrHCwan/3sJIaF+fd4zfCVMTX5ym9JbATZRoSn5sKvwdjgskN87b 01/Q== X-Gm-Message-State: AOAM532koQzFd5qldPxnDrarcZIm/8bYnLxRh6zs6ZjMG4fqRPJXJ0C1 blF9A4rvw5y++K8szzmxDHY= X-Google-Smtp-Source: ABdhPJyOC0568eU6lc6SzJ6OpSuyYnc33AiWvb9GyXkshfYjmkbcQI03/Wk3sJjAD7B4LUqAVKEEjA== X-Received: by 2002:a4a:df02:: with SMTP id i2mr14956280oou.14.1626810423360; Tue, 20 Jul 2021 12:47:03 -0700 (PDT) Received: from [192.168.0.41] (75-166-102-22.hlrn.qwest.net. [75.166.102.22]) by smtp.gmail.com with ESMTPSA id i16sm4561643oie.5.2021.07.20.12.47.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Jul 2021 12:47:03 -0700 (PDT) Subject: Re: '#pragma GCC diagnostic' (mis-)use in 'statement' of 'if' To: Thomas Schwinge , gcc-patches@gcc.gnu.org Cc: Andrew Stubbs , Hafiz Abid Qadeer , doko@debian.org, Jakub Jelinek , David Malcolm References: <816ce216-bf6a-75ca-4241-4861ec43ab27@gmail.com> <87pmvii29w.fsf@euler.schwinge.homeip.net> <2e085663-b619-5240-6a40-961420341ebd@codesourcery.com> <87bl6ybsic.fsf@euler.schwinge.homeip.net> <87v9558n4j.fsf@euler.schwinge.homeip.net> <87sg098jk9.fsf@euler.schwinge.homeip.net> From: Martin Sebor Message-ID: <21cf55d5-9a95-dd71-a939-10883d781b7f@gmail.com> Date: Tue, 20 Jul 2021 13:47:01 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <87sg098jk9.fsf@euler.schwinge.homeip.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jul 2021 19:47:05 -0000 On 7/20/21 2:40 AM, Thomas Schwinge wrote: > Hi! > > On 2021-07-20T09:23:24+0200, I wrote: >> On 2021-07-19T10:46:35+0200, I wrote: >>> | On 7/16/21 11:42 AM, Thomas Schwinge wrote: >>> |> On 2021-07-09T17:11:25-0600, Martin Sebor via Gcc-patches wrote: >>> |>> The attached tweak avoids the new -Warray-bounds instances when >>> |>> building libatomic for arm. Christophe confirms it resolves >>> |>> the problem (thank you!) >>> |> >>> |> As Abid has just reported in >>> |> , similar >>> |> problem with GCN target libgomp build: >>> |> >>> |> In function ‘gcn_thrs’, >>> |> inlined from ‘gomp_thread’ at [...]/source-gcc/libgomp/libgomp.h:803:10, >>> |> inlined from ‘GOMP_barrier’ at [...]/source-gcc/libgomp/barrier.c:34:29: >>> |> [...]/source-gcc/libgomp/libgomp.h:792:10: error: array subscript 0 is outside array bounds of ‘__lds struct gomp_thread * __lds[0]’ [-Werror=array-bounds] >>> |> 792 | return *thrs; >>> |> | ^~~~~ >>> |> >>> |> gcc/config/gcn/gcn.h: c_register_addr_space ("__lds", ADDR_SPACE_LDS); \ >>> |> >>> |> libgomp/libgomp.h-static inline struct gomp_thread *gcn_thrs (void) >>> |> libgomp/libgomp.h-{ >>> |> libgomp/libgomp.h- /* The value is at the bottom of LDS. */ >>> |> libgomp/libgomp.h: struct gomp_thread * __lds *thrs = (struct gomp_thread * __lds *)4; >>> |> libgomp/libgomp.h- return *thrs; >>> |> libgomp/libgomp.h-} >>> |> >>> |> ..., plus a few more. Work-around: >>> |> >>> |> struct gomp_thread * __lds *thrs = (struct gomp_thread * __lds *)4; >>> |> +# pragma GCC diagnostic push >>> |> +# pragma GCC diagnostic ignored "-Warray-bounds" >>> |> return *thrs; >>> |> +# pragma GCC diagnostic pop >>> |> >>> |> ..., but it's a bit tedious to add that in all that the other places, >>> |> too. >>> >>> Wasn't so bad after all; a lot of duplicates due to 'libgomp.h'. I've >>> thus pushed "[gcn] Work-around libgomp 'error: array subscript 0 is >>> outside array bounds of ‘__lds struct gomp_thread * __lds[0]’ >>> [-Werror=array-bounds]' [PR101484]" to master branch in commit >>> 9f2bc5077debef2b046b6c10d38591ac324ad8b5, see attached. >> >> As I should find, these '#pragma GCC diagnostic [...]' directives cause >> some code generation changes (that seems unexpected, problematic!). >> (Martin, any idea? Might be a pre-existing problem, of course.) > > OK, phew. Martin: your diagnostic changes are *not* to be blamed for > code generation changes -- it's my '#pragma GCC diagnostic pop' > placement that triggers: > >> This >> results in a lot (ten thousands) of 'GCN team arena exhausted' run-time >> diagnostics, also leading to a few FAILs: >> >> PASS: libgomp.c/../libgomp.c-c++-common/for-11.c (test for excess errors) >> [-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-11.c execution test >> >> PASS: libgomp.c/../libgomp.c-c++-common/for-12.c (test for excess errors) >> [-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-12.c execution test >> >> PASS: libgomp.c/../libgomp.c-c++-common/for-3.c (test for excess errors) >> [-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-3.c execution test >> >> PASS: libgomp.c/../libgomp.c-c++-common/for-5.c (test for excess errors) >> [-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-5.c execution test >> >> PASS: libgomp.c/../libgomp.c-c++-common/for-6.c (test for excess errors) >> [-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-6.c execution test >> >> PASS: libgomp.c/../libgomp.c-c++-common/for-9.c (test for excess errors) >> [-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-9.c execution test >> >> Same for 'libgomp.c++'. >> >> It remains to be analyzed how '#pragma GCC diagnostic [...]' directives >> can cause code generation changes; for now I'm working around the >> "unexpected" '-Werror=array-bounds' diagnostics differently: > > In addition to a few in straight-line code, I also had these two: > >> --- a/libgomp/libgomp.h >> +++ b/libgomp/libgomp.h >> @@ -128,7 +128,10 @@ team_malloc (size_t size) >> : "=v"(result) : "v"(TEAM_ARENA_FREE), "v"(size), "e"(1L) : "memory"); >> >> /* Handle OOM. */ >> +# pragma GCC diagnostic push >> +# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */ >> if (result + size > *(void * __lds *)TEAM_ARENA_END) >> +# pragma GCC diagnostic pop >> { >> /* While this is experimental, let's make sure we know when OOM >> happens. */ >> @@ -162,8 +159,11 @@ team_free (void *ptr) >> However, if we fell back to using heap then we should free it. >> It would be better if this function could be a no-op, but at least >> LDS loads are cheap. */ >> +# pragma GCC diagnostic push >> +# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */ >> if (ptr < *(void * __lds *)TEAM_ARENA_START >> || ptr >= *(void * __lds *)TEAM_ARENA_END) >> +# pragma GCC diagnostic pop >> free (ptr); >> } >> #else > > ..., and it appears that the '#pragma GCC diagnostic pop' are considered > here to be the 'statement' of the 'if'! That's (a) unexpected (to me, at > least) for this kind of "non-executable" '#pragma', and (b) certainly > would be worth a dignostic, like we have for OMP pragmas, for example: > > if (context == pragma_stmt) > { > error_at (loc, "%<#pragma %s%> may only be used in compound statements", > "[...]"); > I agree, that does seem quite surprising. I opened pr101538 only to find that the problem is already tracked in pr63326. > Addressing that is for another day. David Malcolm (CC'd) has a patch attached to pr63326 to issue a warning to point out that #pragmas are treated as statements that would help prevent this type of a bug. David, do you still plan to submit it? Martin