Joseph, I would appreciate if you could take a look at this?
This fixes the remaining issues which requires me to turn the warnings off with -Wno-vla-parameter and -Wno-nonnull in my projects. Am Montag, dem 03.04.2023 um 21:34 +0200 schrieb Martin Uecker: > > With the relatively new warnings (11..) affecting VLA bounds, > I now get a lot of false positives with -Wall. In general, I find > the new warnings very useful, but they seem a bit too > aggressive and some minor tweaks are needed, otherwise they are > too noisy. This patch suggests two changes: > > 1. For VLA bounds non-null is implied only when 'static' is > used (similar to clang) and not already when a bound > 0 is > specified: > > int foo(int n, char buf[static n]); > > int foo(10, 0); // warning with 'static' but not without. > > > (It also seems problematic to require a size of 0 to indicate > that the pointer may be null, because 0 is not allowed in > ISO C as a size. It is also inconsistent to how arrays with > static bound behave.) > > There seems to be agreement about this change in PR98541. > > > 2. GCC always warns when the number of unspecified > bounds is different between two declarations: > > int foo(int n, char buf[*]); > int foo(int n, char buf[n]); > > or > > int foo(int n, char buf[n]); > int foo(int n, char buf[*]); > > But the first version is useful if the size expression > can not be specified in a header (e.g. because it uses > a macro or variable not available there) and there is > currently no easy way to avoid this. The warning for > both cases was by design, but I suggest to limit the > warning to the second case. > > Note that the logic currently applied by GCC is too > simplistic anyway, as GCC does not warn for > > int foo(int x, int y, double m[*][y]); > int foo(int x, int y, double m[x][*]); > > because the number of specified / unspecified bounds > is the same. So I suggest to go with the attached > patch now and add more precise warnings later > if there is more experience with these warning > in gernal and if this then still seems desirable. > > > Martin > > > Less warnings for parameters declared as arrays [PR98541, PR98536] > > To avoid false positivies, tune the warnings for parameters declared > as arrays with size expressions. Only warn about null arguments with > 'static'. Also do not warn when more bounds are specified in the new > declaration than before. > > PR c/98541 > PR c/98536 > > c-family/ > * c-warn.cc (warn_parm_array_mismatch): Do not warn if more > bounds are specified. > > gcc/ > * gimple-ssa-warn-access.cc > (pass_waccess::maybe_check_access_sizes): For VLA bounds > in parameters, only warn about null pointers with 'static'. > > gcc/testsuite: > * gcc.dg/Wnonnull-4: Adapt test. > * gcc.dg/Wstringop-overflow-40.c: Adapt test. > * gcc.dg/Wvla-parameter-4.c: Adapt test. > * gcc.dg/attr-access-2.c: Adapt test. > > > diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc > index 9ac43a1af6e..f79fb876142 100644 > --- a/gcc/c-family/c-warn.cc > +++ b/gcc/c-family/c-warn.cc > @@ -3599,23 +3599,13 @@ warn_parm_array_mismatch (location_t origloc, tree > fndecl, tree newparms) > continue; > } > > - if (newunspec != curunspec) > + if (newunspec > curunspec) > { > location_t warnloc = newloc, noteloc = origloc; > const char *warnparmstr = newparmstr.c_str (); > const char *noteparmstr = curparmstr.c_str (); > unsigned warnunspec = newunspec, noteunspec = curunspec; > > - if (newunspec < curunspec) > - { > - /* If the new declaration has fewer unspecified bounds > - point the warning to the previous declaration to make > - it clear that that's the one to change. Otherwise, > - point it to the new decl. */ > - std::swap (warnloc, noteloc); > - std::swap (warnparmstr, noteparmstr); > - std::swap (warnunspec, noteunspec); > - } > if (warning_n (warnloc, OPT_Wvla_parameter, warnunspec, > "argument %u of type %s declared with " > "%u unspecified variable bound", > @@ -3641,16 +3631,11 @@ warn_parm_array_mismatch (location_t origloc, tree > fndecl, tree newparms) > continue; > } > } > - > /* Iterate over the lists of VLA variable bounds, comparing each > - pair for equality, and diagnosing mismatches. The case of > - the lists having different lengths is handled above so at > - this point they do . */ > - for (tree newvbl = newa->size, curvbl = cura->size; newvbl; > + pair for equality, and diagnosing mismatches. */ > + for (tree newvbl = newa->size, curvbl = cura->size; newvbl && curvbl; > newvbl = TREE_CHAIN (newvbl), curvbl = TREE_CHAIN (curvbl)) > { > - gcc_assert (curvbl); > - > tree newpos = TREE_PURPOSE (newvbl); > tree curpos = TREE_PURPOSE (curvbl); > > @@ -3663,7 +3648,6 @@ warn_parm_array_mismatch (location_t origloc, tree > fndecl, tree newparms) > and both are the same expression they are necessarily > the same. */ > continue; > - > pretty_printer pp1, pp2; > const char* const newbndstr = expr_to_str (pp1, newbnd, "*"); > const char* const curbndstr = expr_to_str (pp2, curbnd, "*"); > diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc > index b3de4b77924..a405f830fb5 100644 > --- a/gcc/gimple-ssa-warn-access.cc > +++ b/gcc/gimple-ssa-warn-access.cc > @@ -3478,27 +3478,14 @@ pass_waccess::maybe_check_access_sizes (rdwr_map > *rwm, tree fndecl, tree fntype, > > if (integer_zerop (ptr)) > { > - if (sizidx >= 0 && tree_int_cst_sgn (sizrng[0]) > 0) > + if (!access.second.internal_p > + && sizidx >= 0 && tree_int_cst_sgn (sizrng[0]) > 0) > { > /* Warn about null pointers with positive sizes. This is > different from also declaring the pointer argument with > attribute nonnull when the function accepts null pointers > only when the corresponding size is zero. */ > - if (access.second.internal_p) > - { > - const std::string argtypestr > - = access.second.array_as_string (ptrtype); > - > - if (warning_at (loc, OPT_Wnonnull, > - "argument %i of variable length " > - "array %s is null but " > - "the corresponding bound argument " > - "%i value is %s", > - ptridx + 1, argtypestr.c_str (), > - sizidx + 1, sizstr)) > - arg_warned = OPT_Wnonnull; > - } > - else if (warning_at (loc, OPT_Wnonnull, > + if (warning_at (loc, OPT_Wnonnull, > "argument %i is null but " > "the corresponding size argument " > "%i value is %s", > diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c > b/gcc/testsuite/gcc.dg/Wnonnull-4.c > index 2c1c45a9856..1f14fbba45d 100644 > --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c > +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c > @@ -27,9 +27,9 @@ void test_fca_n (int r_m1) > T ( 0); > > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array > 'char\\\[n]' is null but the corresponding bound argument 1 value is 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array > 'char\\\[n]' is null but the corresponding bound argument 1 value is 9" } > - T (max); // { dg-warning "argument 2 of variable length array > 'char\\\[n]' is null but the corresponding bound argument 1 value is \\d+" } > + T ( 1); // { dg-bogus "argument 2 of variable length array > 'char\\\[n]' is null but the corresponding bound argument 1 value is 1" } > + T ( 9); // { dg-bogus "argument 2 of variable length array > 'char\\\[n]' is null but the corresponding bound argument 1 value is 9" } > + T (max); // { dg-bogus "argument 2 of variable length array > 'char\\\[n]' is null but the corresponding bound argument 1 value is \\d+" } > } > > > @@ -55,9 +55,9 @@ void test_fsa_x_n (int r_m1) > T ( 0); > > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array > 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 value > is 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array > 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 value > is 9" } > - T (max); // { dg-warning "argument 2 of variable length array > 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 value > is \\d+" } > + T ( 1); // { dg-bogus "argument 2 of variable length array > 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 value > is 1" } > + T ( 9); // { dg-bogus "argument 2 of variable length array > 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 value > is 9" } > + T (max); // { dg-bogus "argument 2 of variable length array > 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 value > is \\d+" } > } > > > @@ -83,9 +83,9 @@ void test_fia_1_n (int r_m1) > T ( 0); > > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array > 'int\\\[1]\\\[n]' is null but the corresponding bound argument 1 value is 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array > 'int\\\[1]\\\[n]' is null but the corresponding bound argument 1 value is 9" } > - T (max); // { dg-warning "argument 2 of variable length array > 'int\\\[1]\\\[n]' is null but the corresponding bound argument 1 value is > \\d+" } > + T ( 1); // { dg-bogus "argument 2 of variable length array > 'int\\\[1]\\\[n]' is null but the corresponding bound argument 1 value is 1" } > + T ( 9); // { dg-bogus "argument 2 of variable length array > 'int\\\[1]\\\[n]' is null but the corresponding bound argument 1 value is 9" } > + T (max); // { dg-bogus "argument 2 of variable length array > 'int\\\[1]\\\[n]' is null but the corresponding bound argument 1 value is > \\d+" } > } > > > @@ -111,9 +111,9 @@ void test_fla_3_n (int r_m1) > T ( 0); > > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array > 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 1 value > is 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array > 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 1 value > is 9" } > - T (max); // { dg-warning "argument 2 of variable length array > 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 1 value > is \\d+" } > + T ( 1); // { dg-bogus "argument 2 of variable length array 'long > int\\\[3]\\\[n]' is null but the corresponding bound argument 1 value is 1" } > + T ( 9); // { dg-bogus "argument 2 of variable length array 'long > int\\\[3]\\\[n]' is null but the corresponding bound argument 1 value is 9" } > + T (max); // { dg-bogus "argument 2 of variable length array 'long > int\\\[3]\\\[n]' is null but the corresponding bound argument 1 value is > \\d+" } > } > > > @@ -139,9 +139,9 @@ void test_fda_n_5 (int r_m1) > T ( 0); > > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array > 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is > 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array > 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is > 9" } > - T (max); // { dg-warning "argument 2 of variable length array > 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is > \\d+" } > + T ( 1); // { dg-bogus "argument 2 of variable length array > 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is > 1" } > + T ( 9); // { dg-bogus "argument 2 of variable length array > 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is > 9" } > + T (max); // { dg-bogus "argument 2 of variable length array > 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is > \\d+" } > } > > > @@ -167,7 +167,7 @@ void test_fca_n_n (int r_m1) > T ( 0); > > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array > 'char\\\[n]\\\[n]' is null but the corresponding bound argument 1 value is 1" > } > - T ( 9); // { dg-warning "argument 2 of variable length array > 'char\\\[n]\\\[n]' is null but the corresponding bound argument 1 value is 9" > } > - T (max); // { dg-warning "argument 2 of variable length array > 'char\\\[n]\\\[n]' is null but the corresponding bound argument 1 value is > \\d+" } > + T ( 1); // { dg-bogus "argument 2 of variable length array > 'char\\\[n]\\\[n]' is null but the corresponding bound argument 1 value is 1" > } > + T ( 9); // { dg-bogus "argument 2 of variable length array > 'char\\\[n]\\\[n]' is null but the corresponding bound argument 1 value is 9" > } > + T (max); // { dg-bogus "argument 2 of variable length array > 'char\\\[n]\\\[n]' is null but the corresponding bound argument 1 value is > \\d+" } > } > diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c > b/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c > index 386c92dc7a8..9e0ad1f3aff 100644 > --- a/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c > +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c > @@ -11,6 +11,7 @@ void fxa2 (int16_t[2]) __attribute__ ((nonnull)); > void fas2 (int16_t[static 2]); > > void fvla (unsigned n, int16_t[n]); > +void fvlaS (unsigned n, int16_t[static n]); > > void test_array_1_dim (void) > { > @@ -33,7 +34,8 @@ void test_array_1_dim (void) > fas2 (a1); // { dg-warning "'fas2' accessing 4 bytes in a > region of size 2 " } > fas2 (&i); // { dg-warning "'fas2' accessing 4 bytes in a > region of size 2 " } > > - fvla (1, 0); // { dg-warning "\\\[-Wnonnull" } > + fvla (1, 0); > + fvlaS (1, 0); // { dg-warning "\\\[-Wnonnull" } > fvla (1, &i); > fvla (2, a2); > fvla (2, a1); // { dg-warning "'fvla' accessing 4 bytes in a > region of size 2 " } > @@ -47,6 +49,7 @@ void fxac2 (const int16_t[2]) __attribute__ ((nonnull)); > void facs2 (const int16_t[static 2]); > > void fvlac (unsigned n, const int16_t[n]); > +void fvlacS (unsigned n, const int16_t[static n]); > > void test_const_array_1_dim (void) > { > @@ -69,7 +72,8 @@ void test_const_array_1_dim (void) > facs2 (a1); // { dg-warning "'facs2' reading 4 bytes from > a region of size 2 " } > facs2 (&i); // { dg-warning "'facs2' reading 4 bytes from > a region of size 2 " } > > - fvlac (1, 0); // { dg-warning "\\\[-Wnonnull" } > + fvlac (1, 0); > + fvlacS (1, 0); // { dg-warning "\\\[-Wnonnull" } > fvlac (1, &i); > fvlac (2, a2); > fvlac (2, a1); // { dg-warning "'fvlac' reading 4 bytes from > a region of size 2 " } > diff --git a/gcc/testsuite/gcc.dg/Wvla-parameter-4.c > b/gcc/testsuite/gcc.dg/Wvla-parameter-4.c > index 599ad19a3e4..79f72a94c7f 100644 > --- a/gcc/testsuite/gcc.dg/Wvla-parameter-4.c > +++ b/gcc/testsuite/gcc.dg/Wvla-parameter-4.c > @@ -9,12 +9,9 @@ extern int m, n; > > typedef int IA3[3]; > > -/* Verify the warning points to the declaration with more unspecified > - bounds, guiding the user to specify them rather than making them all > - unspecified. */ > -void* f_pIA3ax (IA3 *x[*]); // { dg-warning "argument 1 of type > 'int \\\(\\\*\\\[\\\*]\\\)\\\[3]' .aka '\[^\n\r\}\]+'. declared with 1 > unspecified variable bound" } > void* f_pIA3ax (IA3 *x[*]); > -void* f_pIA3ax (IA3 *x[n]); // { dg-message "subsequently > declared as 'int \\\(\\\*\\\[n]\\\)\\\[3]' with 0 unspecified variable > bounds" "note" } > +void* f_pIA3ax (IA3 *x[*]); > +void* f_pIA3ax (IA3 *x[n]); > void* f_pIA3ax (IA3 *x[n]) { return x; } > > > diff --git a/gcc/testsuite/gcc.dg/attr-access-2.c > b/gcc/testsuite/gcc.dg/attr-access-2.c > index 76baddffc9f..616b7a9527c 100644 > --- a/gcc/testsuite/gcc.dg/attr-access-2.c > +++ b/gcc/testsuite/gcc.dg/attr-access-2.c > @@ -60,16 +60,6 @@ RW (2, 1) void f10 (int n, char a[n]) // { dg-warning > "attribute 'access *\\\( > // { dg-warning "argument 2 of type > 'char\\\[n]' declared as a variable length array" "" { target *-*-* } .-1 } > { (void)&n; (void)&a; } > > - > -/* The following is diagnosed to point out declarations with the T[*] > - form in headers where specifying the bound is just as important as > - in the definition (to detect bugs). */ > - void f11 (int, char[*]); // { dg-warning "argument 2 of type > 'char\\\[\\\*\\\]' declared with 1 unspecified variable bound" } > - void f11 (int m, char a[m]); // { dg-message "subsequently > declared as 'char\\\[m]' with 0 unspecified variable bounds" "note" } > -RW (2, 1) void f11 (int n, char arr[n]) // { dg-message "subsequently > declared as 'char\\\[n]' with 0 unspecified variable bounds" "note" } > -{ (void)&n; (void)&arr; } > - > - > /* Verify that redeclaring a function with attribute access applying > to an array parameter of any form is not diagnosed. */ > void f12__ (int, int[]) RW (2, 1); > > > >