The 09/23/2020 21:45, Jeff Law wrote: > On 9/23/20 11:45 AM, Martin Sebor via Gcc-patches wrote: > > On 9/23/20 9:44 AM, Szabolcs Nagy wrote: > > > The 09/23/2020 09:22, Szabolcs Nagy wrote: > > > > The 09/21/2020 12:45, Martin Sebor via Gcc-patches wrote: > > > > > On 9/21/20 12:20 PM, Vaseeharan Vinayagamoorthy wrote: > > > > > > After this patch, I am seeing this -Warray-parameter error: > > > > > > > > > > > > In file included from ../include/pthread.h:1, > > > > > > from ../sysdeps/nptl/thread_db.h:25, > > > > > > from ../nptl/descr.h:32, > > > > > > from ../sysdeps/aarch64/nptl/tls.h:44, > > > > > > from ../include/errno.h:25, > > > > > > from ../sysdeps/unix/sysv/linux/sysdep.h:23, > > > > > > from > > > > > > ../sysdeps/unix/sysv/linux/generic/sysdep.h:22, > > > > > > from > > > > > > ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:24, > > > > > > from <stdin>:1: > > > > > > ../sysdeps/nptl/pthread.h:734:47: error: argument 1 of > > > > > > type ‘struct __jmp_buf_tag *’ declared as a pointer > > > > > > [-Werror=array-parameter=] > > > > > > 734 | extern int __sigsetjmp (struct __jmp_buf_tag > > > > > > *__env, int __savemask) __THROWNL; > > > > > > | ~~~~~~~~~~~~~~~~~~~~~~^~~~~ > > > > > > In file included from ../include/setjmp.h:2, > > > > > > from ../nptl/descr.h:24, > > > > > > from ../sysdeps/aarch64/nptl/tls.h:44, > > > > > > from ../include/errno.h:25, > > > > > > from ../sysdeps/unix/sysv/linux/sysdep.h:23, > > > > > > from > > > > > > ../sysdeps/unix/sysv/linux/generic/sysdep.h:22, > > > > > > from > > > > > > ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:24, > > > > > > from <stdin>:1: > > > > > > ../setjmp/setjmp.h:54:46: note: previously declared as > > > > > > an array ‘struct __jmp_buf_tag[1]’ > > > > > > 54 | extern int __sigsetjmp (struct __jmp_buf_tag > > > > > > __env[1], int __savemask) __THROWNL; > > > > > > | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ > > > > > > cc1: all warnings being treated as errors > > > > > > > > > > The warning flags differences between the forms of array parameters > > > > > in redeclarations of the same function, including pointers vs arrays > > > > > as in this instance. It needs to be suppressed in glibc, either by > > > > > making the function declarations consistent, or by #pragma diagnostic. > > > > > (IIRC, the pointer declaration comes before struct __jmp_buf_tag has > > > > > been defined so simply using the array form there doesn't work without > > > > > defining the type first.) > > > > > > > > > > I would expect the warning to be suppressed when using the installed > > > > > library thanks to -Wno-system-headers. > > > > > > > > why is this a warning? i'm not convinced it > > > > should be in -Wall. > > > > The main motivation for the warning is to detect unintentional > > inconsistencies between function redeclarations that make deriving > > true true intent difficult or impossible (e.g, T[3] vs T[1], or > > T[] vs T[1], or equivalently T* vs T[1]). > > > > One goal is to support the convention where a constant array bound > > in a function array parameter is used in lieu of the [static N] > > notation (i.e., the minimum number of elements the caller is > > expected to make available). The [static N] notation is little > > known, used only exceedingly rarely, and isn't available in C++. > > The array notation is used more often, although by no means common. > > > > The ultimate goal is to motivate users to take advantage of GCC's > > ability to check ordinary functions for out-of-bounds accesses to > > array arguments. The checking is only feasible if all declarations > > of the same function, including its definition, use a consistent > > notation to specify the same bound. Including the strict > > -Warray-parameter=2 setting in -Wall helps support this goal > > (-Warray-parameter=1 doesn't warn for mismatches in the forms > > of ordinary array bounds without [static].) > > > > I mentioned the results of testing the patch with a number of > > packages, including Glibc, Binutils/GDB, Glibc, and the kernel, > > in the overview of the patch series: > > https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550920.html > > It explains why I chose not to relax the warning to accommodate > > the Glibc use case. > > > > Based on my admittedly limited testing I'm not concerned about > > the warning having adverse effects. But if broader excposure > > shows that it is prone to some it can certainly be adjusted. > > Jeff does periodic mass rebuilds of all of Fedora with the top > > of GCC trunk so we should know soon. > > Yea. If the patch was in last week's snapshot, then it should start > spinning the 9000 Fedora packages later tonight alongside some Ranger bits > we're testing. I'm on PTO till Monday though, so I won't be looking at the > results until Tuesday probably...
any news on this? i still think that the warning is only appropriate for [static N] and not [N], (and that the main reason static N is not used is because compilers ignore it.) so i'd like to see if the warning is causing problems in practice before fixing glibc.