Hi Martin! On 2021-07-09T17:11:25-0600, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> 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 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101374#c16>, 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. (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 = (unsigned int *)0xffff0ffc; > + return addr; > +} > > +#define __kernel_helper_version (*__kernel_helper_version()) (No 'volatile' in the original code, by the way.) Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
