* [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]
@ 2023-10-31 19:05 Martin Uecker
2023-11-02 7:36 ` Richard Biener
2023-11-07 3:58 ` Hans-Peter Nilsson
0 siblings, 2 replies; 12+ messages in thread
From: Martin Uecker @ 2023-10-31 19:05 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener
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.)
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 " }
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]
2023-10-31 19:05 [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541] Martin Uecker
@ 2023-11-02 7:36 ` Richard Biener
2023-11-07 3:58 ` Hans-Peter Nilsson
1 sibling, 0 replies; 12+ messages in thread
From: Richard Biener @ 2023-11-02 7:36 UTC (permalink / raw)
To: Martin Uecker; +Cc: gcc-patches
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 " }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]
2023-10-31 19:05 [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541] Martin Uecker
2023-11-02 7:36 ` Richard Biener
@ 2023-11-07 3:58 ` Hans-Peter Nilsson
2023-11-07 4:01 ` Jeff Law
1 sibling, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2023-11-07 3:58 UTC (permalink / raw)
To: Martin Uecker; +Cc: gcc-patches, richard.guenther
> From: Martin Uecker <uecker@tugraz.at>
> Date: Tue, 31 Oct 2023 20:05:09 +0100
> 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.
This patch caused a testsuite regression: there's now an
"excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
targets (and 64-bit targets testing with a "-m32" option)
after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
brgds, H-P
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]
2023-11-07 3:58 ` Hans-Peter Nilsson
@ 2023-11-07 4:01 ` Jeff Law
2023-11-07 5:56 ` Martin Uecker
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2023-11-07 4:01 UTC (permalink / raw)
To: Hans-Peter Nilsson, Martin Uecker; +Cc: gcc-patches, richard.guenther
On 11/6/23 20:58, Hans-Peter Nilsson wrote:
>> From: Martin Uecker <uecker@tugraz.at>
>> Date: Tue, 31 Oct 2023 20:05:09 +0100
>
>> 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.
>
> This patch caused a testsuite regression: there's now an
> "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> targets (and 64-bit targets testing with a "-m32" option)
> after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
It caused failures for just about every target ;( Presumably it worked
on x86_64...
jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]
2023-11-07 4:01 ` Jeff Law
@ 2023-11-07 5:56 ` Martin Uecker
2023-11-16 4:24 ` Hans-Peter Nilsson
0 siblings, 1 reply; 12+ messages in thread
From: Martin Uecker @ 2023-11-07 5:56 UTC (permalink / raw)
To: Jeff Law, Hans-Peter Nilsson; +Cc: gcc-patches, richard.guenther
Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
>
> On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > From: Martin Uecker <uecker@tugraz.at>
> > > Date: Tue, 31 Oct 2023 20:05:09 +0100
> >
> > > 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.
> >
> > This patch caused a testsuite regression: there's now an
> > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > targets (and 64-bit targets testing with a "-m32" option)
> > after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
> It caused failures for just about every target ;( Presumably it worked
> on x86_64...
I do not think this is a true regression
just a problem with the test on 32-bit which somehow surfaced
due to the change.
The excess error is:
FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
Excess errors:
/home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
2147483647 [-Wstringop-overflow=]
I think the warning was suppressed before due to the other (nonnull)
warning which I removed in this case.
I think the simple fix might be to to turn off -Wstringop-overflow.
Link to the change:
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=6e9ee44d96e5bda8808dd9d8ccf58d2525383f6b
Martin
>
> jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]
2023-11-07 5:56 ` Martin Uecker
@ 2023-11-16 4:24 ` Hans-Peter Nilsson
2023-11-23 17:05 ` Ping: [PATCH] Fix PR112419 (was: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]) Hans-Peter Nilsson
0 siblings, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2023-11-16 4:24 UTC (permalink / raw)
To: gcc-patches; +Cc: Martin Uecker, jeffreyalaw, richard.guenther
> From: Martin Uecker <uecker@tugraz.at>
> Date: Tue, 07 Nov 2023 06:56:25 +0100
> Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> >
> > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > This patch caused a testsuite regression: there's now an
> > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > targets (and 64-bit targets testing with a "-m32" option)
> > > after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
> > It caused failures for just about every target ;( Presumably it worked
> > on x86_64...
>
> I do not think this is a true regression
> just a problem with the test on 32-bit which somehow surfaced
> due to the change.
>
> The excess error is:
>
> FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> Excess errors:
> /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> 2147483647 [-Wstringop-overflow=]
>
> I think the warning was suppressed before due to the other (nonnull)
> warning which I removed in this case.
>
> I think the simple fix might be to to turn off -Wstringop-overflow.
No, that trigs many of the dg-warnings that are tested.
(I didn't pay attention to the actual warning messages and
tried to pursue that at first.)
Maybe think it's best to actually expect the warning, like
so.
Maintainers of 16-bit targets will have to address their
concerns separately. For example, they may choose to not
run the test at all.
Ok to commit?
Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit targets [PR112419]
PR testsuite/112419
* gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
maximum object size for 32-bit targets.
---
gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c
index 1f14fbba45df..d63e76da70a2 100644
--- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
+++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
@@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
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+" }
+// { dg-warning "size 4294967256 exceeds maximum object size" "" { target ilp32 } .-1 }
}
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Ping: [PATCH] Fix PR112419 (was: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541])
2023-11-16 4:24 ` Hans-Peter Nilsson
@ 2023-11-23 17:05 ` Hans-Peter Nilsson
2023-11-27 15:36 ` Ping: [PATCH] Fix PR112419 Jeff Law
0 siblings, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2023-11-23 17:05 UTC (permalink / raw)
To: gcc-patches; +Cc: Martin Uecker, jeffreyalaw, richard.guenther
> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 16 Nov 2023 05:24:06 +0100
>
> > From: Martin Uecker <uecker@tugraz.at>
> > Date: Tue, 07 Nov 2023 06:56:25 +0100
>
> > Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > >
> > > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > > This patch caused a testsuite regression: there's now an
> > > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > > targets (and 64-bit targets testing with a "-m32" option)
> > > > after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
> > > It caused failures for just about every target ;( Presumably it worked
> > > on x86_64...
> >
> > I do not think this is a true regression
> > just a problem with the test on 32-bit which somehow surfaced
> > due to the change.
> >
> > The excess error is:
> >
> > FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> > Excess errors:
> > /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> > 2147483647 [-Wstringop-overflow=]
> >
> > I think the warning was suppressed before due to the other (nonnull)
> > warning which I removed in this case.
> >
> > I think the simple fix might be to to turn off -Wstringop-overflow.
>
> No, that trigs many of the dg-warnings that are tested.
>
> (I didn't pay attention to the actual warning messages and
> tried to pursue that at first.)
>
> Maybe think it's best to actually expect the warning, like
> so.
>
> Maintainers of 16-bit targets will have to address their
> concerns separately. For example, they may choose to not
> run the test at all.
>
> Ok to commit?
>
> Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit targets [PR112419]
>
> PR testsuite/112419
> * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
> maximum object size for 32-bit targets.
> ---
> gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> index 1f14fbba45df..d63e76da70a2 100644
> --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
> +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
> 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+" }
> +// { dg-warning "size 4294967256 exceeds maximum object size" "" { target ilp32 } .-1 }
> }
>
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Ping: [PATCH] Fix PR112419
2023-11-23 17:05 ` Ping: [PATCH] Fix PR112419 (was: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]) Hans-Peter Nilsson
@ 2023-11-27 15:36 ` Jeff Law
2023-11-27 15:54 ` Martin Uecker
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2023-11-27 15:36 UTC (permalink / raw)
To: Hans-Peter Nilsson, gcc-patches; +Cc: Martin Uecker, richard.guenther
On 11/23/23 10:05, Hans-Peter Nilsson wrote:
>> From: Hans-Peter Nilsson <hp@axis.com>
>> Date: Thu, 16 Nov 2023 05:24:06 +0100
>>
>>> From: Martin Uecker <uecker@tugraz.at>
>>> Date: Tue, 07 Nov 2023 06:56:25 +0100
>>
>>> Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
>>>>
>>>> On 11/6/23 20:58, Hans-Peter Nilsson wrote:
>>>>> This patch caused a testsuite regression: there's now an
>>>>> "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
>>>>> targets (and 64-bit targets testing with a "-m32" option)
>>>>> after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
>>>> It caused failures for just about every target ;( Presumably it worked
>>>> on x86_64...
>>>
>>> I do not think this is a true regression
>>> just a problem with the test on 32-bit which somehow surfaced
>>> due to the change.
>>>
>>> The excess error is:
>>>
>>> FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
>>> Excess errors:
>>> /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
>>> 2147483647 [-Wstringop-overflow=]
>>>
>>> I think the warning was suppressed before due to the other (nonnull)
>>> warning which I removed in this case.
>>>
>>> I think the simple fix might be to to turn off -Wstringop-overflow.
>>
>> No, that trigs many of the dg-warnings that are tested.
>>
>> (I didn't pay attention to the actual warning messages and
>> tried to pursue that at first.)
>>
>> Maybe think it's best to actually expect the warning, like
>> so.
>>
>> Maintainers of 16-bit targets will have to address their
>> concerns separately. For example, they may choose to not
>> run the test at all.
>>
>> Ok to commit?
>>
>> Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit targets [PR112419]
>>
>> PR testsuite/112419
>> * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
>> maximum object size for 32-bit targets.
>> ---
>> gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c
>> index 1f14fbba45df..d63e76da70a2 100644
>> --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
>> +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
>> @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
>> 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+" }
>> +// { dg-warning "size 4294967256 exceeds maximum object size" "" { target ilp32 } .-1 }
>> }
Unfortunately I think we need to go back to the original issue that
Martin (I think) dismissed.
Specifically, this is a regression. It's very clear that prior to the
patch in question there was no diagnostic about the size of the
requested memory allocation and after the patch in question we get the
"exceeds maximum object size" diagnostic.
Now one explanation could be that the diagnostic is warranted and it was
a bug that the diagnostic hadn't been emitted prior to Martin's patch.
In this case some kind of dg-blah is warranted, but I don't think anyone
has made this argument.
Jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Ping: [PATCH] Fix PR112419
2023-11-27 15:36 ` Ping: [PATCH] Fix PR112419 Jeff Law
@ 2023-11-27 15:54 ` Martin Uecker
2023-11-27 16:10 ` Martin Uecker
2023-11-30 17:27 ` Hans-Peter Nilsson
0 siblings, 2 replies; 12+ messages in thread
From: Martin Uecker @ 2023-11-27 15:54 UTC (permalink / raw)
To: Jeff Law, Hans-Peter Nilsson, gcc-patches; +Cc: richard.guenther
Am Montag, dem 27.11.2023 um 08:36 -0700 schrieb Jeff Law:
>
> On 11/23/23 10:05, Hans-Peter Nilsson wrote:
> > > From: Hans-Peter Nilsson <hp@axis.com>
> > > Date: Thu, 16 Nov 2023 05:24:06 +0100
> > >
> > > > From: Martin Uecker <uecker@tugraz.at>
> > > > Date: Tue, 07 Nov 2023 06:56:25 +0100
> > >
> > > > Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > > > >
> > > > > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > > > > This patch caused a testsuite regression: there's now an
> > > > > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > > > > targets (and 64-bit targets testing with a "-m32" option)
> > > > > > after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
> > > > > It caused failures for just about every target ;( Presumably it worked
> > > > > on x86_64...
> > > >
> > > > I do not think this is a true regression
> > > > just a problem with the test on 32-bit which somehow surfaced
> > > > due to the change.
> > > >
> > > > The excess error is:
> > > >
> > > > FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> > > > Excess errors:
> > > > /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> > > > 2147483647 [-Wstringop-overflow=]
> > > >
> > > > I think the warning was suppressed before due to the other (nonnull)
> > > > warning which I removed in this case.
> > > >
> > > > I think the simple fix might be to to turn off -Wstringop-overflow.
> > >
> > > No, that trigs many of the dg-warnings that are tested.
> > >
> > > (I didn't pay attention to the actual warning messages and
> > > tried to pursue that at first.)
> > >
> > > Maybe think it's best to actually expect the warning, like
> > > so.
> > >
> > > Maintainers of 16-bit targets will have to address their
> > > concerns separately. For example, they may choose to not
> > > run the test at all.
> > >
> > > Ok to commit?
> > >
> > > Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit targets [PR112419]
> > >
> > > PR testsuite/112419
> > > * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
> > > maximum object size for 32-bit targets.
> > > ---
> > > gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > index 1f14fbba45df..d63e76da70a2 100644
> > > --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
> > > 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+" }
> > > +// { dg-warning "size 4294967256 exceeds maximum object size" "" { target ilp32 } .-1 }
> > > }
> Unfortunately I think we need to go back to the original issue that
> Martin (I think) dismissed.
>
> Specifically, this is a regression. It's very clear that prior to the
> patch in question there was no diagnostic about the size of the
> requested memory allocation and after the patch in question we get the
> "exceeds maximum object size" diagnostic.
>
> Now one explanation could be that the diagnostic is warranted and it was
> a bug that the diagnostic hadn't been emitted prior to Martin's patch.
> In this case some kind of dg-blah is warranted, but I don't think anyone
> has made this argument.
>
I believe the warning is correct but was suppressed before.
My plan was to split up the test case in one which is for
-Wstringop-overflow and one which is for -Wnonnull and then
one could turn off the -Wstringop-overflow for the tests
which are actually for -Wnonnull. But adding the dg-blah
would certainly be simpler.
Martin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Ping: [PATCH] Fix PR112419
2023-11-27 15:54 ` Martin Uecker
@ 2023-11-27 16:10 ` Martin Uecker
2023-11-30 17:27 ` Hans-Peter Nilsson
1 sibling, 0 replies; 12+ messages in thread
From: Martin Uecker @ 2023-11-27 16:10 UTC (permalink / raw)
To: Jeff Law, Hans-Peter Nilsson, gcc-patches; +Cc: richard.guenther
Am Montag, dem 27.11.2023 um 16:54 +0100 schrieb Martin Uecker:
> Am Montag, dem 27.11.2023 um 08:36 -0700 schrieb Jeff Law:
> >
> > On 11/23/23 10:05, Hans-Peter Nilsson wrote:
> > > > From: Hans-Peter Nilsson <hp@axis.com>
> > > > Date: Thu, 16 Nov 2023 05:24:06 +0100
> > > >
> > > > > From: Martin Uecker <uecker@tugraz.at>
> > > > > Date: Tue, 07 Nov 2023 06:56:25 +0100
> > > >
> > > > > Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > > > > >
> > > > > > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > > > > > This patch caused a testsuite regression: there's now an
> > > > > > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > > > > > targets (and 64-bit targets testing with a "-m32" option)
> > > > > > > after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
> > > > > > It caused failures for just about every target ;( Presumably it worked
> > > > > > on x86_64...
> > > > >
> > > > > I do not think this is a true regression
> > > > > just a problem with the test on 32-bit which somehow surfaced
> > > > > due to the change.
> > > > >
> > > > > The excess error is:
> > > > >
> > > > > FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> > > > > Excess errors:
> > > > > /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> > > > > 2147483647 [-Wstringop-overflow=]
> > > > >
> > > > > I think the warning was suppressed before due to the other (nonnull)
> > > > > warning which I removed in this case.
> > > > >
> > > > > I think the simple fix might be to to turn off -Wstringop-overflow.
> > > >
> > > > No, that trigs many of the dg-warnings that are tested.
> > > >
> > > > (I didn't pay attention to the actual warning messages and
> > > > tried to pursue that at first.)
> > > >
> > > > Maybe think it's best to actually expect the warning, like
> > > > so.
> > > >
> > > > Maintainers of 16-bit targets will have to address their
> > > > concerns separately. For example, they may choose to not
> > > > run the test at all.
> > > >
> > > > Ok to commit?
> > > >
> > > > Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit targets [PR112419]
> > > >
> > > > PR testsuite/112419
> > > > * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
> > > > maximum object size for 32-bit targets.
> > > > ---
> > > > gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > index 1f14fbba45df..d63e76da70a2 100644
> > > > --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
> > > > 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+" }
> > > > +// { dg-warning "size 4294967256 exceeds maximum object size" "" { target ilp32 } .-1 }
> > > > }
> > Unfortunately I think we need to go back to the original issue that
> > Martin (I think) dismissed.
> >
> > Specifically, this is a regression. It's very clear that prior to the
> > patch in question there was no diagnostic about the size of the
> > requested memory allocation and after the patch in question we get the
> > "exceeds maximum object size" diagnostic.
> >
> > Now one explanation could be that the diagnostic is warranted and it was
> > a bug that the diagnostic hadn't been emitted prior to Martin's patch.
> > In this case some kind of dg-blah is warranted, but I don't think anyone
> > has made this argument.
> >
> I believe the warning is correct but was suppressed before.
>
>
> My plan was to split up the test case in one which is for
> -Wstringop-overflow and one which is for -Wnonnull and then
> one could turn off the -Wstringop-overflow for the tests
> which are actually for -Wnonnull. But adding the dg-blah
> would certainly be simpler.
Specifically, also with 13.2 if you suppress the warning which
I removed with -Wno-nonnull you will get the otherwise hidden
-Wstringop-overflow warning with -m32:
See here: https://godbolt.org/z/ev5GhMonq
The warning also seems correct to me, so I suggest to accept
the proposed patch.
Martin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Ping: [PATCH] Fix PR112419
2023-11-27 15:54 ` Martin Uecker
2023-11-27 16:10 ` Martin Uecker
@ 2023-11-30 17:27 ` Hans-Peter Nilsson
2023-12-01 14:57 ` Jeff Law
1 sibling, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2023-11-30 17:27 UTC (permalink / raw)
To: jeffreyalaw; +Cc: gcc-patches, Martin Uecker, richard.guenther
> From: Martin Uecker <uecker@tugraz.at>
> Cc: richard.guenther@gmail.com
> Am Montag, dem 27.11.2023 um 08:36 -0700 schrieb Jeff Law:
> >
> > On 11/23/23 10:05, Hans-Peter Nilsson wrote:
> > > > From: Hans-Peter Nilsson <hp@axis.com>
> > > > Date: Thu, 16 Nov 2023 05:24:06 +0100
> > > >
> > > > > From: Martin Uecker <uecker@tugraz.at>
> > > > > Date: Tue, 07 Nov 2023 06:56:25 +0100
> > > >
> > > > > Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > > > > >
> > > > > > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > > > > > This patch caused a testsuite regression: there's now an
> > > > > > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > > > > > targets (and 64-bit targets testing with a "-m32" option)
> > > > > > > after your r14-5115-g6e9ee44d96e5. It's logged as PR112419.
> > > > > > It caused failures for just about every target ;( Presumably it worked
> > > > > > on x86_64...
> > > > >
> > > > > I do not think this is a true regression
> > > > > just a problem with the test on 32-bit which somehow surfaced
> > > > > due to the change.
> > > > >
> > > > > The excess error is:
> > > > >
> > > > > FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> > > > > Excess errors:
> > > > > /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3: warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> > > > > 2147483647 [-Wstringop-overflow=]
> > > > >
> > > > > I think the warning was suppressed before due to the other (nonnull)
> > > > > warning which I removed in this case.
> > > > >
> > > > > I think the simple fix might be to to turn off -Wstringop-overflow.
> > > >
> > > > No, that trigs many of the dg-warnings that are tested.
> > > >
> > > > (I didn't pay attention to the actual warning messages and
> > > > tried to pursue that at first.)
> > > >
> > > > Maybe think it's best to actually expect the warning, like
> > > > so.
> > > >
> > > > Maintainers of 16-bit targets will have to address their
> > > > concerns separately. For example, they may choose to not
> > > > run the test at all.
> > > >
> > > > Ok to commit?
> > > >
> > > > Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit targets [PR112419]
> > > >
> > > > PR testsuite/112419
> > > > * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
> > > > maximum object size for 32-bit targets.
> > > > ---
> > > > gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > index 1f14fbba45df..d63e76da70a2 100644
> > > > --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
> > > > 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+" }
> > > > +// { dg-warning "size 4294967256 exceeds maximum object size" "" { target ilp32 } .-1 }
> > > > }
> > Unfortunately I think we need to go back to the original issue that
> > Martin (I think) dismissed.
> >
> > Specifically, this is a regression. It's very clear that prior to the
> > patch in question there was no diagnostic about the size of the
> > requested memory allocation and after the patch in question we get the
> > "exceeds maximum object size" diagnostic.
> >
> > Now one explanation could be that the diagnostic is warranted and it was
> > a bug that the diagnostic hadn't been emitted prior to Martin's patch.
> > In this case some kind of dg-blah is warranted, but I don't think anyone
> > has made this argument.
> >
> I believe the warning is correct but was suppressed before.
>
>
> My plan was to split up the test case in one which is for
> -Wstringop-overflow and one which is for -Wnonnull and then
> one could turn off the -Wstringop-overflow for the tests
> which are actually for -Wnonnull. But adding the dg-blah
> would certainly be simpler.
Sort-of-mid-week ping (only because status quo isn't clear):
Jeff, are you content with Martin U:s reply (i.e. patch ok)
or are you waiting for a follow-up?
Perhaps it's already in your overflowing queue, then please
ignore this, I'll just ping in a week. ;-)
IMHO, after looking at the test-case I'd expect *more*
warnings for ilp32 targets; i.e. it was a bug that it didn't
show up before.
brgds, H-P
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Ping: [PATCH] Fix PR112419
2023-11-30 17:27 ` Hans-Peter Nilsson
@ 2023-12-01 14:57 ` Jeff Law
0 siblings, 0 replies; 12+ messages in thread
From: Jeff Law @ 2023-12-01 14:57 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: gcc-patches, Martin Uecker, richard.guenther
On 11/30/23 10:27, Hans-Peter Nilsson wrote:
>>
>>
>> My plan was to split up the test case in one which is for
>> -Wstringop-overflow and one which is for -Wnonnull and then
>> one could turn off the -Wstringop-overflow for the tests
>> which are actually for -Wnonnull. But adding the dg-blah
>> would certainly be simpler.
>
> Sort-of-mid-week ping (only because status quo isn't clear):
> Jeff, are you content with Martin U:s reply (i.e. patch ok)
> or are you waiting for a follow-up?
>
> Perhaps it's already in your overflowing queue, then please
> ignore this, I'll just ping in a week. ;-)
>
> IMHO, after looking at the test-case I'd expect *more*
> warnings for ilp32 targets; i.e. it was a bug that it didn't
> show up before.
Sorry, not waiting on anything. Just crazy busy with some personal stuff.
I think we can go with the patch as-is.
Jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-12-01 14:57 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-31 19:05 [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541] Martin Uecker
2023-11-02 7:36 ` Richard Biener
2023-11-07 3:58 ` Hans-Peter Nilsson
2023-11-07 4:01 ` Jeff Law
2023-11-07 5:56 ` Martin Uecker
2023-11-16 4:24 ` Hans-Peter Nilsson
2023-11-23 17:05 ` Ping: [PATCH] Fix PR112419 (was: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]) Hans-Peter Nilsson
2023-11-27 15:36 ` Ping: [PATCH] Fix PR112419 Jeff Law
2023-11-27 15:54 ` Martin Uecker
2023-11-27 16:10 ` Martin Uecker
2023-11-30 17:27 ` Hans-Peter Nilsson
2023-12-01 14:57 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).