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.

Reply via email to