From patchwork Mon Nov 4 22:05:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1189201 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-512366-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="uuVzcjxA"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="a2a73OL5"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 476RgH6ZXPz9sTX for ; Tue, 5 Nov 2019 09:05:14 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=nw55fp+EjgdNMV/pCWWfYKHv+d6B346F41S1SBHCg6GLUICf2F vDc3Rry/gv0Ax+2nrYGSwXYUFz+C8Z+Fkxc1YFrlKPQW52Qx2Uk0zD6JshFmdZ1D gD/Zb5gBtdHofSz5bHwjjn0MJatv9QRGp+qYaOLokhkeZYrA5eGGRplCY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=y5lg/ov6b+KUPlnj89EXrjnVg70=; b=uuVzcjxAM1OS2AnpGEbg l5k2aqBweLVQ7bYQFOVqhhXTyDD52IUPYMebiiBTQoD+v1P+YL5fo19LFeRdRVkr MoN/hZeinBzU2mAJjHpcxUMD7S6XD9XlBeOaLcmlumNYbveZh3xF47ixNzi3v0i9 R3f95IJ8+C7L0vNJ6prkwqM= Received: (qmail 44375 invoked by alias); 4 Nov 2019 22:05:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 44367 invoked by uid 89); 4 Nov 2019 22:05:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=interior X-HELO: mail-yb1-f174.google.com Received: from mail-yb1-f174.google.com (HELO mail-yb1-f174.google.com) (209.85.219.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Nov 2019 22:05:04 +0000 Received: by mail-yb1-f174.google.com with SMTP id 4so8519853ybq.9 for ; Mon, 04 Nov 2019 14:05:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=zJRu+J9AoWdbZWf3IPJhOp0PqCyHSCsN9oKWkvmxJ74=; b=a2a73OL5CdapUAd6PSKFA1MsQuRsoPf4t0pm2FFEBYR24lRtdk2zHLFqspebw9MPRs PkN/s10LqzX99hmCUsCr2z026Nun9verEYh0JGv2WycQPPE4G1MeaSA0X+al3EOnQ6lG UdfLNwmcfvOTuSfjS6/tw+6rAllsaBHQCxdVu0xtuLveroXc1jrltR6lqLv9JechxVos vFMNNpIARZ2nnnJ/i8E6oWbThbA6H5eYgc2nbQbCMGSQGWmdpsy3UN/zLvNt2sgpFoGZ /O3stb04Ca9vS4iBGtRlFaKQoAZRz0Y8AbfycWOW/p43WNS3qdXfvk6avbSVljBNXaQV GrQA== Received: from [192.168.0.41] (97-118-98-145.hlrn.qwest.net. [97.118.98.145]) by smtp.gmail.com with ESMTPSA id x78sm874600ywg.108.2019.11.04.14.05.01 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Nov 2019 14:05:01 -0800 (PST) To: gcc-patches From: Martin Sebor Subject: [PATCH] avoid folding of invalid indices to compound literals (PR 92341) Message-ID: Date: Mon, 4 Nov 2019 15:05:00 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 X-IsSubscribed: yes While testing some other changes I noticed that -Warray-bounds fails to detect out-of-bounds indices to compound literals such as in: int *p = (int[]){ 1, 2, 3 }; // ... p[3] = 7; This is because SRA transforms such references into accesses to uninitialized scalar variables and also sets the TREE_NO_WARNING bit for the replacement variables. This prevents -Wuninitialized from detecting such bugs, although that wouldn't be the right warning to issue in these cases). The attached patch tweaks SRA to avoid this transformation when the access is out of the bounds of the referenced variable. That in turn lets -Warray-bounds diagnose these invalid accesses. The patch also adjusts -Warray-bounds to reference to correct index and message and issue the warning even for zero-length compound literal arrays. This was exposed and the fix is relied on by the test I wrote for the compound literals. Finally, the change also corrects an oversight of mine from some time ago in failing to handle out-of-bounds indices relative to addresses of function parameters. This is a trivial one-line tweak that could be submitted separately but it doesn't seem worth the overhead. Tested on x86_64-linux. Martin PR middle-end/92341 - missing -Warray-bounds indexing past the end of a compound literal PR middle-end/82612 - missing -Warray-bounds on a non-zero offset from the address of a non-array object gcc/testsuite/ChangeLog: PR middle-end/92341 PR middle-end/82612 * gcc.dg/Warray-bounds-50.c: New test. * gcc.dg/Warray-bounds-51.c: New test. gcc/ChangeLog: PR middle-end/92341 PR middle-end/82612 * tree-sra.c (get_access_for_expr): Fail for out-of-bounds offsets. * tree-vrp.c (vrp_prop::check_array_ref): Correct index and text of message printed in a warning for empty arrays. (vrp_prop::check_mem_ref): Also handle function parameters and empty arrays. diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-50.c b/gcc/testsuite/gcc.dg/Warray-bounds-50.c new file mode 100644 index 00000000000..c27e6a494f3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Warray-bounds-50.c @@ -0,0 +1,96 @@ +/* PR middle-end/92341 - missing -Warray-bounds indexing past the end + of a compound literal + { dg-do compile } + { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */ + +#include "range.h" + +#define INT_MAX __INT_MAX__ +#define INT_MIN (-__INT_MAX__ - 1) + +void sink (int, ...); + + +#define T(...) sink (__LINE__, (__VA_ARGS__)) + + +void direct_idx_cst (void) +{ + T ((int[]){ }[-1]); // { dg-warning "array subscript -1 is outside array bounds of 'int\\\[0]'" } + T ((int[]){ }[0]); // { dg-warning "array subscript 0 is outside array bounds of 'int\\\[0]'" } + T ((int[]){ }[1]); // { dg-warning "array subscript 1 is outside array bounds of 'int\\\[0]'" } + + T ((int[]){ 1 }[-1]); // { dg-warning "array subscript -1 is below array bounds of 'int\\\[1]'" } + T ((int[]){ 1 }[0]); + T ((int[]){ 1 }[1]); // { dg-warning "array subscript 1 is above array bounds of 'int\\\[1]'" } + T ((int[]){ 1 }[INT_MIN]); // { dg-warning "array subscript -\[0-9\]+ is below array bounds of 'int\\\[1]'" } + T ((int[]){ 1 }[INT_MAX]); // { dg-warning "array subscript \[0-9\]+ is above array bounds of 'int\\\[1]'" } + T ((int[]){ 1 }[SIZE_MAX]); // { dg-warning "array subscript \[0-9\]+ is above array bounds of 'int\\\[1]'" } +} + + +void direct_idx_var (int i) +{ + T ((char[]){ }[i]); // { dg-warning "array subscript i is outside array bounds of 'char\\\[0]'" } + T ((int[]){ }[i]); // { dg-warning "array subscript i is outside array bounds of 'int\\\[0]'" } +} + + +void direct_idx_range (void) +{ + ptrdiff_t i = SR (-2, -1); + + T ((int[]){ 1 }[i]); // { dg-warning "array subscript \[ \n\r]+ is outside array bounds of 'int\\\[0]'" "pr?????" { xfail *-*-* } } +} + + +#undef T +#define T(idx, ...) do { \ + int *p = (__VA_ARGS__); \ + sink (p[idx]); \ + } while (0) + +void ptr_idx_cst (void) +{ + T (-1, (int[]){ }); // { dg-warning "array subscript -1 is outside array bounds of 'int\\\[0]'" } + T ( 0, (int[]){ }); // { dg-warning "array subscript 0 is outside array bounds of 'int\\\[0]'" } + T (+1, (int[]){ }); // { dg-warning "array subscript 1 is outside array bounds of 'int\\\[0]'" } + + T (-1, (int[]){ 1 }); // { dg-warning "array subscript -1 is outside array bounds of 'int\\\[1]'" } + T ( 0, (int[]){ 1 }); + T (+1, (int[]){ 1 }); // { dg-warning "array subscript 1 is outside array bounds of 'int\\\[1]'" } + T (INT_MIN, (int[]){ 1 }); // { dg-warning "array subscript -\[0-9\]+ is outside array bounds of 'int\\\[1]'" } + T (INT_MAX, (int[]){ 1 }); // { dg-warning "array subscript \[0-9\]+ is outside array bounds of 'int\\\[1]'" } + T (SIZE_MAX, (int[]){ 1 }); // { dg-warning "array subscript -?\[0-9\]+ is outside array bounds of 'int\\\[1]'" } +} + + +void ptr_idx_var (int i) +{ + T (i, (int[]){ }); // { dg-warning "array subscript \[^\n\r\]+ is outside array bounds of 'int\\\[0]'" } + T (i, (int[]){ 1 }); + T (i, (int[]){ i, 1 }); +} + +void ptr_idx_range (void) +{ + ptrdiff_t i = SR (-2, -1); + + T (i, (int[]){ }); // { dg-warning "array subscript \\\[-2, -1] is outside array bounds of 'int\\\[0]'" } + T (i, (int[]){ 1 }); // { dg-warning "array subscript \\\[-2, -1] is outside array bounds of 'int\\\[1]'" } + T (i, (int[]){ i }); // { dg-warning "array subscript \\\[-2, -1] is outside array bounds of 'int\\\[1]'" } + + i = SR (0, 1); + + T (i, (int[]){ }); // { dg-warning "array subscript \\\[0, 1] is outside array bounds of 'int\\\[0]'" } + T (i, (int[]){ 1 }); + + i = SR (1, 2); + T (i, (int[]){ 1 }); // { dg-warning "array subscript \\\[1, 2] is outside array bounds of 'int\\\[1]'" } + + i = SR (2, 3); + T (i, (int[]){ 1, 2, 3 }); + + i = SR (3, 4); + T (i, (int[]){ 2, 3, 4 }); // { dg-warning "array subscript \\\[3, 4] is outside array bounds of 'int\\\[3]'" } +} diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-51.c b/gcc/testsuite/gcc.dg/Warray-bounds-51.c new file mode 100644 index 00000000000..644fcd02cb6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Warray-bounds-51.c @@ -0,0 +1,24 @@ +/* PR middle-end/82612 - missing -Warray-bounds on a non-zero offset + from the address of a non-array object + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +int i; +int f0 (void) +{ + int *p = &i; + return p[2]; // { dg-warning "-Warray-bounds" } +} + +int f1 (void) +{ + int i; + int *p = &i; + return p[2]; // { dg-warning "-Warray-bounds" } +} + +int f2 (int i) +{ + int *p = &i; + return p[2]; // { dg-warning "-Warray-bounds" } +} diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 3f104238d93..44862690559 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -3068,6 +3068,13 @@ get_access_for_expr (tree expr) || !DECL_P (base)) return NULL; + if (tree basesize = DECL_SIZE (base)) + { + poly_int64 sz = tree_to_poly_int64 (basesize); + if (offset < 0 || known_le (sz, offset)) + return NULL; + } + if (!bitmap_bit_p (candidate_bitmap, DECL_UID (base))) return NULL; diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 31258615247..536fb96bf80 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -4122,18 +4122,18 @@ bool vrp_prop::check_array_ref (location_t location, tree ref, bool ignore_off_by_one) { - tree low_sub, up_sub; - tree low_bound, up_bound, up_bound_p1; - if (TREE_NO_WARNING (ref)) return false; - low_sub = up_sub = TREE_OPERAND (ref, 1); - up_bound = array_ref_up_bound (ref); + tree low_sub = TREE_OPERAND (ref, 1); + tree up_sub = low_sub; + tree up_bound = array_ref_up_bound (ref); /* Set for accesses to interior zero-length arrays. */ bool interior_zero_len = false; + tree up_bound_p1; + if (!up_bound || TREE_CODE (up_bound) != INTEGER_CST || (warn_array_bounds < 2 @@ -4186,7 +4186,7 @@ vrp_prop::check_array_ref (location_t location, tree ref, up_bound_p1 = int_const_binop (PLUS_EXPR, up_bound, build_int_cst (TREE_TYPE (up_bound), 1)); - low_bound = array_ref_low_bound (ref); + tree low_bound = array_ref_low_bound (ref); tree artype = TREE_TYPE (TREE_OPERAND (ref, 0)); @@ -4195,8 +4195,8 @@ vrp_prop::check_array_ref (location_t location, tree ref, /* Empty array. */ if (up_bound && tree_int_cst_equal (low_bound, up_bound_p1)) warned = warning_at (location, OPT_Warray_bounds, - "array subscript %E is above array bounds of %qT", - low_bound, artype); + "array subscript %E is outside array bounds of %qT", + low_sub, artype); const value_range *vr = NULL; if (TREE_CODE (low_sub) == SSA_NAME) @@ -4410,6 +4410,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref, { arg = TREE_OPERAND (arg, 0); if (TREE_CODE (arg) != STRING_CST + && TREE_CODE (arg) != PARM_DECL && TREE_CODE (arg) != VAR_DECL) return false; } @@ -4493,7 +4494,9 @@ vrp_prop::check_mem_ref (location_t location, tree ref, if (ignore_off_by_one) ubound += 1; - if (offrange[0] >= ubound || offrange[1] < arrbounds[0]) + if (arrbounds[0] == arrbounds[1] + || offrange[0] >= ubound + || offrange[1] < arrbounds[0]) { /* Treat a reference to a non-array object as one to an array of a single element. */