From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 4E9563858D33 for ; Fri, 3 Mar 2023 11:21:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4E9563858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 4DCD920479; Fri, 3 Mar 2023 11:20:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1677842459; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yw73ZT3XOfR9tAGDBbuIsaUMFEctis793lPG7NPlN4w=; b=U/86r06R2Itf7OPEhmG6aUBVaLxZxxUNWdaAkq/RuqoqSIhm3/lUbe7Ky+1GZhDp28yRCz 5P2kSBUwYP8SwsByfMYfGC8XwKE7kEeso3mo9OvKiy47lk56cLFmof1vlohJJNlyqf9ACn 7VamEqLshx1vFqSXj7HL+mv4ypJXMl4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1677842459; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yw73ZT3XOfR9tAGDBbuIsaUMFEctis793lPG7NPlN4w=; b=x4L/XbphTPKBFJGcW8xqx4+HH9GSUFsj/LMWnPpyfxKGuyRYNlg9Dok1UV3C+95n2ojDlE UBysP0U/E38T4rDA== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 568B62C141; Fri, 3 Mar 2023 11:20:58 +0000 (UTC) Date: Fri, 3 Mar 2023 11:20:58 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] waccess: Fix two -Wnonnull warning issues [PR108986] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="-1609957120-1750834244-1677842459=:27913" X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP 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: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. ---1609957120-1750834244-1677842459=:27913 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT On Fri, 3 Mar 2023, Jakub Jelinek wrote: > Hi! > > The following patch fixes 2 issues with the -Wnonnull warning. > > One, fixed by the second hunk, is that the warning wording is bogus > since r11-3305, instead of printing > warning: argument 1 to ‘int[static 7]’ is null where non-null expected [-Wnonnull] > it prints > warning: argument 1 to ‘int[static 28]’ is null where non-null expected [-Wnonnull] > access_size is measured in bytes, so obviously will be correct as array > number of elements only if it is 1 byte element array. > In the function, access_nelts is either constant (if sizidx == -1) or > when the array size is determined by some other parameter, I believe a value > passed to that argument. > Later on we query the range of it: > if (get_size_range (m_ptr_qry.rvals, access_nelts, stmt, sizrng, 1)) > which I bet must just return accesS_nelts in sizrng[0] and [1] if it is > constant. access_size is later computed as: > tree access_size = NULL_TREE; > if (tree_int_cst_sgn (sizrng[0]) >= 0) > { > if (COMPLETE_TYPE_P (argtype)) > { > ... > wide_int minsize = wi::to_wide (sizrng[0], prec); > minsize *= wi::to_wide (argsize, prec); > access_size = wide_int_to_tree (sizetype, minsize); > } > } > else > access_size = access_nelts; > } > and immediately after this the code does: > if (integer_zerop (ptr)) > { > if (sizidx >= 0 && tree_int_cst_sgn (sizrng[0]) > 0) > { > some other warning wording > } > else if (access_size && access.second.static_p) > { > this spot > } > } > So, because argtype is complete, access_size has been multiplied by > argsize, but in case of this exact warning ("this spot" above) > I believe access_nelts must be really constant, otherwise > "some other warning wording" would handle it. So, I think access_nelts > is exactly what we want to print there. > > The other problem is that since the introduction of -Wdangling-pointer > in r12-6606, the pass has early and late instances and while lots of > stuff in the pass is guarded on being done in the late pass only, > this particular function is not, furthermore it is emitting two different > warnings in a loop and already messes up with stuff like clearing > warning suppression for one of the warning (ugh!). The end effect is > that we warn twice about the same problem, once in the early and once in > the late pass. Now, e.g. with -O2 -Wall we warn just once, during the > early pass, as it is then optimized away, so I think just making this > late warning only wouldn't be best. This patch instead returns early > if either of the warnings is suppressed on the call stmt already. > I think if one of the passes warned on it already (even if say on some other > argument), then warning again (even on some other argument) is unnecessary, > if both problems are visible in the same pass we'll still warn about both. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Thanks, Richard. > 2023-03-03 Jakub Jelinek > > PR c/108986 > * gimple-ssa-warn-access.cc (pass_waccess::maybe_check_access_sizes): > Return immediately if OPT_Wnonnull or OPT_Wstringop_overflow_ is > suppressed on stmt. For [static %E] warning, print access_nelts > rather than access_size. Fix up comment wording. > > * gcc.dg/Wnonnull-8.c: New test. > > --- gcc/gimple-ssa-warn-access.cc.jj 2023-02-22 20:50:27.418519815 +0100 > +++ gcc/gimple-ssa-warn-access.cc 2023-03-02 19:04:24.744280016 +0100 > @@ -3318,6 +3318,10 @@ void > pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype, > gimple *stmt) > { > + if (warning_suppressed_p (stmt, OPT_Wnonnull) > + || warning_suppressed_p (stmt, OPT_Wstringop_overflow_)) > + return; > + > auto_diagnostic_group adg; > > /* Set if a warning has been issued for any argument (used to decide > @@ -3501,7 +3505,7 @@ pass_waccess::maybe_check_access_sizes ( > if (warning_at (loc, OPT_Wnonnull, > "argument %i to %<%T[static %E]%> " > "is null where non-null expected", > - ptridx + 1, argtype, access_size)) > + ptridx + 1, argtype, access_nelts)) > arg_warned = OPT_Wnonnull; > } > > @@ -3593,7 +3597,7 @@ pass_waccess::maybe_check_access_sizes ( > "in a call with type %qT", fntype); > } > > - /* Set the bit in case if was cleared and not set above. */ > + /* Set the bit in case it was cleared and not set above. */ > if (opt_warned != no_warning) > suppress_warning (stmt, opt_warned); > } > --- gcc/testsuite/gcc.dg/Wnonnull-8.c.jj 2023-03-02 17:28:24.323898410 +0100 > +++ gcc/testsuite/gcc.dg/Wnonnull-8.c 2023-03-02 17:27:51.804376322 +0100 > @@ -0,0 +1,14 @@ > +/* PR c/108986 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wall" } */ > + > +void > +foo (int a[static 7]) > +{ > +} > + > +int > +main () > +{ > + foo ((int *) 0); /* { dg-warning "argument 1 to 'int\\\[static 7\\\]' is null where non-null expected" } */ > +} > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) ---1609957120-1750834244-1677842459=:27913--