From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id E82F03858D32 for ; Thu, 2 Nov 2023 07:39:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E82F03858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E82F03858D32 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::133 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698910783; cv=none; b=tNGne2MnNaQri4UH4pqOy8BYztEX4K48SJiBzT/5rLTgwaVooJzZjuu7bj/ztMKLm4BAMkQTgkxjbb+0m0XvRwLrTqNjYCkd3XtVPGza/B65MOwRyUbCkV0Dl0nnswLI3c0RLewBKfQRSYTXX9UZTw52qCh73yATZVU0hb+ujzY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698910783; c=relaxed/simple; bh=T2nsxQ3y5mtvsJ667Dwl+xWnKvov7r+dbguT1+FyOqo=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=JdQNoX3H7hDCTMKW2qGMQ7TN8HWjBrn1MrQaqHxXQO53YHqavkGbRawXtGfa9CbzdpocHnNzhb2LsJ+UgU4acuv8eA+leYPPenPK8Js8gdITP+kYIro0PeEAE5HerHmjuE5ZHr29ZBApZvS9cw8a50ytv2rHAtZXYYMbH6BJAV4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x133.google.com with SMTP id 2adb3069b0e04-5079f9675c6so711820e87.2 for ; Thu, 02 Nov 2023 00:39:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1698910771; x=1699515571; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YGmcWEtlGLittUWyBaCca0NrP0u7TX+Im2QUHMZZ15c=; b=S+EKbDS/oyixYDCbv1G8pP7iuMhZzkZOWfGu5vfCHiy9yb1lJj1Kp3T365F2PNL95V 5LJ96xK7OGBFsQ0hK2Avv5b4TcVIsnHBJX+XiV2eiIjp1mgnvTI6pH4FYPrtuAoOZ3Ba /G2dUy2UpDgUNhZTfHQ8vXXeUC12IUHIIhyjslJAMJuS10FRliY87ohVZ3tILt7KEy1X 63Fgyrx4VSosjzXmZ1HM6GLGLCd+xDyCnj3G1BKsZk62cEDs/eFGopCZCeQPpQlyN6kd w7FnfeEh8LbQRrcHIOXsKK7Z1CqgYmGIkbGxcVXSz5uOmn8AXOd1XMdDGX1gFi74byr2 07zQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698910771; x=1699515571; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YGmcWEtlGLittUWyBaCca0NrP0u7TX+Im2QUHMZZ15c=; b=VTWH2eLWNQCp6Jlu3+v112veCDiwU8a2opvugluOF9LjS3LVuo+ZyN3gYCReckLWlv QAhIjEPoGJ7ymtIxic9kJkqkBJOVyz1jL1UX6/KcW5MFg71vNjKwyBF5V4OY+S9dEdg9 fRoDkSHP7e/YvDvvrwicFwVdU1qQGTlVxAuVO+VxRPde8F8ne0Tjy48fZZWnnN8brj8a odrJEV/aRyEQtkKxHjHgOoTrHxp9Zi1+tJoKd7OnHG7jvPhA+RN1cQEKwUqDTyVCUH+s fDSqFh9DFKKfldHw+0VEFM+oxaSf1GV+L01oy4x/ZQLyRysD691S26kQVS+W7biWzLvo VG2A== X-Gm-Message-State: AOJu0YyNxEUNGadZ8giY+vVKcVaXGGtbJOTIPe5L9fS841x0Z7doN5+I ZvKOJlgTNbks4Dpe1muhnn01DgUrijeOmo+x/Oo= X-Google-Smtp-Source: AGHT+IHjjI02jqVY/ARCmE2/VkOpQvLtS7CFUU5En5YtTVB/RB+v+vVp30NPXz1Fx6L3dCta7vRAGtDZjkMzRsodomc= X-Received: by 2002:a05:6512:201c:b0:508:266a:e85f with SMTP id a28-20020a056512201c00b00508266ae85fmr13524767lfb.1.1698910771016; Thu, 02 Nov 2023 00:39:31 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Thu, 2 Nov 2023 08:36:14 +0100 Message-ID: Subject: Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541] To: Martin Uecker Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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: On Tue, Oct 31, 2023 at 8:05=E2=80=AFPM Martin Uecker wr= ote: > > > 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.c= c > 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 >=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 pointer= s > 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); > > // 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 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+" } > } > > > @@ -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 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+" } > } > > > @@ -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 f= rom a region of size 2 " } > facs2 (&i); // { dg-warning "'facs2' reading 4 bytes f= rom 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 f= rom a region of size 2 " } >