correct fix to avoid false positives for vectorized stores (PR 96963, 94655)

Message ID	2f9fc19a-3a20-5eef-42a4-ed6bf6070457@gmail.com
State	New
Headers	show Return-Path: <gcc-patches-bounces@gcc.gnu.org> DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B6E01384A40A To: gcc-patches <gcc-patches@gcc.gnu.org> 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-Type: multipart/mixed; boundary="------------6CDE0AF7DB048AC4A017E2ED" Content-Language: en-US Precedence: list From: Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Martin Sebor <msebor@gmail.com> Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces@gcc.gnu.org>
Series	correct fix to avoid false positives for vectorized stores (PR 96963, 94655) \| expand correct fix to avoid false positives for vectorized stores (PR 96963, 94655)

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; +}

correct fix to avoid false positives for vectorized stores (PR 96963, 94655)

Commit Message

Comments

Patch