From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38559 invoked by alias); 3 Aug 2017 08:46:03 -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 38259 invoked by uid 89); 3 Aug 2017 08:45:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy= X-HELO: mail-qk0-f194.google.com Received: from mail-qk0-f194.google.com (HELO mail-qk0-f194.google.com) (209.85.220.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Aug 2017 08:45:44 +0000 Received: by mail-qk0-f194.google.com with SMTP id d136so626576qkg.3 for ; Thu, 03 Aug 2017 01:45:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Hcyf2mPD4jrTv1QPPm7IDo26pmu1s0Oz+ok1pzeZt04=; b=f1CY9VkBj+O4/P0jvQ9qJgQzu8iXEkD2uL1b+J2rjDOIU2Pdi0O4WptEtFoeOSihTB T0utv70YVbl7O1Oi+Xkk7WIaTfdht0wMTrds5MjrplSoVVG5RnlPcg/pdSfC+YXwClm+ nb0odTPWEh/v6uRcUKIEXD7LA/G9IIKCsofG8bOdk07+Z7XL3XKMoujAY7jL/7dCKu3Q 6sLMQfwL0s8sgh+26KHzZGzgp/dd2XM4YiHeRduAJk8QmmXTrepUNIouUcyLDHnYsIua XhAlHVKqFszLeNyE2w0Dl4KEC41nL6Xnjhb3m3ClRfk3yszEygjpvKiamDzpZQhhooCs vEJA== X-Gm-Message-State: AHYfb5izNTNSDQ15glFynoivSNaKcyW58LV4Yvl4JlY6WDHJGxb4AAbD Vehfw5qYb1ggpSt1X3GwsS0yU8O7xQ== X-Received: by 10.55.197.88 with SMTP id p85mr1262865qki.281.1501749940830; Thu, 03 Aug 2017 01:45:40 -0700 (PDT) MIME-Version: 1.0 Received: by 10.140.43.35 with HTTP; Thu, 3 Aug 2017 01:45:40 -0700 (PDT) In-Reply-To: References: <8a0de82f-d68e-9c37-0406-f216e3f92884@gmail.com> From: Richard Biener Date: Thu, 03 Aug 2017 08:46:00 -0000 Message-ID: Subject: Re: [PING #2] [PATCH] enhance -Wrestrict to handle string built-ins (PR 78918) To: Jeff Law Cc: Martin Sebor , Gcc Patch List Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00267.txt.bz2 On Wed, Aug 2, 2017 at 7:10 PM, Jeff Law wrote: > On 08/01/2017 03:25 AM, Richard Biener wrote: >> On Tue, Aug 1, 2017 at 11:23 AM, Richard Biener >> wrote: >>> On Tue, Aug 1, 2017 at 4:27 AM, Martin Sebor wrote: >>>> Richard, >>>> >>>> in discussing this work Jeff mentioned that your comments on >>>> the tree-ssa-alias.c parts would be helpful. When you have >>>> a chance could you please give it a once over and let me know >>>> if you have any suggestions or concerns? There are no visible >>>> changes to existing clients of the pass, just extensions that >>>> are relied on only by the new diagnostics. >>>> >>>> https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01264.html >>>> >>>> I expect to revisit the sprintf %s patch mentioned below and >>>> see how to simplify it by taking advantage of the changes >>>> implemented here. >>> >>> Your ptr_deref_alias_decl_p returns true when must_alias is true >>> but there is no must-alias relationship. >>> >>> The ao_ref_init_from_ptr_and_size doesn't belong there. >>> >>> I dislike all of the changes related to returning an alias "size". >>> >>> Please keep away from the alias machinery. >>> >>> Warning during folding is similarly bad design. Please don't add >>> more such things. >>> >>> Thanks for putting this on my radar though. >>> Richard. >> >> Oh, for a constructive comment this all feels like sth for either >> sanitizers or a proper static analysis tool. As I outlined repeatedly >> I belive GCC could host a static analysis tool but it surely should >> not be interwinded into it's optimization passes or tools. > I disagree strongly here. > > Sanitiers catch problems after the fact and only if you've compiled your > code with sanitization and manage to find a way to trigger the problem path. > > Other static analysis tools are useful, but they aren't an integral part > of the edit, compile, debug cycle and due to their cost are often run > long after code was committed. > > Finding useful warnings that can be issued as part of the compile, edit, > debug cycle is, IMHO, far more useful than sanitizers or independent > static checkers. > > -- > > > I think finding a way to exploit information that our various analyzers > can provide to give precise warnings is a good thing. So it seemed > natural that if we wanted to ask a must-alias question that we should be > querying the alias oracle rather than implementing that kind of query > within the sprintf warnings. I'm not sure why you'd say "Please keep > away from the alias machinery". > > I'm a little confused here... Well, simply because the way as implemented isn't a must-alias query but maybe one that's good enough for warnings (reduces false positives but surely doesn't eliminate them). There's a must alias query already, in stmt_kills_ref_p. It's a matter of refactoring to make a refs_must_alias_p. Then propose that "overlap range" stuff separately. But I'm against lumping this all in as an innocent change suggesting the machinery can do sth (must alias) when it really can't. I also do not like adding a "must alias" bool to the may-alias APIs as the implementation is fundamentally may-alias, must-alias really is very different. And to repeat, no, I do not want a "good-enough-for-warnings" must-alias in an API that's supposed to be used by optimizations where "good enough" is not good enough. Richard. > > > Jeff