From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 65314 invoked by alias); 18 Mar 2016 17:04:05 -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 65289 invoked by uid 89); 18 Mar 2016 17:04:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=faithful, sk:maybe_n, promising, factored X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 18 Mar 2016 17:04:02 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 6DEEB3DB; Fri, 18 Mar 2016 17:04:01 +0000 (UTC) Received: from slagheap.utah.redhat.com (ovpn-113-164.phx2.redhat.com [10.3.113.164]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2IH40VI011150; Fri, 18 Mar 2016 13:04:00 -0400 Subject: Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression To: Martin Sebor , Gcc Patch List , Jason Merrill References: <56E72C33.8000301@gmail.com> <56E9B9C3.5000908@redhat.com> <56EB1ECA.90800@gmail.com> Cc: Marek Polacek , Jakub Jelinek From: Jeff Law Message-ID: <56EC3500.4030701@redhat.com> Date: Fri, 18 Mar 2016 17:48:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56EB1ECA.90800@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg01104.txt.bz2 On 03/17/2016 03:16 PM, Martin Sebor wrote: >>> static tree cxx_eval_constant_expression (const constexpr_ctx *, tree, >>> - bool, bool *, bool *, tree * = NULL); >>> + bool, bool *, bool *, bool * = NULL, >>> + tree * = NULL); >> I didn't look deeply, but do you end up fixing all (most) of the callers >> of cxx_eval_constant_expression? If so, then you don't need the default >> initialization. > > Thanks for the comments. The patch only modifies about 10 out > of the 70 or so calls to the function in the file and the default > argument helps avoid making the rest of the changes. (I have some > ideas for improving the APIs of these functions that I'd like to > run by Jason when we're done with the 6.0 work.) OK. Then let's keep the default initialization. > > The difficulty I've run into with detecting these problems in later > phases is that some invalid expressions have already been simplified > by the front end. The example that applies here (even though this > is still the front end) is this: Yea. I was hoping that the delayed folding work would be helping in getting a more faithful representation out of the front-ends. > > constexpr int* p = 0; > constexpr bool b0 = &p[0] == 0; // accepted > constexpr bool b1 = &p[1] == 0; // rejected > > Both b0 and b1 are invalid and should be diagnosed, but only b1 > is. b1 isn't because because by the time we see its initializer > in constexpr.c it's been transformed into the equivalent of "b1 > = (int*)ps" (though we don't see the cast which would also make > it invalid). > > But if we can avoid these early simplifying transformations and > retain a more faithful representation of the original source then > doing the checking later will likely be simpler and result in > detecting more problems with greater consistency and less effort. Do we know where the folding is happening for this case and is it something we can reasonably defer? ie, is this just a case we missed as part of the deferred folding work and hence should have its own distinct BZ to track? >> Hmm, I thought we already had code to do this somewhere. It looks like >> it's moved around quite a bit. I think you want to be using >> symtab_node::nonzero_address to determine if a given symbol must bind to >> a nonzero address. > > Thanks for the hint. I had looked for existing functions but > couldn't find one that worked. decl_with_nonnull_addr_p() in > c-common.c looked promising but it's not accessible here and > it doesn't do the right thing when HAS_DECL_ASSEMBLER_NAME_P() > is false (it ICEs). Yea, I found the same mis-mash of bits that didn't look directly usable for the problem you're tackling. What's odd is I would have sworn that we had code to do exactly what you wanted, but I wasn't able to find it, either as a distinct routine or open-coded. > > In the end, I added a new function, maybe_nonzero_address(), > that calls symtab_node::nonzero_address(), and that I factored > out of tree_single_nonzero_warnv_p() that I was then able to > use in fold_comparison(). Sounds good. > > I've made a few other small adjustments to the patch to avoid > one false positive, and a few test cases, and tweak the expected > diagnostics now that Marek has fixed 70194. > > I've also wrote myself a small sed script to replace blocks of > 8 spaces with tabs and ran the patch through it. I'll integrate > it into my workflow so I hopefully don't have to worry about this > ever again. I'll try to take a look at the updated patch shortly. It may still hit too much of the C++ front-end for me to be comfortable reviewing -- we'll see. jeff