On Wed, 12 Nov 2014, Marek Polacek wrote:
> On Wed, Nov 12, 2014 at 10:49:38AM +0100, Richard Biener wrote:
> > TYPE_OVERFLOW_WRAPS returns nonsense for !integral types and thus
> > this check likely restricts the transform to non-floats for example.
> > The macro documentation states "given an integral type ..." thus
> > it should be sth like
> >
> > && (!INTEGRAL_TYPE_P (type)
> > || TYPE_OVERFLOW_WRAPS (type)
> > || (flag_sanitize ...
>
> Ah, I can see now.
>
> > and the TYPE_OVERFLOW_* macros should probably be guarded with
> > tree checking against use on non-integral types (you might want
> > to do that as a followup).
>
> Sure.
>
> > > && ((FLOAT_TYPE_P (type)
> > > /* Avoid this transformation if B is a positive REAL_CST.
> > > */
> > > && (TREE_CODE (arg1) != REAL_CST
> > > diff --git gcc/match.pd gcc/match.pd
> > > index 29b5ab2..5a485f3 100644
> > > --- gcc/match.pd
> > > +++ gcc/match.pd
> > > @@ -291,7 +291,9 @@ along with GCC; see the file COPYING3. If not see
> > > (simplify
> > > (minus (convert1? @0) (convert2? (negate @1)))
> > > (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
> > > - && tree_nop_conversion_p (type, TREE_TYPE (@1)))
> > > + && tree_nop_conversion_p (type, TREE_TYPE (@1))
> > > + && (TYPE_OVERFLOW_WRAPS (type)
> > > + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0))
> >
> > This has the same issue with unnecessarily restricting FLOAT types, so
> > can you fix that here and in the two other patterns already guarded
> > with flag_sanitize?
>
> Done. Note that one pattern had SANITIZE_SI_OVERFLOW check, but not
> the TYPE_OVERFLOW_WRAPS check. Which leads me to thinking that we
> might want a new predicate for
>
> (!INTEGRAL_TYPE_P (type)
> || TYPE_OVERFLOW_WRAPS (type)
> || (flag_sanitize ...))
>
> Do you agree? If so, I'll do that as another followup.
TYPE_OVERFLOW_SANITIZES?
> > Ok with that changes.
>
> Thanks, I'll install the following then.
Thanks.
Richard.
> 2014-11-12 Marek Polacek <pola...@redhat.com>
>
> * fold-const.c (fold_binary_loc): Don't fold if the result
> is undefined.
> * match.pd (A + (-B) -> A - B, A - (-B) -> A + B,
> -(-A) -> A): Likewise.
>
> * c-c++-common/ubsan/overflow-sub-4.c: New test.
> * c-c++-common/ubsan/overflow-sub-2.c: Adjust dg-output.
> * c-c++-common/ubsan/overflow-int128.c: Likewise.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index 756f469..85fada3 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -10544,6 +10544,9 @@ fold_binary_loc (location_t loc,
>
> /* A - B -> A + (-B) if B is easily negatable. */
> if (negate_expr_p (arg1)
> + && (!INTEGRAL_TYPE_P (type)
> + || TYPE_OVERFLOW_WRAPS (type)
> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
> && ((FLOAT_TYPE_P (type)
> /* Avoid this transformation if B is a positive REAL_CST. */
> && (TREE_CODE (arg1) != REAL_CST
> diff --git gcc/match.pd gcc/match.pd
> index 39abe25..4e5e6c5 100644
> --- gcc/match.pd
> +++ gcc/match.pd
> @@ -285,19 +285,25 @@ along with GCC; see the file COPYING3. If not see
> /* Apply STRIP_NOPS on @0 and the negate. */
> (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
> && tree_nop_conversion_p (type, TREE_TYPE (@1))
> - && (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
> + && (!INTEGRAL_TYPE_P (type)
> + || TYPE_OVERFLOW_WRAPS (type)
> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0))
> (minus (convert @0) (convert @1))))
> /* A - (-B) -> A + B */
> (simplify
> (minus (convert1? @0) (convert2? (negate @1)))
> (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
> - && tree_nop_conversion_p (type, TREE_TYPE (@1)))
> + && tree_nop_conversion_p (type, TREE_TYPE (@1))
> + && (!INTEGRAL_TYPE_P (type)
> + || TYPE_OVERFLOW_WRAPS (type)
> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0))
> (plus (convert @0) (convert @1))))
> /* -(-A) -> A */
> (simplify
> (negate (convert? (negate @1)))
> (if (tree_nop_conversion_p (type, TREE_TYPE (@1))
> - && (TYPE_OVERFLOW_WRAPS (type)
> + && (!INTEGRAL_TYPE_P (type)
> + || TYPE_OVERFLOW_WRAPS (type)
> || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0))
> (convert @1)))
>
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> index 125d6bf..4384d7c 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> @@ -41,7 +41,7 @@ main (void)
> /* { dg-output "\[^\n\r]*signed integer overflow:
> 0x7fffffffffffffffffffffffffffff9b \\+ 1024 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: -1 \\+
> 0x80000000000000000000000000000000 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow:
> 0x80000000000000000000000000000000 \\+ -1 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow:
> 0x80000000000000000000000000000000 \\+ -1 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow:
> 0x80000000000000000000000000000000 - 1 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow:
> 0x80000000000000000000000000000064 \\+ -1024 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow:
> 0x7fffffffffffffffffffffffffffffff \\* 2 cannot be represented in type
> '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*negation of 0x80000000000000000000000000000000
> cannot be represented in type '__int128'; cast to an unsigned type to negate
> this value to itself\[^\n\r]*(\n|\r\n|\r)" } */
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> index daf6a54..88c4762 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> @@ -43,12 +43,12 @@ main (void)
> }
>
> /* { dg-output "signed integer overflow: -2147483648 - 1 cannot be
> represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 \\+ -1 cannot
> be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147483548 \\+ -1024
> cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 \\+ -1 cannot
> be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147482648 \\+ -1048576
> cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 - 1 cannot be
> represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147483548 - 1024 cannot
> be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 - 1 cannot be
> represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147482648 - 1048576
> cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1 cannot be
> represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1 cannot
> be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1024
> cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1 cannot
> be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1048576
> cannot be represented in type 'long int'\[^\n\r]*" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1 cannot be
> represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1024 cannot
> be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1 cannot be
> represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1048576
> cannot be represented in type 'long int'\[^\n\r]*" } */
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> index e69de29..519b7ba 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> @@ -0,0 +1,19 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +
> +int
> +main ()
> +{
> + int x = INT_MIN;
> + int y = 0;
> + int z;
> + asm ("" : "+g" (y));
> + asm ("" : "+g" (x));
> + z = y - (-x);
> + asm ("" : "+g" (z));
> +}
> +
> +/* { dg-output "negation of -2147483648 cannot be represented in type
> 'int'\[^\n\r]*; cast to an unsigned type to negate this value to
> itself\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: 0 - -2147483648 cannot be
> represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
>
> Marek
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany
