From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2451 invoked by alias); 30 Oct 2017 19:53:20 -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 2440 invoked by uid 89); 30 Oct 2017 19:53:20 -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_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 19:53:18 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 15CD4AD51; Mon, 30 Oct 2017 19:53:15 +0000 (UTC) Date: Mon, 30 Oct 2017 19:59:00 -0000 User-Agent: K-9 Mail for Android In-Reply-To: <79634da6-bf31-b7f0-15f5-0436fc21a51a@gmail.com> References: <79634da6-bf31-b7f0-15f5-0436fc21a51a@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [PATCH] enhance -Warray-bounds to detect out-of-bounds offsets (PR 82455) To: Martin Sebor CC: Gcc Patch List ,Jeff Law From: Richard Biener Message-ID: X-SW-Source: 2017-10/txt/msg02248.txt.bz2 On October 30, 2017 4:19:25 PM GMT+01:00, Martin Sebor w= rote: >On 10/30/2017 05:45 AM, Richard Biener wrote: >> On Sun, 29 Oct 2017, Martin Sebor wrote: >>=20 >>> 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. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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? > >>=20 >> 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. > >>=20 >> So it looks like you're on the wrong track. Yes, >>=20 >> strcpy (p->a + 6, "y"); >>=20 >> _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. GIMPLE is neither C nor C++.=20 Richard.=20 > >Martin > >>=20 >> Richard. >>=20 >>> 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] =3D 'x'; // existing -Warray-bounds >>> >>> strcpy (p->a + 6, "y"); // enhanced -Warray-bounds >>> } >>> >>> a.c: In function =E2=80=98f=E2=80=99: >>> a.c:7:3: warning: offset 6 is out of bounds of =E2=80=98char[4]=E2= =80=99 >[-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 >=E2=80=98char[4]=E2=80=99 >>> [-Warray-bounds] >>> p->a[5] =3D 'x'; >>> ~~~~^~~