public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Uecker <uecker@tugraz.at>
To: Jeff Law <jeffreyalaw@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Less warnings for parameters declared as arrays [PR98541, PR98536]
Date: Wed, 05 Apr 2023 15:25:33 +0200	[thread overview]
Message-ID: <08213278b2703d14fc298ecdcdb0fa94dc9f3d0c.camel@tugraz.at> (raw)
In-Reply-To: <7e3070d8-c214-808e-3e52-a22aba64d786@gmail.com>

Am Dienstag, dem 04.04.2023 um 19:31 -0600 schrieb Jeff Law:
> 
> On 4/3/23 13:34, Martin Uecker via Gcc-patches wrote:
> > 
> > 
> > With the relatively new warnings (11..) affecting VLA bounds,
> > I now get a lot of false positives with -Wall. In general, I find
> > the new warnings very useful, but they seem a bit too
> > aggressive and some minor tweaks are needed, otherwise they are
> > too noisy.  This patch suggests two changes:
> > 
> > 1. For VLA bounds non-null is implied only when 'static' is
> > used (similar to clang) and not already when a bound > 0 is
> > specified:
> > 
> > int foo(int n, char buf[static n]);
> > 
> > int foo(10, 0); // warning with 'static' but not without.
> > 
> > 
> > (It also seems problematic to require a size of 0 to indicate
> > that the pointer may be null, because 0 is not allowed in
> > ISO C as a size. It is also inconsistent to how arrays with
> > static bound behave.)
> > 
> > There seems to be agreement about this change in PR98541.
> > 
> > 
> > 2. GCC always warns when the number of unspecified
> > bounds is different between two declarations:
> > 
> > int foo(int n, char buf[*]);
> > int foo(int n, char buf[n]);
> > 
> > or
> > 
> > int foo(int n, char buf[n]);
> > int foo(int n, char buf[*]);
> > 
> > But the first version is useful if the size expression
> > can not be specified in a header (e.g. because it uses
> > a macro or variable not available there) and there is
> > currently no easy way to avoid this.  The warning for
> > both cases was by design,  but I suggest to limit the
> > warning to the second case.
> > 
> > Note that the logic currently applied by GCC is too
> > simplistic anyway, as GCC does not warn for
> > 
> > int foo(int x, int y, double m[*][y]);
> > int foo(int x, int y, double m[x][*]);
> > 
> > because the number of specified / unspecified bounds
> > is the same.  So I suggest to go with the attached
> > patch now and add  more precise warnings later
> > if there is more experience with these warning
> > in gernal and if this then still seems desirable.
> > 
> > 
> > Martin
> > 
> > 
> >      Less warnings for parameters declared as arrays [PR98541, PR98536]
> >      
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >      To avoid false positivies, tune the warnings for parameters declared
> >      as arrays with size expressions.  Only warn about null arguments with
> >      'static'.  Also do not warn when more bounds are specified in the new
> >      declaration than before.
> >      
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >              PR c/98541
> >              PR c/98536
> >      
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >              c-family/
> >              * c-warn.cc (warn_parm_array_mismatch): Do not warn if more
> >              bounds are specified.
> >      
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >              gcc/
> >              * gimple-ssa-warn-access.cc
> >                (pass_waccess::maybe_check_access_sizes): For VLA bounds
> >              in parameters, only warn about null pointers with 'static'.
> >      
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >              gcc/testsuite:
> >              * gcc.dg/Wnonnull-4: Adapt test.
> >              * gcc.dg/Wstringop-overflow-40.c: Adapt test.
> >              * gcc.dg/Wvla-parameter-4.c: Adapt test.
> >              * gcc.dg/attr-access-2.c: Adapt test.
> Neither appears to be a regression.  Seems like it should defer to gcc-14.

The false positives are a regression relative to earlier version of GCC
because the warnings are completely new (since GCC 11).   I could bring
this back later, but this means there is another version of GCC where 
one might need to turn off these warnings.  My concern is also that
an otherwise very useful feature that brings some safety benefits gets
underused. 

Martin





  reply	other threads:[~2023-04-05 13:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-03 19:34 Martin Uecker
2023-04-05  1:31 ` Jeff Law
2023-04-05 13:25   ` Martin Uecker [this message]
2023-04-20  7:00   ` Martin Uecker
2023-07-19 21:19   ` [PING 2] " Martin Uecker
2023-07-31  7:24 ` [PING 3] " Martin Uecker
2023-07-31 16:28   ` Joseph Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=08213278b2703d14fc298ecdcdb0fa94dc9f3d0c.camel@tugraz.at \
    --to=uecker@tugraz.at \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).