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);
>
>
>
>
