Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Oct 31, 2023 at 8:05 PM Martin Uecker <uecker@tugraz.at> wrote:
>
>
> This is a revised part of previously posted patch which
> I split up. C FE changes which another false positive
> were already merged, but I still need approval for this
>  middle-end change.  It would be nice to get this in,
> because it fixes some rather annoying (for me atleast)
> false positive warnings with no easy workaround.
>
> In the following example,
>
> int foo(int n, float matrix[n], float opt[n]);
> foo(n, matrix, NULL);
>
> GCC warns about NULL iff n > 0.  This is problematic for
> several reasons:
> 1. It causes false positives (and I turn off -Wnonnull
> in one of my projects for this reason)
> 2. It is inconsistent with regular arrays where there is no
> warning in this case.
> 3. The size parameter is sometimes shared (as in this example)
> so passing zero to avoid the warning is only possible by
> making the code more complex.
> 4. Passing zero as a workaround is technically UB.
>
>
> (The original author of the warning code, Martin S seemed to
> agree with this change according to this discussion in Bugzilla.)

OK.

>
>
>     Reduce false positives for -Wnonnull for VLA parameters [PR98541]
>
>     This patch limits the warning about NULL arguments to VLA
>     parameters declared [static n].
>
>             PR c/98541
>
>     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.
>
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index e439d1b9b68..8b734295f09 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -3477,27 +3477,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 " }
>