From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27946 invoked by alias); 24 Jul 2015 08:06:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 27935 invoked by uid 89); 24 Jul 2015 08:06:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f174.google.com Received: from mail-wi0-f174.google.com (HELO mail-wi0-f174.google.com) (209.85.212.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 24 Jul 2015 08:06:28 +0000 Received: by wicgb10 with SMTP id gb10so17241592wic.1 for ; Fri, 24 Jul 2015 01:06:25 -0700 (PDT) X-Received: by 10.180.78.136 with SMTP id b8mr4480280wix.89.1437725185384; Fri, 24 Jul 2015 01:06:25 -0700 (PDT) Received: from [10.39.107.101] (089144232101.atnat0041.highway.a1.net. [89.144.232.101]) by smtp.gmail.com with ESMTPSA id lg6sm11323299wjb.10.2015.07.24.01.06.23 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 Jul 2015 01:06:24 -0700 (PDT) User-Agent: K-9 Mail for Android In-Reply-To: <55B1CD5B.6010101@redhat.com> References: <55B127D7.3020202@redhat.com> <3EAB7D8C-698E-418A-A269-DF4389142264@gmail.com> <55B1CD5B.6010101@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: PR c/16351 Extend Wnonnull for returns_nonnull From: Bernhard Reutner-Fischer Date: Fri, 24 Jul 2015 08:09:00 -0000 To: Jeff Law ,=?ISO-8859-1?Q?Manuel_L=F3pez-Ib=E1=F1ez?= ,Gcc Patch List ,Jason Merrill ,"Joseph S. Myers" Message-ID: <3BF63D43-13A6-4227-9195-305AED17DCBE@gmail.com> X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg02005.txt.bz2 On July 24, 2015 7:30:03 AM GMT+02:00, Jeff Law wrote: >On 07/23/2015 04:44 PM, Bernhard Reutner-Fischer wrote: >> On July 23, 2015 7:43:51 PM GMT+02:00, Jeff Law >wrote: >>> On 07/22/2015 09:29 AM, Manuel López-Ibáñez wrote: >>>> While looking at PR c/16351, I noticed that all tests proposed for >>>> -Wnull-attribute >>>> (https://gcc.gnu.org/ml/gcc-patches/2014-01/msg01715.html) could be >>>> warned from the FEs by simply extending the existing Wnonnull. >>>> >>>> Bootstrapped and regression tested on x86_64-linux-gnu. >>>> >>>> OK? >>>> >>>> >>>> gcc/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez >>>> >>>> PR c/16351 >>>> * doc/invoke.texi (Wnonnull): Document behavior for >>>> returns_nonnull. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez >>>> >>>> PR c/16351 >>>> * c-c++-common/wnonnull-1.c: New test. >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez >>>> >>>> PR c/16351 >>>> * typeck.c (check_return_expr): Call >maybe_warn_returns_nonnull. >>>> >>>> >>>> gcc/c-family/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez >>>> >>>> PR c/16351 >>>> * c-common.c (maybe_warn_returns_nonnull): New. >>>> * c-common.h (maybe_warn_returns_nonnull): Declare. >>>> >>>> gcc/c/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez >>>> >>>> PR c/16351 >>>> * c-typeck.c (c_finish_return): Call >maybe_warn_returns_nonnull. >>> FWIW, we have the usual tension here between warning in the >front-end >>> vs >>> warning after optimization and exploiting dataflow analysis. >>> >>> Warning in the front-ends like this can generate false positives >(such >>> as a NULL return in an unreachable path and miss cases where the >NULL >>> has to be propagated into the return by later optimizations. >>> >>> However warning in the front-ends will tend to have more stable >>> diagnostics from release to release. >>> >>> Warning after optimization/analysis will tend to generate fewer >false >>> positives and can pick up cases where the didn't explicitly appear >in >>> the return statement, but had to be propagated in by the optimizers. >Of >>> >>> course, these warnings are less stable release-to-release and >require >>> the optimizers/analysis phases to be run. >>> >>> I've always preferred exploiting optimization and analysis to both >>> reduce false positives and expose the non-trivial which show up via >>> optimizations. But I also understand that's simply my preference >and >>> that others have a different preference. >>> >>> I'll tentatively approve for the trunk, but I think we still want >>> warnings after optimization/analysis. Which will likely lead to a >>> scheme like I proposed many years for uninitialized variables where >we >>> have multiple modes. One warns in the front-end like your >implemention >>> >>> does, the other defers the warning until after analysis & >optimization. >>> >>> So please keep 16351 open after committing. >> >> -W{no-,}{f,m}e IIRC was proposed before. Won't help >https://gcc.gnu.org/PR55035 though where struct stores just escape too >much -- AFAIU -- but still.. >> >Things like uninitialized variable analysis are inherently going to >cause false positives. It's just a fact of life. > >Looking at the reduced testcase in that PR, I'm pretty sure its a bogus > >reduction. Yes, the original, smallish test case in comment #4 is different, AFAICS. > >We could have N == 0 when we encounter the head of the first loop. So >no members of temp[] will be initialized. We then have the call to >bar() which could change the value of N to 1. We then hit the second >loop and read temp[0] which is uninitialized. The warning for the >reduced testcase is correct. Agree. > >If the original source has the same overall structure (an unbound write > >between the key loops), then the warning is correct. If the writes can > >be proven to not clobber the loop bounds (such that the two key loops >iterate over the same number of elements), then we'd have a false >positive warning (and a failure of jump threading to discover the >unexecutable path). AFAIU the write in the testcase in comment #4 does not clobber loop bounds, so I think the warning is wrong. Thanks,