From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailrelay.tugraz.at (mailrelay.tugraz.at [129.27.2.202]) by sourceware.org (Postfix) with ESMTPS id 4EFE63858C5E for ; Mon, 31 Jul 2023 07:24:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4EFE63858C5E Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=tugraz.at Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tugraz.at Received: from vra-168-66.tugraz.at (vra-168-66.tugraz.at [129.27.168.66]) by mailrelay.tugraz.at (Postfix) with ESMTPSA id 4RDqVT1lBTz1LMXp; Mon, 31 Jul 2023 09:24:00 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 mailrelay.tugraz.at 4RDqVT1lBTz1LMXp DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tugraz.at; s=mailrelay; t=1690788241; bh=v6ik5PeC3FfNDhefSoOddmxfRTcmWLYaLzldx9SC1/U=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=WaSzzl4GTs2/R2/exT3kIXIzlVMr8ldueQJOyG5Ic76s0CBSTQi6182KZTkdEtRvt 20lfsE9sdIK1SEpxv1jP++mz266Yhul5WlQZZAtTX/ClMny087IEyAY4gH+wExnoZ3 0y2ua0lj29ZCXKudEmy+jMu2eb1lOu2eX43okavE= Message-ID: Subject: [PING 3] [PATCH] Less warnings for parameters declared as arrays [PR98541, PR98536] From: Martin Uecker To: gcc-patches@gcc.gnu.org Cc: Joseph Myers Date: Mon, 31 Jul 2023 09:24:00 +0200 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4-2 MIME-Version: 1.0 X-TUG-Backscatter-control: G/VXY7/6zeyuAY/PU2/0qw X-Spam-Scanner: SpamAssassin 3.003001 X-Spam-Score-relay: -1.9 X-Scanned-By: MIMEDefang 2.74 on 129.27.10.117 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Joseph, I would appreciate if you could take a look at this? =20 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: >=20 > 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: >=20 > 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: >=20 > int foo(int n, char buf[static n]); >=20 > int foo(10, 0); // warning with 'static' but not without. >=20 >=20 > (It also seems problematic to require a size of 0 to indicate=C2=A0 > 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.)=20 >=20 > There seems to be agreement about this change in PR98541. >=20 >=20 > 2. GCC always warns when the number of unspecified > bounds is different between two declarations: >=20 > int foo(int n, char buf[*]); > int foo(int n, char buf[n]); >=20 > or >=20 > int foo(int n, char buf[n]); > int foo(int n, char buf[*]); >=20 > 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,=C2=A0 but I suggest to limit the > warning to the second case.=20 >=20 > Note that the logic currently applied by GCC is too > simplistic anyway, as GCC does not warn for >=20 > int foo(int x, int y, double m[*][y]); > int foo(int x, int y, double m[x][*]); >=20 > 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=C2=A0 > in gernal and if this then still seems desirable. >=20 >=20 > Martin >=20 >=20 > Less warnings for parameters declared as arrays [PR98541, PR98536] > =20 > 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. > =20 > PR c/98541 > PR c/98536 > =20 > c-family/ > * c-warn.cc (warn_parm_array_mismatch): Do not warn if more > bounds are specified. > =20 > gcc/ > * gimple-ssa-warn-access.cc > (pass_waccess::maybe_check_access_sizes): For VLA bounds > in parameters, only warn about null pointers with 'static'. > =20 > 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. >=20 >=20 > 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, tre= e fndecl, tree newparms) > continue; > } > =20 > - if (newunspec !=3D curunspec) > + if (newunspec > curunspec) > { > location_t warnloc =3D newloc, noteloc =3D origloc; > const char *warnparmstr =3D newparmstr.c_str (); > const char *noteparmstr =3D curparmstr.c_str (); > unsigned warnunspec =3D newunspec, noteunspec =3D curunspec; > =20 > - 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, tre= e 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 =3D newa->size, curvbl =3D cura->size; newvbl; > + pair for equality, and diagnosing mismatches. */ > + for (tree newvbl =3D newa->size, curvbl =3D cura->size; newvbl && = curvbl; > newvbl =3D TREE_CHAIN (newvbl), curvbl =3D TREE_CHAIN (curvbl)) > { > - gcc_assert (curvbl); > - > tree newpos =3D TREE_PURPOSE (newvbl); > tree curpos =3D TREE_PURPOSE (curvbl); > =20 > @@ -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 =3D expr_to_str (pp1, newbnd, "*"); > const char* const curbndstr =3D expr_to_str (pp2, curbnd, "*"); > diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.c= c > 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, > =20 > if (integer_zerop (ptr)) > { > - if (sizidx >=3D 0 && tree_int_cst_sgn (sizrng[0]) > 0) > + if (!access.second.internal_p > + && sizidx >=3D 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 > - =3D 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 =3D 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/Wno= nnull-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); > =20 > // 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+" } > } > =20 > =20 > @@ -55,9 +55,9 @@ void test_fsa_x_n (int r_m1) > T ( 0); > =20 > // Verify positive bounds. > - T ( 1); // { dg-warning "argument 2 of variable length array= 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 valu= e is 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array= 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 valu= e is 9" } > - T (max); // { dg-warning "argument 2 of variable length array= 'short int\\\[]\\\[n]' is null but the corresponding bound argument 1 valu= e 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+" } > } > =20 > =20 > @@ -83,9 +83,9 @@ void test_fia_1_n (int r_m1) > T ( 0); > =20 > // 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+" } > } > =20 > =20 > @@ -111,9 +111,9 @@ void test_fla_3_n (int r_m1) > T ( 0); > =20 > // 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 valu= e is 1" } > - T ( 9); // { dg-warning "argument 2 of variable length array= 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 1 valu= e is 9" } > - T (max); // { dg-warning "argument 2 of variable length array= 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 1 valu= e 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+" } > } > =20 > =20 > @@ -139,9 +139,9 @@ void test_fda_n_5 (int r_m1) > T ( 0); > =20 > // 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+" } > } > =20 > =20 > @@ -167,7 +167,7 @@ void test_fca_n_n (int r_m1) > T ( 0); > =20 > // 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]); > =20 > void fvla (unsigned n, int16_t[n]); > +void fvlaS (unsigned n, int16_t[static n]); > =20 > 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 " } > =20 > - 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]); > =20 > void fvlac (unsigned n, const int16_t[n]); > +void fvlacS (unsigned n, const int16_t[static n]); > =20 > 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 f= rom a region of size 2 " } > facs2 (&i); // { dg-warning "'facs2' reading 4 bytes f= rom a region of size 2 " } > =20 > - 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 f= rom 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; > =20 > typedef int IA3[3]; > =20 > -/* 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 t= ype '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 de= clared 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; } > =20 > =20 > 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-warnin= g "attribute 'access *\\\( > // { dg-warning "argument 2 of t= ype 'char\\\[n]' declared as a variable length array" "" { target *-*-* } = .-1 } > { (void)&n; (void)&a; } > =20 > - > -/* 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 t= ype 'char\\\[\\\*\\\]' declared with 1 unspecified variable bound" } > - void f11 (int m, char a[m]); // { dg-message "subsequently de= clared as 'char\\\[m]' with 0 unspecified variable bounds" "note" } > -RW (2, 1) void f11 (int n, char arr[n]) // { dg-message "subsequently de= clared 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); >=20 >=20 >=20 >=20