From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id A0FAF3857421 for ; Fri, 16 Jul 2021 17:42:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A0FAF3857421 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: grfmuRkKCkuLpfnXyzkJHRGq4lCmJjBc4Z7fNtSm89J5RRXVwXEvjTJZjeNylEVbjy8WuU4PV+ QeZ0g7SlCpsjSj/4ch1Di1MjgnHOLnnnONRh2y3YoJRJ45bPPtIkjIHDXXnjhQhCHosdHytY04 jQRCzcEshB8m8aTwicyIJHtpmkkY0k1tCZ7jikQQw6FstytA8XtkwXEp2D1gm1pk0wh2ZMTLof XZiVR5xyA7pGZaslVhuD4ITm4rsa9a+4LGSNX+yWVbH0NRqxZXJc89tZCYboo8LrbzIspyvrMA XIpfTLVAxAKc4kuAic/aUSd8 X-IronPort-AV: E=Sophos;i="5.84,245,1620720000"; d="scan'208";a="63791605" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 16 Jul 2021 09:42:28 -0800 IronPort-SDR: Pt7CTyV1Oh+/7L6vYCAtrhpF8GDTokzboFioKUtG23GVLvXm3WXqbI0bx3ibh3SMoPB8TmmUlC WioaQaVUyz+69RHEvw10oj4mh2Rli5yiz6/yHUjzF1E/76j6WENFFDtZHjiToIsWtlBePUd7an QX73hpCT8NKX9sj19VK9aVOhO+qUNsd3JtpqynT+3bpV2OFFt+pJdtIuip7+NxUKWUkk+mWJfY EnDgxNetuE34nyAPbQfgrWUeoxyZrNgZ5xWbhcOxyGHCU08d3LvTyO0K5Jx2SptBhUReAAbLbN J3c= From: Thomas Schwinge To: Martin Sebor CC: , Christophe Lyon , Hafiz Abid Qadeer , Andrew Stubbs Subject: Re: [PATCH libatomic/arm] avoid warning on constant addresses (PR 101379) In-Reply-To: <816ce216-bf6a-75ca-4241-4861ec43ab27@gmail.com> References: <816ce216-bf6a-75ca-4241-4861ec43ab27@gmail.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/27.1 (x86_64-pc-linux-gnu) Date: Fri, 16 Jul 2021 19:42:19 +0200 Message-ID: <87pmvii29w.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, 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: Fri, 16 Jul 2021 17:42:31 -0000 Hi Martin! 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 =E2=80=98gcn_thrs=E2=80=99, inlined from =E2=80=98gomp_thread=E2=80=99 at [...]/source-gcc/libg= omp/libgomp.h:803:10, inlined from =E2=80=98GOMP_barrier=E2=80=99 at [...]/source-gcc/lib= gomp/barrier.c:34:29: [...]/source-gcc/libgomp/libgomp.h:792:10: error: array subscript 0 is = outside array bounds of =E2=80=98__lds struct gomp_thread * __lds[0]=E2=80= =99 [-Werror=3Darray-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 =3D (struct gomp_t= hread * __lds *)4; libgomp/libgomp.h- return *thrs; libgomp/libgomp.h-} ..., plus a few more. Work-around: struct gomp_thread * __lds *thrs =3D (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. (So I'll consider some GCN-specific '-Wno-array-bounds' if we don't get to resolve this otherwise, soon.) > As we have discussed, the main goal of this class of warnings > is to detect accesses at addresses derived from null pointers > (e.g., to struct members or array elements at a nonzero offset). (ACK, and thanks for that work!) > Diagnosing accesses at hardcoded addresses is incidental because > at the stage they are detected the two are not distinguishable > from each another. > > I'm planning (hoping) to implement detection of invalid pointer > arithmetic involving null for GCC 12, so this patch is a stopgap > solution to unblock the arm libatomic build without compromising > the warning. Once the new detection is in place these workarounds > can be removed or replaced with something more appropriate (e.g., > declaring the objects at the hardwired addresses with an attribute > like AVR's address or io; that would enable bounds checking at > those addresses as well). Of course, we may simply re-work the libgomp/GCN code -- but don't we first need to answer the question whether the current code is actually "bad"? Aren't we going to get a lot of similar reports from kernel/embedded/other low-level software developers, once this is out in the wild? I mean: > PR bootstrap/101379 - libatomic arm build failure after r12-2132 due to -= Warray-bounds on a constant address > > libatomic/ChangeLog: > * /config/linux/arm/host-config.h (__kernel_helper_version): New > function. Adjust shadow macro. > > diff --git a/libatomic/config/linux/arm/host-config.h b/libatomic/config/= linux/arm/host-config.h > index 1520f237d73..777d08a2b85 100644 > --- a/libatomic/config/linux/arm/host-config.h > +++ b/libatomic/config/linux/arm/host-config.h > @@ -39,8 +39,14 @@ typedef void (__kernel_dmb_t) (void); > #define __kernel_dmb (*(__kernel_dmb_t *) 0xffff0fa0) > > /* Kernel helper page version number. */ > -#define __kernel_helper_version (*(unsigned int *)0xffff0ffc) Are such (not un-common) '#define's actually "bad", and anyhow ought to be replaced by something like the following? > +static inline unsigned* > +__kernel_helper_version () > +{ > + unsigned *volatile addr =3D (unsigned int *)0xffff0ffc; > + return addr; > +} > > +#define __kernel_helper_version (*__kernel_helper_version()) (No 'volatile' in the original code, by the way.) Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955