From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84345 invoked by alias); 30 Oct 2017 20:13:11 -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 84332 invoked by uid 89); 30 Oct 2017 20:13:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy= X-HELO: mail-oi0-f52.google.com Received: from mail-oi0-f52.google.com (HELO mail-oi0-f52.google.com) (209.85.218.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 20:13:08 +0000 Received: by mail-oi0-f52.google.com with SMTP id g125so24146549oib.12 for ; Mon, 30 Oct 2017 13:13:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=STQWQ7q7ZT6HhotSN93kD6icRL1edU5M9gpbSuIGzeg=; b=SrnZdRHq0urIE2G7xcuiXlwUypJzagpGKgUSkO2L1ixotH+bPenV+ROUBE9goQIbaV 7FaM/g1a1FJf5rE7isBSIlV5ikhAPJyCv6rfayG3SYQjXYs/VyK/uGGSjG8b64XPPYfW VKSsVBWuggt+mqnsDkT+cwkQZ55XJWsQtVtkPgk2Hx3zLTlyyYeSg9aiFI0G7j7V0p+G 2tOUepagpt89q3L9kKPoiSQrd0ynPQEFKq2YzRIeX+pVwBSt/ts/hCK9dKjKKEnmLJOB 3Z7ThUGSLJKOLNfCIEvwuUr7E0Sl008BArRavcnECVrJ13OHavVd1obIEWqhnc/qmVVX 5vmQ== X-Gm-Message-State: AMCzsaXHFqa8ComQYHwcoUU45bJpfKNlCIyw/khQsx34oG/NxySh/4tB BIlp/cWErsrchU+nu9MTERE= X-Google-Smtp-Source: ABhQp+QX4ssnM2MHh9Livv6+R81UW/Gi2GZfxQ9GWG8sHlsTmW27qI4c59XJCz3ywqkncfoWT9eu4A== X-Received: by 10.157.64.5 with SMTP id m5mr5980945ote.93.1509394386949; Mon, 30 Oct 2017 13:13:06 -0700 (PDT) Received: from [172.20.33.7] ([67.201.107.102]) by smtp.gmail.com with ESMTPSA id p82sm6316547oih.9.2017.10.30.13.13.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 30 Oct 2017 13:13:06 -0700 (PDT) Subject: Re: [PATCH] enhance -Warray-bounds to detect out-of-bounds offsets (PR 82455) To: Richard Biener Cc: Gcc Patch List , Jeff Law References: <79634da6-bf31-b7f0-15f5-0436fc21a51a@gmail.com> From: Martin Sebor Message-ID: Date: Mon, 30 Oct 2017 20:40:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg02251.txt.bz2 On 10/30/2017 01:53 PM, Richard Biener wrote: > On October 30, 2017 4:19:25 PM GMT+01:00, 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 forwprop? >> >>> >>> 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. >> >>> >>> 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.) > > On GIMPLE this is indistinguishable from (short *) (p->a) + 3. Sure, they're both the same: pa_3 = &p_2(D)->a; _1 = pa_3 + 6; and that's fine because the implementation of the warning sees and uses the byte offset from the beginning of a, so I don't understand the problem you are pointing out. Can you clarify what you mean? Martin > > GIMPLE is neither C nor C++. > > Richard. > >> >> Martin >> >>> >>> Richard. >>> >>>> Jeff, this is the enhancement you were interested in when we spoke >>>> last week. >>>> >>>> Thanks >>>> Martin >>>> >>>> $ cat a.c && gcc -O2 -S -Wall a.c >>>> struct A { char a[4]; void (*pf)(void); }; >>>> >>>> void f (struct A *p) >>>> { >>>> p->a[5] = 'x'; // existing -Warray-bounds >>>> >>>> strcpy (p->a + 6, "y"); // enhanced -Warray-bounds >>>> } >>>> >>>> a.c: In function ‘f’: >>>> a.c:7:3: warning: offset 6 is out of bounds of ‘char[4]’ >> [-Warray-bounds] >>>> strcpy (p->a + 6, "y"); >>>> ^~~~~~~~~~~~~~~~~~~~~~ >>>> a.c:1:17: note: member declared here >>>> struct A { char a[4]; void (*pf)(void); }; >>>> ^ >>>> a.c:5:7: warning: array subscript 5 is above array bounds of >> ‘char[4]’ >>>> [-Warray-bounds] >>>> p->a[5] = 'x'; >>>> ~~~~^~~ >