From patchwork Thu Jan 21 23:38:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1430142 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=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=Wodwvcj1; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DMJkB3KgVz9sB4 for ; Fri, 22 Jan 2021 10:38:39 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CF9A6388A438; Thu, 21 Jan 2021 23:38:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CF9A6388A438 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1611272315; bh=7YOlpsteyJetD11gupeQoZ480OswEcwf+3NwFU5Wsqg=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=Wodwvcj1IR9F0kFfzjZKORzX1DpUgP6WPzi3c08xBbuO7ZeBxYS/BUGmzRsyX6mOL zyAvkBhaRNar4RWoy2V7cPntCwWXw2OW7fBINK2mDq78gYafpjGen9U8QVKVEmDoxe 2KDhqJgf31psLJmILqV3kYcsAWX0+FnN5VmibJaM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com [IPv6:2607:f8b0:4864:20::72c]) by sourceware.org (Postfix) with ESMTPS id B6E01384A40A for ; Thu, 21 Jan 2021 23:38:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B6E01384A40A Received: by mail-qk1-x72c.google.com with SMTP id c7so3568033qke.1 for ; Thu, 21 Jan 2021 15:38:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version:content-language; bh=7YOlpsteyJetD11gupeQoZ480OswEcwf+3NwFU5Wsqg=; b=JKhGSLhL0SbhY1gNfMP3tRZGBoe+WdTGLQCVlTo+i5Fo+DyC0XnU7qOEBGmv5BoQA9 X8X9Zd8eK37bZ61un5VQKFWO/G/BUq/tR/ArTirjP+s/NuhdWtdZgLXiXuwu7zGlyRhx 1d3id3rVPnD7NheMk/s9AhV5OmLOQ3aHZuJQNtIX7WxTJUV9zNBfFnl48eFVj8cq0jGj BpaE00MtuuD1gtK53V4i0CSBpaE2/SRk9LABfi/dyfPo9/yZ+HXCuBO9Zyodnj1nYMgA lH6ut1TNAIsALBmJkNvfxPphLCCHhG1sr+qmQzweBOGwv4ca+5v4Pqm9DZG4orV4oYZQ ip8g== X-Gm-Message-State: AOAM533dOPj9zEhZ84jG9fN2PzuUuX5sZ6lyRiV/cRa2BSdOz1DP4GLP flZRV3O66Qj3PF8ldS/LTtKqtcnja5k= X-Google-Smtp-Source: ABdhPJzF/V4RP3r+XaIbWwLna4kBbtZK2p/DQIyJCcTYQNN5BQthPbBi045AMC7Z4v4DCcafHKWf9g== X-Received: by 2002:a37:c0b:: with SMTP id 11mr2431968qkm.32.1611272312077; Thu, 21 Jan 2021 15:38:32 -0800 (PST) Received: from [192.168.0.41] (184-96-239-30.hlrn.qwest.net. [184.96.239.30]) by smtp.gmail.com with ESMTPSA id a206sm4866905qkc.30.2021.01.21.15.38.30 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 21 Jan 2021 15:38:31 -0800 (PST) To: gcc-patches Subject: [PATCH] correct fix to avoid false positives for vectorized stores (PR 96963, 94655) Message-ID: <2f9fc19a-3a20-5eef-42a4-ed6bf6070457@gmail.com> Date: Thu, 21 Jan 2021 16:38:30 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 Content-Language: en-US X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Martin Sebor via Gcc-patches From: Martin Sebor Reply-To: Martin Sebor Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" The hack I put in compute_objsize() last January for pr93200 isn't quite correct. It happened to suppress the false positive there but, due to what looks like a thinko on my part, not in some other test cases involving vectorized stores. The attached change adjusts the hack to have compute_objsize() give up on all MEM_REFs with a vector type. This effectively disables -Wstringop-{overflow,overread} for vector accesses (either written by the user or synthesized by GCC from ordinary accesses). It doesn't affect -Warray-bounds because this warning doesn't use compute_objsize() yet. When it does (it should considerably simplify the code) some additional changes will be needed to preserve -Warray-bounds for out of bounds vector accesses. The test this patch adds should serve as a reminder to make it if we forget. Tested on x86_64-linux. Since PR 94655 was reported against GCC 10 I'd like to apply this fix to both the trunk and the 10 branch. Martin PR middle-end/96963 - -Wstringop-overflow false positive with -ftree-vectorize when assigning consecutive char struct members gcc/ChangeLog: PR middle-end/96963 * builtins.c (compute_objsize_r): Correct a workaround for vectorized assignments. gcc/testsuite/ChangeLog: PR middle-end/96963 * gcc.dg/Wstringop-overflow-65.c: New test. * gcc.dg/Warray-bounds-69.c: Same. diff --git a/gcc/builtins.c b/gcc/builtins.c index 0aed008687c..2ffe472d4ea 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -5425,24 +5425,33 @@ compute_objsize_r (tree ptr, int ostype, access_ref *pref, ++pref->deref; tree ref = TREE_OPERAND (ptr, 0); - tree reftype = TREE_TYPE (ref); - if (!addr && code == ARRAY_REF - && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE) - /* Avoid arrays of pointers. FIXME: Hande pointers to arrays - of known bound. */ - return false; + { + tree reftype = TREE_TYPE (ref); + if (!addr && code == ARRAY_REF + && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE) + /* Avoid arrays of pointers. FIXME: Hande pointers to arrays + of known bound. */ + return false; + } + { + tree ptrtype = TREE_TYPE (ptr); + if (POINTER_TYPE_P (ptrtype)) + ptrtype = TREE_TYPE (ptrtype); - if (code == MEM_REF && TREE_CODE (reftype) == POINTER_TYPE) - { - /* Give up for MEM_REFs of vector types; those may be synthesized - from multiple assignments to consecutive data members. See PR - 93200. - FIXME: Deal with this more generally, e.g., by marking up such - MEM_REFs at the time they're created. */ - reftype = TREE_TYPE (reftype); - if (TREE_CODE (reftype) == VECTOR_TYPE) + if (code == MEM_REF && TREE_CODE (ptrtype) == VECTOR_TYPE) + { + /* Hack: Give up for MEM_REFs of vector types; those may be + synthesized from multiple assignments to consecutive data + members (see PR 93200 and 96963). + FIXME: Vectorized assignments should only be present after + vectorization so this hack is only necessary after it has + run and could be avoided in calls from prior passes (e.g., + tree-ssa-strlen.c). + FIXME: Deal with this more generally, e.g., by marking up + such MEM_REFs at the time they're created. */ return false; - } + } + } if (!compute_objsize_r (ref, ostype, pref, snlim, qry)) return false; diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-69.c b/gcc/testsuite/gcc.dg/Warray-bounds-69.c new file mode 100644 index 00000000000..5a955774124 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Warray-bounds-69.c @@ -0,0 +1,74 @@ +/* Verify that storing a bigger vector into smaller space is diagnosed. + { dg-do compile } + { dg-options "-O2 -Warray-bounds" } */ + +typedef __INT16_TYPE__ int16_t; +typedef __attribute__ ((__vector_size__ (32))) char C32; + +typedef __attribute__ ((__vector_size__ (64))) int16_t I16_64; + +void sink (void*); + + +void nowarn_c32 (char c) +{ + extern char nowarn_a32[32]; + + void *p = nowarn_a32; + *(C32*)p = (C32){ c }; + sink (p); + + char a32[32]; + p = a32; + *(C32*)p = (C32){ c }; + sink (p); +} + +/* The invalid stores below are diagnosed by -Warray-bounds only + because it doesn't use compute_objsize(). If/when that changes + the function might need adjusting to avoid the hack put in place + to avoid false positives due to vectorization. */ + +void warn_c32 (char c) +{ + extern char warn_a32[32]; // { dg-message "'warn_a32'" "note" } + + void *p = warn_a32 + 1; + *(C32*)p = (C32){ c }; // { dg-warning "\\\[-Warray-bounds" } + + /* Verify a local variable too. */ + char a32[32]; // { dg-message "'a32'" } + p = a32 + 1; + *(C32*)p = (C32){ c }; // { dg-warning "\\\[-Warray-bounds" } + sink (p); +} + + +void nowarn_i16_64 (int16_t i) +{ + extern char nowarn_a64[64]; + + void *p = nowarn_a64; + I16_64 *q = (I16_64*)p; + *q = (I16_64){ i }; + + char a64[64]; + q = (I16_64*)a64; + *q = (I16_64){ i }; + sink (q); +} + +void warn_i16_64 (int16_t i) +{ + extern char warn_a64[64]; // { dg-message "'warn_a64'" } + + void *p = warn_a64 + 1; + I16_64 *q = (I16_64*)p; + *q = (I16_64){ i }; // { dg-warning "\\\[-Warray-bounds" } + + char a64[64]; // { dg-message "'a64'" } + p = a64 + 1; + q = (I16_64*)p; + *q = (I16_64){ i }; // { dg-warning "\\\[-Warray-bounds" } + sink (p); +} diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c index cb2c329aa84..cc28d22864c 100644 --- a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c @@ -24,17 +24,22 @@ void nowarn_c32 (char c) sink (p); } +/* The tests below fail as a result of the hack for PR 96963. However, + with -Wall, the invalid stores are diagnosed by -Warray-bounds which + runs before vectorization and so doesn't need the hack. If/when + -Warray changes to use compute_objsize() this will need adjusting. */ + void warn_c32 (char c) { - extern char warn_a32[32]; // { dg-message "at offset 32 into destination object 'warn_a32' of size 32" "note" } + extern char warn_a32[32]; // { dg-message "at offset 32 into destination object 'warn_a32' of size 32" "note" "pr97027" { xfail *-*-* } } void *p = warn_a32 + 1; - *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" } + *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } } /* Verify a local variable too. */ char a32[32]; p = a32 + 1; - *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" } + *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } } sink (p); } diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-65.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-65.c new file mode 100644 index 00000000000..d3e301dbc2f --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-65.c @@ -0,0 +1,98 @@ +/* PR middle-end/96963 - -Wstringop-overflow false positive with + -ftree-vectorize when assigning consecutive char struct members + { dg-do compile } + { dg-options "-O2 -Wall -ftree-vectorize" } */ + +void sink (void*); + +struct Char +{ + int i; + char c, d, e, f; + char a[2], b[2]; +}; + +void nowarn_char_assign (struct Char *p) +{ + sink (&p->c); + + /* Verify the bogus warning triggered by the tree-ssa-strlen.c pass + is not issued. */ + p->c = 1; // { dg-bogus "\\\[-Wstringop-overflow"] } + p->d = 2; + p->e = 3; + p->f = 4; +} + +void nowarn_char_array_assign (struct Char *p) +{ + sink (p->a); + + p->a[0] = 1; // { dg-bogus "\\\[-Wstringop-overflow"] } + p->a[1] = 2; + p->b[0] = 3; + p->b[1] = 4; +} + +void warn_char_array_assign_interior (struct Char *p) +{ + sink (p->a); + + p->a[0] = 1; + p->a[1] = 2; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" + /* Warnings are only suppressed for trailing arrays. Verify + one is issued for an interior array. */ + p->a[2] = 5; // { dg-warning "\\\[-Wstringop-overflow" } +#pragma GCC diagnostic pop +} + +void warn_char_array_assign_trailing (struct Char *p) +{ + /* This is separated from warn_char_array_assign_interior because + otherwise GCC removes the store to p->a[2] as dead since it's + overwritten by p->b[0]. */ + sink (p->b); + + p->b[0] = 3; + p->b[1] = 4; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" + /* Warnings are only suppressed for trailing arrays with at most + one element. Verify one is issued for a two-element array. */ + p->b[2] = 5; // { dg-warning "\\\[-Wstringop-overflow" } +#pragma GCC diagnostic pop +} + + +/* Also verify there's no warning for other types than char (even though + the problem was limited to chars and -Wstringop-overflow should only + trigger for character accesses). */ + +struct Short +{ + int i; + short c, d, e, f; + short a[2], b[2]; +}; + +void nowarn_short_assign (struct Short *p) +{ + sink (&p->c); + + p->c = 1; + p->d = 2; + p->e = 3; + p->f = 4; +} + +void nowarn_short_array_assign (struct Short *p) +{ + sink (p->a); + + p->a[0] = 1; + p->a[1] = 2; + p->b[0] = 3; + p->b[1] = 4; +}