From patchwork Wed Nov 6 00:00:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1190073 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-512561-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="yJW304Hn"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="LU2MIz6V"; 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 4776BG44yRz9sP6 for ; Wed, 6 Nov 2019 11:00:52 +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:from :subject:to:message-id:date:mime-version:content-type; q=dns; s= default; b=Qr2QOhvpQAERdaO5qKOr7TrLhFnn6dO+DXv8Rrj44eZbBBfjlrwcM lTZBPA/YHhqGCP7jtmAexaDn7Wl9mHSh3xcc0h17DGYVayjzovkwkZnoaCl0PWGI vI32G442BZm4scYnHKXFGGRsVZgR/V6XrRSkILxPjP9Nf/BWVeb0s0= 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:from :subject:to:message-id:date:mime-version:content-type; s= default; bh=JpRxf8o75L5NgG8+qVnkkTssKws=; b=yJW304HnvnDlIiQRhZTU wjdV0Jl8A4+YjgVLqG50wWu7bChLWOj5SHCtOpzZ/ijt45pHtjgoXVgjh1WUvT8l 6xAPx+8lQnK5vz/CWA+IADlU8kIImfG0UWlI4ilQnNaDNwo8eLwt3eCY6siCOUdg Du/SWZKrtyHA6yFZllXPDWE= Received: (qmail 54109 invoked by alias); 6 Nov 2019 00:00:27 -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 54086 invoked by uid 89); 6 Nov 2019 00:00:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=vast X-HELO: mail-yb1-f180.google.com Received: from mail-yb1-f180.google.com (HELO mail-yb1-f180.google.com) (209.85.219.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Nov 2019 00:00:13 +0000 Received: by mail-yb1-f180.google.com with SMTP id e13so3336558ybh.4 for ; Tue, 05 Nov 2019 16:00:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:subject:to:message-id:date:user-agent:mime-version :content-language; bh=5l8e/e0Usopj8c7DOlCdyYTherh4JcW9a4QltKSh0GU=; b=LU2MIz6Vl59N6D1zTgDqrYBykM5v2hR5K5T0IHa2n9uGKLzoXgDUnN+jQDdnrXVAs+ kaRV4WcB90eKulcVwLK9l6sfLjCCpX5DjEq6TvBVBH9Ama2plsMWz+YwHzU1qulU+LRZ RN7+Uz7GKjExQND2SPknyb1WM24Z61uTGFlYHiEp1GjTTigNgUiLhvG3mQqaSZIVFLxd C9TXHXgVFcXowwCF0nJJtGHCueFwFOCrSxVgv2ybob+7UURwmpNugR6AlXr8+uvmVL6m zR7EYqAVIVCAhhZokzPYwmZ9nXFm/sXPEL3x9U+vb5o4f8M7NBjMB+xRbWyD6BPQvOMs NO3w== Received: from [192.168.0.41] (97-118-98-145.hlrn.qwest.net. [97.118.98.145]) by smtp.gmail.com with ESMTPSA id 137sm15658093ywu.84.2019.11.05.16.00.10 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 Nov 2019 16:00:10 -0800 (PST) From: Martin Sebor Subject: [PATCH] tweak component_ref_size to extend -Wzero-length-array-bounds and not ICE (PR 92373) To: gcc-patches Message-ID: <2b4af17c-9214-9922-3a37-6224e8f4971f@gmail.com> Date: Tue, 5 Nov 2019 17:00:09 -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 After considering more instances of the enhanced -Warray-bounds and the new -Wzero-length-array-bounds warnings I realized that there are some where the former is being issued for zero-length arrays but where the latter would be more appropriate. The attached change tweaks the logic in component_ref_size to have some of the latter instances replace some of the former, effectively relaxing -Warray-bounds a bit. -Wzero-length-array-bounds triggers for accesses to zero-length arrays with non-negative indices, interior or otherwise, that are within the bounds of the largest enclosing object. All other accesses are diagnosed via -Warray-bounds. The change replaces all instances of -Warray-bounds in Glibc with -Wzero-length-array-bounds. In the kernel, it similarly turns the vast majority of distinct -Warray-bounds warnings (56) into -Wzero-length-array-bounds, leaving only four -Warray-bounds. This should make adopting GCC 10 much easier for low-level code that relies on dangerous "tricks" involving zero-length arrays without sacrificing the ability to detect bugs in other projects. While making the change I also removed an incorrect assumption I introduced last week that was causing an ICE (as reported in PR 92373). Tested on x86_64-linux. Martin PR tree-optimization/92373 - ICE in -Warray-bounds on access to member array in an initialized char buffer gcc/testsuite/ChangeLog: PR tree-optimization/92373 * gcc.dg/Warray-bounds-55.c: New test. * gcc.dg/Wzero-length-array-bounds-2.c: New test. gcc/ChangeLog: PR tree-optimization/92373 * tree.c (component_ref_size): Only consider initializers of objects of matching struct types. Return null for for instances of interior zero-length arrays. Index: gcc/tree.c =================================================================== --- gcc/tree.c (revision 277853) +++ gcc/tree.c (working copy) @@ -13635,6 +13635,8 @@ component_ref_size (tree ref, bool *interior_zero_ return NULL_TREE; base = TREE_OPERAND (ref, 0); + while (TREE_CODE (base) == COMPONENT_REF) + base = TREE_OPERAND (base, 0); baseoff = tree_to_poly_int64 (byte_position (TREE_OPERAND (ref, 1))); } @@ -13656,27 +13658,28 @@ component_ref_size (tree ref, bool *interior_zero_ memsize = NULL_TREE; - /* MEMBER is a true flexible array member. Compute its size from - the initializer of the BASE object if it has one. */ - if (tree init = DECL_P (base) ? DECL_INITIAL (base) : NULL_TREE) - { - init = get_initializer_for (init, member); - if (init) - { - memsize = TYPE_SIZE_UNIT (TREE_TYPE (init)); - if (tree refsize = TYPE_SIZE_UNIT (reftype)) - { - /* Use the larger of the initializer size and the tail - padding in the enclosing struct. */ - poly_int64 rsz = tree_to_poly_int64 (refsize); - rsz -= baseoff; - if (known_lt (tree_to_poly_int64 (memsize), rsz)) - memsize = wide_int_to_tree (TREE_TYPE (memsize), rsz); - } + if (typematch) + /* MEMBER is a true flexible array member. Compute its size from + the initializer of the BASE object if it has one. */ + if (tree init = DECL_P (base) ? DECL_INITIAL (base) : NULL_TREE) + { + init = get_initializer_for (init, member); + if (init) + { + memsize = TYPE_SIZE_UNIT (TREE_TYPE (init)); + if (tree refsize = TYPE_SIZE_UNIT (reftype)) + { + /* Use the larger of the initializer size and the tail + padding in the enclosing struct. */ + poly_int64 rsz = tree_to_poly_int64 (refsize); + rsz -= baseoff; + if (known_lt (tree_to_poly_int64 (memsize), rsz)) + memsize = wide_int_to_tree (TREE_TYPE (memsize), rsz); + } - baseoff = 0; - } - } + baseoff = 0; + } + } if (!memsize) { @@ -13689,7 +13692,7 @@ component_ref_size (tree ref, bool *interior_zero_ /* The size of a flexible array member of an extern struct with no initializer cannot be determined (it's defined in another translation unit and can have an initializer - witth an arbitrary number of elements). */ + with an arbitrary number of elements). */ return NULL_TREE; /* Use the size of the base struct or, for interior zero-length @@ -13696,10 +13699,12 @@ component_ref_size (tree ref, bool *interior_zero_ arrays, the size of the enclosing type. */ memsize = TYPE_SIZE_UNIT (bt); } - else + else if (DECL_P (base)) /* Use the size of the BASE object (possibly an array of some other type such as char used to store the struct). */ memsize = DECL_SIZE_UNIT (base); + else + return NULL_TREE; } /* If the flexible array member has a known size use the greater Index: gcc/testsuite/gcc.dg/Warray-bounds-55.c =================================================================== --- gcc/testsuite/gcc.dg/Warray-bounds-55.c (nonexistent) +++ gcc/testsuite/gcc.dg/Warray-bounds-55.c (working copy) @@ -0,0 +1,28 @@ +/* PR middle-end/92373 - ICE in -Warray-bounds on access to member array + in an initialized char buffer + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +void sink (void*); + +struct S +{ + char data[1]; +}; + +char a[6] = { }; + +int f (void) +{ + struct S *p = (struct S*)a; + return p->data[4]; + +} + +void g (void) +{ + char b[6] = { }; + struct S *p = (struct S*)b; + p->data[4] = 0; + sink (p); +} Index: gcc/testsuite/gcc.dg/Wzero-length-array-bounds-2.c =================================================================== --- gcc/testsuite/gcc.dg/Wzero-length-array-bounds-2.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wzero-length-array-bounds-2.c (working copy) @@ -0,0 +1,125 @@ +/* Test to verify that -Wzero-length-bounds and not -Warray-bounds is + issued for accesses to interior zero-length array members that are + within the bounds of the enclosing struct. + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +void sink (void*); + +struct A { int i; }; +struct B { int j; struct A a[0]; }; + +struct C +{ + struct B b1; + struct B b2; +}; + + +void test_B_ref (struct B *p) +{ + // References with negative indices are always diagnosed by -Warray-bounds + // even though they could be considered the same as past the end accesses + // to trailing zero-length arrays. + p->a[-1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->a[ 0].i = 0; + p->a[ 1].i = 0; + sink (p); + + p[1].a[-1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p[1].a[ 0].i = 0; + p[1].a[ 1].i = 0; +} + + +void test_C_ref (struct C *p) +{ + p->b1.a[-1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->b1.a[ 0].i = 0; // { dg-warning "\\\[-Wzero-length-bounds" } + p->b1.a[ 1].i = 0; // { dg-warning "\\\[-Wzero-length-bounds" } + + // Accesses to trailing zero-length arrays are not diagnosed (should + // they be?) + p->b2.a[ 0].i = 0; + p->b2.a[ 9].i = 0; +} + + +void test_C_decl (void) +{ + struct C c, *p = &c; + + p->b1.a[-1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + + // c.b1.a[0].i overlaps c.b2.j. + p->b1.a[ 0].i = 0; // { dg-warning "\\\[-Wzero-length-bounds" } + + // c.b1.a[1].i is past the end of c... + p->b1.a[ 1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + sink (p); + + // ...and so are references to all elements of c.b2.a + p->b2.a[ 0].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->b2.a[ 1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + sink (p); +} + + +char cbuf1[1 * sizeof (struct C)]; +char cbuf2[2 * sizeof (struct C)] = { }; + +void test_C_global_buf (void) +{ + struct C *p = (struct C*)&cbuf1; + + p->b1.a[-1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->b1.a[ 0].i = 0; // { dg-warning "\\\[-Wzero-length-bounds" } + p->b1.a[ 1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + sink (p); + + p->b2.a[ 0].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->b2.a[ 1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + sink (p); + + p = (struct C*)&cbuf2; + p->b1.a[-1].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->b1.a[ 0].i = 0; // { dg-warning "\\\[-Wzero-length-bounds" } + p->b1.a[ 1].i = 0; // { dg-warning "\\\[-Wzero-length-bounds" } + sink (p); + + p->b2.a[ 0].i = 0; + p->b2.a[ 1].i = 0; + p->b2.a[ 2].i = 0; // { dg-warning "\\\[-Warray-bounds" } + p->b2.a[ 3].i = 0; // { dg-warning "\\\[-Warray-bounds" } + sink (p); +} + + +void test_C_local_buf (void) +{ + char cbuf1[1 * sizeof (struct C)] = ""; + char cbuf2[2 * sizeof (struct C)] = { }; + + struct C *p = (struct C*)&cbuf1; + + p->b1.a[-1].i = 1; // { dg-warning "\\\[-Warray-bounds" } + p->b1.a[ 0].i = 2; // { dg-warning "\\\[-Wzero-length-bounds" } + p->b1.a[ 1].i = 3; // { dg-warning "\\\[-Warray-bounds" } + sink (p); + + p->b2.a[ 0].i = 4; // { dg-warning "\\\[-Warray-bounds" } + p->b2.a[ 1].i = 5; // { dg-warning "\\\[-Warray-bounds" } + sink (p); + + p = (struct C*)&cbuf2; + p->b1.a[-1].i = 6; // { dg-warning "\\\[-Warray-bounds" } + p->b1.a[ 0].i = 7; // { dg-warning "\\\[-Wzero-length-bounds" } + p->b1.a[ 1].i = 8; // { dg-warning "\\\[-Wzero-length-bounds" } + sink (p); + + p->b2.a[ 0].i = 9; + p->b2.a[ 1].i = 10; + p->b2.a[ 2].i = 11; // { dg-warning "\\\[-Warray-bounds" } + p->b2.a[ 3].i = 12; // { dg-warning "\\\[-Warray-bounds" } + sink (p); +}