On Thu, Sep 23, 2021 at 9:48 AM Hongtao Liu <crazy...@gmail.com> wrote:
>
> On Wed, Sep 22, 2021 at 10:21 PM Martin Sebor <mse...@gmail.com> wrote:
> >
> > On 9/21/21 7:38 PM, Hongtao Liu wrote:
> > > On Mon, Sep 20, 2021 at 4:13 AM Martin Sebor <mse...@gmail.com> wrote:
> > ...
> > >>>>> diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> > >>>>> b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> > >>>>> index 1d79930cd58..9351f7e7a1a 100644
> > >>>>> --- a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> > >>>>> +++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> > >>>>> @@ -1,7 +1,7 @@
> > >>>>> /* PR middle-end/91458 - inconsistent warning for writing past the
> > >>>>> end
> > >>>>> of an array member
> > >>>>> { dg-do compile }
> > >>>>> - { dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf" } */
> > >>>>> + { dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf
> > >>>>> -fno-tree-vectorize" } */
> > >>>>
> > >>>> The testcase is large - what part requires this change? Given the
> > >>>> testcase was added for inconsistent warnings do they now become
> > >>>> inconsistent again as we enable vectorization at -O2?
> > >>>>
> > >>>> That said, the testcase adjustments need some explaining - I suppose
> > >>>> you didn't just slap -fno-tree-vectorize to all of those changing
> > >>>> behavior?
> > >>>>
> > >>> void ga1_ (void)
> > >>> {
> > >>> a1_.a[0] = 0;
> > >>> a1_.a[1] = 1; // { dg-warning
> > >>> "\\\[-Wstringop-overflow" }
> > >>> a1_.a[2] = 2; // { dg-warning
> > >>> "\\\[-Wstringop-overflow" }
> > >>>
> > >>> struct A1 a;
> > >>> a.a[0] = 0;
> > >>> a.a[1] = 1; // { dg-warning
> > >>> "\\\[-Wstringop-overflow" }
> > >>> a.a[2] = 2; // { dg-warning
> > >>> "\\\[-Wstringop-overflow" }
> > >>> sink (&a);
> > >>> }
> > >>>
> > >>> It's supposed to be 2 warning for a.a[1] = 1 and a.a[2] = 1 since
> > >>> there are 2 accesses, but after enabling vectorization, there's only
> > >>> one access, so one warning is missing which causes the failure.
> >
> > With the stores vectorized, is the warning on the correct line or
> > does it point to the first store, the one that's in bounds, as
> > it does with -O3? The latter would be a regression at -O2.
> For the upper case, It points to the second store which is out of
> bounds, the third store warning is missing.
> >
> > >>
> > >> I would find it preferable to change the test code over disabling
> > >> optimizations that are on by default. My concern is that the test
> > >> would no longer exercise the default behavior. (The same goes for
> > >> the -fno-ipa-icf option.)
> > > Hmm, it's a middle-end test, for some backend, it may not do
> > > vectorization(it depends on TARGET_VECTOR_MODE_SUPPORTED_P and
> > > relative cost model).
> >
> > Yes, there are quite a few warning tests like that. Their main
> > purpose is to verify that in common GCC invocations (i.e., without
> > any special options) warnings are a) issued when expected and b)
> > not issued when not expected. Otherwise, middle end warnings are
> > known to have both false positives and false negatives in some
> > invocations, depending on what optimizations are in effect.
> > Indiscriminately disabling common optimizations for these large
> > tests and invoking them under artificial conditions would
> > compromise this goal and hide the problems.
> >
> > If enabling vectorization at -O2 causes regressions in the quality
> > of diagnostics (as the test failure above indicates seems to be
> > happening) we should investigate these and open bugs for them so
> > they can be fixed. We can then tweak the specific failing test
> > cases to avoid the failures until they are fixed.
> There are indeed cases of false positives and false negatives
> .i.e.
> // Verify warning for access to a definition with an initializer that
> // initializes the one-element array member.
> struct A1 a1i_1 = { 0, { 1 } };
>
> void ga1i_1 (void)
> {
> a1i_1.a[0] = 0;
> a1i_1.a[1] = 1; // { dg-warning "\\\[-Wstringop-overflow" }
> a1i_1.a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" }
>
> struct A1 a = { 0, { 1 } }; --- false positive here.
> a.a[0] = 1;
> a.a[1] = 2; // { dg-warning
> "\\\[-Wstringop-overflow" } false negative here.
> a.a[2] = 3; // { dg-warning
> "\\\[-Wstringop-overflow" } false negative here.
> sink (&a);
> }
Similar for
* gcc.dg/Warray-bounds-51.c.
* gcc.dg/Warray-parameter-3.c
* gcc.dg/Wstringop-overflow-14.c
* gcc.dg/Wstringop-overflow-21.c
So there're 3 situations.
1. All accesses are out of bound, and after vectorization, there are
some warnings missing.
2. Part of accesses are inbound, part of accesses are out of bound,
and after vectorization, the warning goes from out of bound line to
inbound line.
3. All access are out of bound, and after vectoriation, all warning
are missing, and goes to a false-positive line.
>
> the last 2 warnings are missing, and there's new warning on the line
> *struct A1 a = { 0, { 1 } };
> >
> > Martin
>
>
>
> --
> BR,
> Hongtao
--
BR,
Hongtao
