From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124896 invoked by alias); 24 Jul 2015 22:17:41 -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 124885 invoked by uid 89); 24 Jul 2015 22:17:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-oi0-f46.google.com Received: from mail-oi0-f46.google.com (HELO mail-oi0-f46.google.com) (209.85.218.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 24 Jul 2015 22:17:40 +0000 Received: by oige126 with SMTP id e126so26809154oig.0 for ; Fri, 24 Jul 2015 15:17:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=UxY3ySeWYKx8DAorS/71EROpo5KlGPdp7CflQD4jkM0=; b=Wz0uQrz1wII00I0/a2Nc5bCs7RCtHBC08GUEM1x1XCxEpP6BtNh+50T5Vbjco8eATx +Dom9zcY/GyLo3t88+zQlDc0llHDQC5GhKdy6iP8szkNQPrWSbxsr9eGxQzwTlUQ+NFL y/L+YJUpMCdB6qUTDTV7VR0tsNsllCOyo4HuVKiqZfNz2v/EZgqg2DHKXUvAM6UJIKMo d2kcCIqpcN7ocBT1YUOs5pS7Nr8tUEnnmVg9AZD+CFFS8RllJXP8DRu28dW+D0vgYO0s jRy7abfw0OJcWrFKu7z/bauedEhcbqiPzoD12lixgpjirBxIFH9n9NFwzzz+rZnZbKpQ 8uiQ== X-Gm-Message-State: ALoCoQkmMQDdqrj+st/8wX6LPy6kcbEjSLtuqnLvxq/r9siERTRGAjO6ud6/Iz2d6i9B5QpOoKRs X-Received: by 10.202.75.13 with SMTP id y13mr16789746oia.92.1437776257470; Fri, 24 Jul 2015 15:17:37 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.56.202 with HTTP; Fri, 24 Jul 2015 15:17:18 -0700 (PDT) In-Reply-To: References: <55B127D7.3020202@redhat.com> <55B29270.1030206@redhat.com> From: Patrick Palka Date: Fri, 24 Jul 2015 22:26:00 -0000 Message-ID: Subject: Re: PR c/16351 Extend Wnonnull for returns_nonnull To: =?UTF-8?B?TWFudWVsIEzDs3Blei1JYsOhw7Fleg==?= Cc: Jeff Law , Gcc Patch List , Jason Merrill , "Joseph S. Myers" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-07/txt/msg02126.txt.bz2 On Fri, Jul 24, 2015 at 5:55 PM, Manuel L=C3=B3pez-Ib=C3=A1=C3=B1ez wrote: > On 24 July 2015 at 21:30, Jeff Law wrote: >> On 07/24/2015 07:45 AM, Manuel L=C3=B3pez-Ib=C3=A1=C3=B1ez wrote: >>> >>> On 23 July 2015 at 19:43, Jeff Law wrote: >>>> >>>> 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. >>> >>> >>> False positives (for the warning as proposed right now) would be >>> strange, since it would mean that a returns_nonnull function returns >>> an explicit NULL in some code-path that is not meant to be executed. >>> That sounds like a bug waiting to happen. >> >> Depends on how you choose to look at things. It's quite common via macr= os & >> such to have unexecutable/unreachable paths. Whether or not to warn abo= ut >> something found on such a path is a matter of personal preference. > > I think it is also a matter of the particular warning and on the > balance of true positives vs. false positives in typical code-bases. > In this case, returning NULL in any code-path from a returns_nonnull > function, even if the path is currently unreachable via some macro > configuration, seems a bad idea. Of course, I'm open to be proven > wrong :-) > >>> Moreover, this warning works in exactly the same cases as >>> __attribute__((nonnull)) does for function arguments, and so far those >>> haven't been a source of false positives. >> >> I'm sure I could write one ;-) And given that a FE based version of this >> will only catch explicit NULLs in argument lists, I consider it so weak = as >> to be virtually useless. > > Well, it is catching exactly all the cases that you were testing for > in your original -Wnull-attribute patch ;-) > >>> I'm very much in favour of this, but only for things that cannot >>> reliably be warned from the FE. Clang has shown that it is possible to >>> improve much more the accuracy of warnings in the FE and still compile >>> faster than GCC by performing some kind of fast CCP (and VRP?) in the >>> FE (or make the CCP and VRP passes occur earlier and even without >>> optimization): >> >> And my assertion is that for things like we're discussing, you need the >> analysis & optimizations both to expose the non-trivial cases and prune = away >> those which are false positives. I consider exposing the non-trivial ca= ses >> and pruning away false positives the most important aspect of this kind = of >> work. > > Based on my experience, I'm not convinced that moving warnings to the > middle-end is a good idea. The middle-end does a very poor job > keeping sensible locations when doing transformations and it will not > only introduce false positives, it will also remove true positives. > The diagnostics often refer to the wrong variable or code that is not > what the user originally wrote, which makes very hard to understand > the problem. One only has to read all the reports we have about > -Warray-bounds, -Wuninitialized, -Wstrict-overflow and other > middle-end warnings. > > For example, Clang is currently able to warn about the following case > without any optimization, while GCC cannot at any optimization level: > > int f(bool b) { > int n; > if (b) > n =3D 1; > return n; > } Is there a PR for this particular test case? I am interested in improving the uninit analysis for gcc 6 so this potentially seems up my alley.