Message ID | 2f9fc19a-3a20-5eef-42a4-ed6bf6070457@gmail.com |
---|---|
State | New |
Headers | show |
Series | correct fix to avoid false positives for vectorized stores (PR 96963, 94655) | expand |
Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html On 1/21/21 4:38 PM, Martin Sebor wrote: > 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
Ping 2: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html On 1/29/21 10:20 AM, Martin Sebor wrote: > Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html > > On 1/21/21 4:38 PM, Martin Sebor wrote: >> 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 >
Ping 3: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html This is a fix for two P2 bugs (false positives). On 2/6/21 10:13 AM, Martin Sebor wrote: > Ping 2: > https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html > > On 1/29/21 10:20 AM, Martin Sebor wrote: >> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html >> >> On 1/21/21 4:38 PM, Martin Sebor wrote: >>> 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 >> >
Ping 4: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html On 2/14/21 5:43 PM, Martin Sebor wrote: > Ping 3: > https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html > > This is a fix for two P2 bugs (false positives). > > On 2/6/21 10:13 AM, Martin Sebor wrote: >> Ping 2: >> https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html >> >> On 1/29/21 10:20 AM, Martin Sebor wrote: >>> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html >>> >>> On 1/21/21 4:38 PM, Martin Sebor wrote: >>>> 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 >>> >> >
Ping 5: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html On 2/22/21 5:20 PM, Martin Sebor wrote: > Ping 4: > https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html > > On 2/14/21 5:43 PM, Martin Sebor wrote: >> Ping 3: >> https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html >> >> This is a fix for two P2 bugs (false positives). >> >> On 2/6/21 10:13 AM, Martin Sebor wrote: >>> Ping 2: >>> https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html >>> >>> On 1/29/21 10:20 AM, Martin Sebor wrote: >>>> Ping: >>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html >>>> >>>> On 1/21/21 4:38 PM, Martin Sebor wrote: >>>>> 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 >>>> >>> >> >
On Fri, Jan 22, 2021 at 12:39 AM Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > 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. The proposed code reads even more confusingly than before. What is called 'ptr' is treated as a reference and what you then name ptrtype is the type of the reference. That leads to code like + if (code == MEM_REF && TREE_CODE (ptrtype) == VECTOR_TYPE) what? the pointer type is a VECTOR_TYPE?! I think you are looking for whether the reference type is a VECTOR_TYPE. Part of the confusion is likely because the code commons if (code == ARRAY_REF || code == MEM_REF) but in one case operand zero is a pointer to the object (MEM_REF) and in one case it is the object (ARRAY_REF). Please refactor this code so one can actually read it literally without second-guessing proper variable names. Thanks, Richard. > Martin
On 3/2/21 3:39 AM, Richard Biener wrote: > On Fri, Jan 22, 2021 at 12:39 AM Martin Sebor via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> 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. > > The proposed code reads even more confusingly than before. > What is called 'ptr' is treated as a reference and what you > then name ptrtype is the type of the reference. > > That leads to code like > > + if (code == MEM_REF && TREE_CODE (ptrtype) == VECTOR_TYPE) > > what? the pointer type is a VECTOR_TYPE?! > > I think you are looking for whether the reference type is a VECTOR_TYPE. > > Part of the confusion is likely because the code commons > > if (code == ARRAY_REF || code == MEM_REF) > > but in one case operand zero is a pointer to the object (MEM_REF) and > in one case > it is the object (ARRAY_REF). > > Please refactor this code so one can actually read it literally > without second-guessing proper variable names. I agree that handling both REFs in the same block makes the code more difficult to follow than it needs to be. In the attached patch I've factored the code out into two helper functions, one for ARRAY_REF and another for MEM_REF, and chosen (hopefully) clearer names for the local variables. A similar refactoring could be done for COMPONENT_REF and also for SSA_NAME to reduce the size of the function and make it much easier to follow. I stopped short of doing that but I can do it for GCC 12 when I move the whole compute_objsize() implementation into a more appropriate place (e.g., its own file). Martin > > Thanks, > Richard. > > > >> Martin
On Tue, Mar 2, 2021 at 9:23 PM Martin Sebor <msebor@gmail.com> wrote: > > On 3/2/21 3:39 AM, Richard Biener wrote: > > On Fri, Jan 22, 2021 at 12:39 AM Martin Sebor via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> 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. > > > > The proposed code reads even more confusingly than before. > > What is called 'ptr' is treated as a reference and what you > > then name ptrtype is the type of the reference. > > > > That leads to code like > > > > + if (code == MEM_REF && TREE_CODE (ptrtype) == VECTOR_TYPE) > > > > what? the pointer type is a VECTOR_TYPE?! > > > > I think you are looking for whether the reference type is a VECTOR_TYPE. > > > > Part of the confusion is likely because the code commons > > > > if (code == ARRAY_REF || code == MEM_REF) > > > > but in one case operand zero is a pointer to the object (MEM_REF) and > > in one case > > it is the object (ARRAY_REF). > > > > Please refactor this code so one can actually read it literally > > without second-guessing proper variable names. > > I agree that handling both REFs in the same block makes the code > more difficult to follow than it needs to be. > > In the attached patch I've factored the code out into two helper > functions, one for ARRAY_REF and another for MEM_REF, and chosen > (hopefully) clearer names for the local variables. > > A similar refactoring could be done for COMPONENT_REF and also > for SSA_NAME to reduce the size of the function and make it much > easier to follow. I stopped short of doing that but I can do it > for GCC 12 when I move the whole compute_objsize() implementation > into a more appropriate place (e.g., its own file). + if (!addr && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE) POINTER_TYPE_P () + /* Avoid arrays of pointers. FIXME: Hande pointers to arrays + of known bound. */ + return false; + if (TREE_CODE (TREE_TYPE (mref)) == 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). VECTOR_TYPE_P (TREE_TYPE (mref)) + 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). You could gate this on cfun->curr_properties & PROP_gimple_lvec which is only set after vectorization. Users can write vector accesses using intrinsics or GCCs vector extension and I guess warning for those is useful (and they less likely will cover multiple fields). Note the vectorizer also uses integer types for vector accesses either when vectorizing using 'generic' vectors (for loads, stores and bit operations we don't need any vector ISA) or when composing vectors. Note store-merging has the same issue (but fortunately runs later?) OK with the above changes. Thanks, Richard. > Martin > > > > > Thanks, > > Richard. > > > > > > > >> Martin >
On 3/3/21 3:31 AM, Richard Biener wrote: > On Tue, Mar 2, 2021 at 9:23 PM Martin Sebor <msebor@gmail.com> wrote: >> >> On 3/2/21 3:39 AM, Richard Biener wrote: >>> On Fri, Jan 22, 2021 at 12:39 AM Martin Sebor via Gcc-patches >>> <gcc-patches@gcc.gnu.org> wrote: >>>> >>>> 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. >>> >>> The proposed code reads even more confusingly than before. >>> What is called 'ptr' is treated as a reference and what you >>> then name ptrtype is the type of the reference. >>> >>> That leads to code like >>> >>> + if (code == MEM_REF && TREE_CODE (ptrtype) == VECTOR_TYPE) >>> >>> what? the pointer type is a VECTOR_TYPE?! >>> >>> I think you are looking for whether the reference type is a VECTOR_TYPE. >>> >>> Part of the confusion is likely because the code commons >>> >>> if (code == ARRAY_REF || code == MEM_REF) >>> >>> but in one case operand zero is a pointer to the object (MEM_REF) and >>> in one case >>> it is the object (ARRAY_REF). >>> >>> Please refactor this code so one can actually read it literally >>> without second-guessing proper variable names. >> >> I agree that handling both REFs in the same block makes the code >> more difficult to follow than it needs to be. >> >> In the attached patch I've factored the code out into two helper >> functions, one for ARRAY_REF and another for MEM_REF, and chosen >> (hopefully) clearer names for the local variables. >> >> A similar refactoring could be done for COMPONENT_REF and also >> for SSA_NAME to reduce the size of the function and make it much >> easier to follow. I stopped short of doing that but I can do it >> for GCC 12 when I move the whole compute_objsize() implementation >> into a more appropriate place (e.g., its own file). > > + if (!addr && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE) > > POINTER_TYPE_P () I think this is intentional: POINTER_TYPE_P() includes C++ references and although they don't come up here (there's no such thing as arrays of references) it didn't seem appropriate to be including them in the test even if it's benign. > > + /* Avoid arrays of pointers. FIXME: Hande pointers to arrays > + of known bound. */ > + return false; > > + if (TREE_CODE (TREE_TYPE (mref)) == 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). > > VECTOR_TYPE_P (TREE_TYPE (mref)) Okay. > > + 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). > > You could gate this on cfun->curr_properties & PROP_gimple_lvec > which is only set after vectorization. Users can write vector > accesses using intrinsics or GCCs vector extension and I guess > warning for those is useful (and they less likely will cover multiple > fields). Thanks, that's useful to know! I'll keep it in mind for GCC 12. > > Note the vectorizer also uses integer types for vector accesses > either when vectorizing using 'generic' vectors (for loads, stores > and bit operations we don't need any vector ISA) or when > composing vectors. > > Note store-merging has the same issue (but fortunately runs later?) Yes, it's the same issue (as is FRE/PRE using one member to write to the next) and it does cause false positives depending on where the code is called from. They're on my to do list for GCC 12 to avoid as we discussed, by transform those into MEM_REFs with the enclosing object + offset instead of the member. > > OK with the above changes. I've committed the patch without the POINTER_TYPE_P change (I'm okay with it if you feel it necessary.) Martin > > Thanks, > Richard. > >> Martin >> >>> >>> Thanks, >>> Richard. >>> >>> >>> >>>> 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; +}