| Message ID | 20240520220207.1977085-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | match: Disable `(type)zero_one_valuep*CST` for 1bit signed types [PR115154] | expand |
On Tue, May 21, 2024 at 12:02 AM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > The problem here is the pattern added in r13-1162-g9991d84d2a8435 > assumes that it is well defined to multiply zero_one_valuep by the truncated > converted integer constant. It is well defined for all types except for signed 1bit types. > Where `a * -1` is produced which is undefined/ > So disable this pattern for 1bit signed types. > > Note the pattern added in r14-3432-gddd64a6ec3b38e is able to workaround the undefinedness except when > `-fsanitize=undefined` is turned on, this is why I added a testcase for that. > > OK for trunk and gcc-14 and gcc-13 branches? Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK for trunk and branches. Please wait until after 13.3. Richard. > PR tree-optimization/115154 > > gcc/ChangeLog: > > * match.pd (convert (mult zero_one_valued_p@1 INTEGER_CST@2)): Disable > for 1bit signed types. > > gcc/testsuite/ChangeLog: > > * c-c++-common/ubsan/signed1bitfield-1.c: New test. > * gcc.c-torture/execute/signed1bitfield-1.c: New test. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/match.pd | 6 +++-- > .../c-c++-common/ubsan/signed1bitfield-1.c | 25 +++++++++++++++++++ > .../gcc.c-torture/execute/signed1bitfield-1.c | 23 +++++++++++++++++ > 3 files changed, 52 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c > create mode 100644 gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 0f9c34fa897..35e3d82b131 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -2395,12 +2395,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > (mult (convert @0) @1))) > > /* Narrow integer multiplication by a zero_one_valued_p operand. > - Multiplication by [0,1] is guaranteed not to overflow. */ > + Multiplication by [0,1] is guaranteed not to overflow except for > + 1bit signed types. */ > (simplify > (convert (mult@0 zero_one_valued_p@1 INTEGER_CST@2)) > (if (INTEGRAL_TYPE_P (type) > && INTEGRAL_TYPE_P (TREE_TYPE (@0)) > - && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@0))) > + && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@0)) > + && (TYPE_UNSIGNED (type) || TYPE_PRECISION (type) > 1)) > (mult (convert @1) (convert @2)))) > > /* (X << C) != 0 can be simplified to X, when C is zero_one_valued_p. > diff --git a/gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c b/gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c > new file mode 100644 > index 00000000000..2ba8cf4dab0 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c > @@ -0,0 +1,25 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fsanitize=undefined" } */ > + > +/* PR tree-optimization/115154 */ > +/* This was being miscompiled with -fsanitize=undefined due to > + `(signed:1)(t*5)` being transformed into `-((signed:1)t)` which > + is undefined. */ > + > +struct s { > + signed b : 1; > +} f; > +int i = 55; > +__attribute__((noinline)) > +void check(int a) > +{ > + if (!a) > + __builtin_abort(); > +} > +int main() { > + int t = i != 5; > + t = t*5; > + f.b = t; > + int tt = f.b; > + check(f.b); > +} > diff --git a/gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c b/gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c > new file mode 100644 > index 00000000000..ab888ca3a04 > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c > @@ -0,0 +1,23 @@ > +/* PR tree-optimization/115154 */ > +/* This was being miscompiled to `(signed:1)(t*5)` > + being transformed into `-((signed:1)t)` which is undefined. > + Note there is a pattern which removes the negative in some cases > + which works around the issue. */ > + > +struct { > + signed b : 1; > +} f; > +int i = 55; > +__attribute__((noinline)) > +void check(int a) > +{ > + if (!a) > + __builtin_abort(); > +} > +int main() { > + int t = i != 5; > + t = t*5; > + f.b = t; > + int tt = f.b; > + check(f.b); > +} > -- > 2.43.0 >
diff --git a/gcc/match.pd b/gcc/match.pd index 0f9c34fa897..35e3d82b131 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -2395,12 +2395,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (mult (convert @0) @1))) /* Narrow integer multiplication by a zero_one_valued_p operand. - Multiplication by [0,1] is guaranteed not to overflow. */ + Multiplication by [0,1] is guaranteed not to overflow except for + 1bit signed types. */ (simplify (convert (mult@0 zero_one_valued_p@1 INTEGER_CST@2)) (if (INTEGRAL_TYPE_P (type) && INTEGRAL_TYPE_P (TREE_TYPE (@0)) - && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@0))) + && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@0)) + && (TYPE_UNSIGNED (type) || TYPE_PRECISION (type) > 1)) (mult (convert @1) (convert @2)))) /* (X << C) != 0 can be simplified to X, when C is zero_one_valued_p. diff --git a/gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c b/gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c new file mode 100644 index 00000000000..2ba8cf4dab0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c @@ -0,0 +1,25 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fsanitize=undefined" } */ + +/* PR tree-optimization/115154 */ +/* This was being miscompiled with -fsanitize=undefined due to + `(signed:1)(t*5)` being transformed into `-((signed:1)t)` which + is undefined. */ + +struct s { + signed b : 1; +} f; +int i = 55; +__attribute__((noinline)) +void check(int a) +{ + if (!a) + __builtin_abort(); +} +int main() { + int t = i != 5; + t = t*5; + f.b = t; + int tt = f.b; + check(f.b); +} diff --git a/gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c b/gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c new file mode 100644 index 00000000000..ab888ca3a04 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c @@ -0,0 +1,23 @@ +/* PR tree-optimization/115154 */ +/* This was being miscompiled to `(signed:1)(t*5)` + being transformed into `-((signed:1)t)` which is undefined. + Note there is a pattern which removes the negative in some cases + which works around the issue. */ + +struct { + signed b : 1; +} f; +int i = 55; +__attribute__((noinline)) +void check(int a) +{ + if (!a) + __builtin_abort(); +} +int main() { + int t = i != 5; + t = t*5; + f.b = t; + int tt = f.b; + check(f.b); +}
The problem here is the pattern added in r13-1162-g9991d84d2a8435 assumes that it is well defined to multiply zero_one_valuep by the truncated converted integer constant. It is well defined for all types except for signed 1bit types. Where `a * -1` is produced which is undefined/ So disable this pattern for 1bit signed types. Note the pattern added in r14-3432-gddd64a6ec3b38e is able to workaround the undefinedness except when `-fsanitize=undefined` is turned on, this is why I added a testcase for that. OK for trunk and gcc-14 and gcc-13 branches? Bootstrapped and tested on x86_64-linux-gnu with no regressions. PR tree-optimization/115154 gcc/ChangeLog: * match.pd (convert (mult zero_one_valued_p@1 INTEGER_CST@2)): Disable for 1bit signed types. gcc/testsuite/ChangeLog: * c-c++-common/ubsan/signed1bitfield-1.c: New test. * gcc.c-torture/execute/signed1bitfield-1.c: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/match.pd | 6 +++-- .../c-c++-common/ubsan/signed1bitfield-1.c | 25 +++++++++++++++++++ .../gcc.c-torture/execute/signed1bitfield-1.c | 23 +++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/ubsan/signed1bitfield-1.c create mode 100644 gcc/testsuite/gcc.c-torture/execute/signed1bitfield-1.c