[PATCH] Don't perform A - (-B) -> A + B when sanitizing

[PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Marek Polacek]

In the following testcase generic_simplify folded A - (-B) -> A + B,
which means we didn't detect an overflow.  So I've tweaked match.pd.

But fold-const.c still can do such a transformation as well, so I had
to tweak it there as well in the same way.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2014-11-12  Marek Polacek  <polacek@redhat.com>

	* match.pd (A - (-B) -> A + B): Check for TYPE_OVERFLOW_WRAPS
	and SANITIZE_SI_OVERFLOW.
	* fold-const.c (fold_binary_loc): 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..504e1b6 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -10544,6 +10544,8 @@ fold_binary_loc (location_t loc,
 
       /* A - B -> A + (-B) if B is easily negatable.  */
       if (negate_expr_p (arg1)
+	  && (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 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))
    (plus (convert @0) (convert @1))))
  /* -(-A) -> A */
  (simplify
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-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-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

Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Richard Biener]

On Wed, 12 Nov 2014, Marek Polacek wrote:

> In the following testcase generic_simplify folded A - (-B) -> A + B,
> which means we didn't detect an overflow.  So I've tweaked match.pd.
> 
> But fold-const.c still can do such a transformation as well, so I had
> to tweak it there as well in the same way.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2014-11-12  Marek Polacek  <polacek@redhat.com>
> 
> 	* match.pd (A - (-B) -> A + B): Check for TYPE_OVERFLOW_WRAPS
> 	and SANITIZE_SI_OVERFLOW.
> 	* fold-const.c (fold_binary_loc): 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..504e1b6 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -10544,6 +10544,8 @@ fold_binary_loc (location_t loc,
>  
>        /* A - B -> A + (-B) if B is easily negatable.  */
>        if (negate_expr_p (arg1)
> +	  && (TYPE_OVERFLOW_WRAPS (type)
> +	      || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)

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 ...

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).

>  	  && ((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?

Ok with that changes.

Thanks,
RIchard.

>     (plus (convert @0) (convert @1))))
>   /* -(-A) -> A */
>   (simplify
> 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-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-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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany

Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Marek Polacek]

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.

> Ok with that changes.

Thanks, I'll install the following then.

2014-11-12  Marek Polacek  <polacek@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

Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Richard Biener]

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  <polacek@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany

Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Marek Polacek]

On Wed, Nov 12, 2014 at 03:22:41PM +0100, Richard Biener wrote:
> TYPE_OVERFLOW_SANITIZES?

Can I call it sanitize_fold_p and put it as a static inline fn
into tree.h?

	Marek

Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Jakub Jelinek]

On Wed, Nov 12, 2014 at 04:07:33PM +0100, Marek Polacek wrote:
> On Wed, Nov 12, 2014 at 03:22:41PM +0100, Richard Biener wrote:
> > TYPE_OVERFLOW_SANITIZES?
> 
> Can I call it sanitize_fold_p and put it as a static inline fn
> into tree.h?

It is a predicate on type, so perhaps sanitized_type_p ?
But TYPE_OVERFLOW_SANITIZED (better than S) sounds good to me too.

	Jakub

Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing

[Richard Biener]

On Wed, 12 Nov 2014, Jakub Jelinek wrote:

> On Wed, Nov 12, 2014 at 04:07:33PM +0100, Marek Polacek wrote:
> > On Wed, Nov 12, 2014 at 03:22:41PM +0100, Richard Biener wrote:
> > > TYPE_OVERFLOW_SANITIZES?
> > 
> > Can I call it sanitize_fold_p and put it as a static inline fn
> > into tree.h?
> 
> It is a predicate on type, so perhaps sanitized_type_p ?
> But TYPE_OVERFLOW_SANITIZED (better than S) sounds good to me too.

Somehow it should be visible that this is all related to
overflow and thus existing TYPE_OVERFLOW_* checks.  So
I like TYPE_OVERFLOW_SANITIZED better ;)

Richard.