From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72017 invoked by alias); 30 Oct 2017 15:19:35 -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 71983 invoked by uid 89); 30 Oct 2017 15:19:35 -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=HX-HELO:sk:mail-io X-HELO: mail-io0-f172.google.com Received: from mail-io0-f172.google.com (HELO mail-io0-f172.google.com) (209.85.223.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 15:19:28 +0000 Received: by mail-io0-f172.google.com with SMTP id n137so27991636iod.6 for ; Mon, 30 Oct 2017 08:19:28 -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=ze1aoZ/eI8ZcncMSmFCBjB0DRZ8+JV/8wLn8TwdyQ84=; b=EnJUZq9+ah+KX5iVCxWuduDnm06dTWLxk1erlGaCS4N9209OonhLqcPY17C2alHYEv PVQUgYUen0XDpIPIwN0ZzIE2Q+fJCSZMXaAlg0hJbfsLE+4A9ltbQJx/9eOk61qEnySz NECW1OIFgqJ6EyXavYENq4jvfsZI8JJMnRUY0JGljjt2ZNSlMWpMsLSaiprX3zLIMbIJ 1AW2oRhDpWUTKl0+oFKGX+Pea+NFa2ZBY8LiCFHtKT+Zee0sB1uz/MTo/tGvOvLMoZiY EXBD8GdQiFlPm4s+vuEyBnx0Uv9dT07QTUYJq/C0ajHw3h2eg89ie8U17v9yW0ve1cSt P+Fw== X-Gm-Message-State: AMCzsaWe4FsLeubKWRxGzTyFDAnqDbykWaq7ASbnrylAG9jzgDwGry9a IpIRUlFP+zKGIpzm97/9YG4= X-Google-Smtp-Source: ABhQp+TRIvqVVdACDES25jrlyNIRBjhOJetOpCwFKlqNCmpk9LDBOH/XefD3GHKw7yAqEHKn0khNlA== X-Received: by 10.36.40.132 with SMTP id h126mr6403332ith.24.1509376766990; Mon, 30 Oct 2017 08:19:26 -0700 (PDT) Received: from [172.20.33.7] ([67.201.107.102]) by smtp.gmail.com with ESMTPSA id e81sm6233059ioi.15.2017.10.30.08.19.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 30 Oct 2017 08:19:26 -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: From: Martin Sebor Message-ID: <79634da6-bf31-b7f0-15f5-0436fc21a51a@gmail.com> Date: Mon, 30 Oct 2017 15:21: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/msg02215.txt.bz2 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.) 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'; >> ~~~~^~~