From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20484 invoked by alias); 30 Oct 2017 21:49: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 20447 invoked by uid 89); 30 Oct 2017 21:49:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=pulling, improving, origins 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 ESMTP; Mon, 30 Oct 2017 21:49:01 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 63D812020F; Mon, 30 Oct 2017 21:49:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 63D812020F Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=law@redhat.com Received: from localhost.localdomain (ovpn-112-25.phx2.redhat.com [10.3.112.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id B9AB560605; Mon, 30 Oct 2017 21:48:59 +0000 (UTC) Subject: Re: [PATCH] enhance -Warray-bounds to detect out-of-bounds offsets (PR 82455) To: Martin Sebor , Richard Biener Cc: Gcc Patch List References: <79634da6-bf31-b7f0-15f5-0436fc21a51a@gmail.com> From: Jeff Law Message-ID: <928a519a-63fd-4e90-8e8f-fcc829d741a2@redhat.com> Date: Mon, 30 Oct 2017 22:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <79634da6-bf31-b7f0-15f5-0436fc21a51a@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg02256.txt.bz2 On 10/30/2017 09:19 AM, Martin Sebor wrote: > On 10/30/2017 05:45 AM, Richard Biener wrote: >> On Sun, 29 Oct 2017, Martin Sebor wrote: >> >>> In my work on -Wrestrict, to issue meaningful warnings, I found >>> it important to detect both out of bounds array indices as well >>> as offsets in calls to restrict-qualified functions like strcpy. >>> GCC already detects some of these cases but my tests for >>> the enhanced warning exposed a few gaps. >>> >>> The attached patch enhances -Warray-bounds to detect more instances >>> out-of-bounds indices and offsets to member arrays and non-array >>> members.  For example, it detects the out-of-bounds offset in the >>> call to strcpy below. >>> >>> The patch is meant to be applied on top posted here but not yet >>> committed: >>>    https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01304.html >>> >>> Richard, since this also touches tree-vrp.c I look for your comments. >> >> You fail to tell what you are changing and why - I have to reverse >> engineer this from the patch which a) isn't easy in this case, b) feels >> like a waste of time.  Esp. since the patch does many things. >> >> My first question is why do you add a warning from forwprop?  It >> _feels_ like you're trying to warn about arbitrary out-of-bound >> addresses at the point they are folded to MEM_REFs.  And it looks >> like you're warning about pointer arithmetic like &p->a + 6. >> That doesn't look correct to me.  Pointer arithmetic in GIMPLE >> is not restricted to operate within fields that are appearantly >> accessed here - the only restriction is with respect to the >> whole underlying pointed-to-object. >> >> By doing the warning from forwprop you'll run into all such cases >> introduced by GCC itself during quite late optimization passes. > > I haven't run into any such cases.  What would a more appropriate > place to detect out-of-bounds offsets?  I'm having a hard time > distinguishing what is appropriate and what isn't.  For instance, > if it's okay to detect some out of bounds offsets/indices in vrp > why is it wrong to do a better job of it in forwpropI think part of the problem is there isn't a well defined place to do this kind of warning. I suspect it's currently in VRP simply because that is where we had range information in the past. It's still the location with the most accurate range information. In a world where we have an embedded context sensitive range analysis engine, we should *really* look at pulling the out of bounds array warnings out of any optimization pass an have a distinct pass to deal with them. I guess in the immediate term the question I would ask Martin is what is it about forwprop that makes it interesting? Is it because of the lowering issues we touched on last week? If so I wonder if we could recreate an array form from a MEM_REF for the purposes of optimization. Or if we could just handle MEM_REFs better within the existing warning. > >> >> You're trying to re-do __builtin_object_size even when that wasn't >> used. > > That's not the quite my intent, although it is close. Wouldn't we be better off improving _b_o_s? > >> >> So it looks like you're on the wrong track.  Yes, >> >>   strcpy (p->a + 6, "y"); >> >> _may_ be "invalid" C (I'm not even sure about that!) but it >> is certainly not invalid GIMPLE. > > Adding (or subtracting) an integer to/from a pointer to an array > is defined in both C and C++ only if the result points to an element > of the array or just past the last element of the array.  Otherwise > it's undefined. (A non-array object is considered an array of one > for this purpose.) I think Richi's argument is that gimple allows things that are not necessarily allowed by the C/C++ standard. For example we support virtual origins from Ada, which internally would look something like invalid C code OTOH, we currently have code in tree-vrp.c which warns if we compute the address of an out of bounds array index which is very C/C++ centric. jeff