diff mbox

Peg down -(-A) -> A transformation

Message ID 20141111174025.GI10852@redhat.com
State New
Headers show

Commit Message

Marek Polacek Nov. 11, 2014, 5:40 p.m. UTC 
While match.pd has been changed to not transform -(-A) to A
when the overflow does not wrap or the signed integer ovreflow
checking is enabled, fold_negate_expr still happily does such
a transformation.  That's bad because then we can't detect
an overflow, as on the following testcase.  But I allowed this
transformation for constants, because VRP seems to rely on that
(abs_extent_range calls fold_unary).

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

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

	* fold-const.c (fold_unary_loc): Don't call fold_negate_expr
	when doing signed integer sanitization or when the type overflow
	wraps.

	* c-c++-common/ubsan/overflow-negate-3.c: New test.


	Marek

Comments

Jakub Jelinek Nov. 11, 2014, 5:49 p.m. UTC | #1 
On Tue, Nov 11, 2014 at 06:40:25PM +0100, Marek Polacek wrote:
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -7862,9 +7862,15 @@ fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>        return fold_view_convert_expr (type, op0);
>  
>      case NEGATE_EXPR:
> -      tem = fold_negate_expr (loc, arg0);
> -      if (tem)
> -	return fold_convert_loc (loc, type, tem);
> +      if (TREE_CODE (arg0) == INTEGER_CST
> +	  || TREE_CODE (arg0) == REAL_CST
> +	  || TYPE_OVERFLOW_WRAPS (type)
> +	  || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
> +	{
> +	  tem = fold_negate_expr (loc, arg0);
> +	  if (tem)
> +	    return fold_convert_loc (loc, type, tem);
> +	}

a) if arg0 is INTEGER_CST, but e.g. INT_MIN, should we
   really fold it for -fsanitize=signed-integer-overflow
   (I'd say in that case we should fold only if it is not
   the minimum value)
b) if the argument is not INTEGRAL_TYPE_P (type), shouldn't
   we fold no matter what flag_sanitize says?  Is the REAL_CST
   case needed in that case?

	Jakub
Richard Biener Nov. 11, 2014, 6:45 p.m. UTC | #2 
On November 11, 2014 6:49:34 PM CET, Jakub Jelinek <jakub@redhat.com> wrote:
>On Tue, Nov 11, 2014 at 06:40:25PM +0100, Marek Polacek wrote:
>> --- gcc/fold-const.c
>> +++ gcc/fold-const.c
>> @@ -7862,9 +7862,15 @@ fold_unary_loc (location_t loc, enum tree_code
>code, tree type, tree op0)
>>        return fold_view_convert_expr (type, op0);
>>  
>>      case NEGATE_EXPR:
>> -      tem = fold_negate_expr (loc, arg0);
>> -      if (tem)
>> -	return fold_convert_loc (loc, type, tem);
>> +      if (TREE_CODE (arg0) == INTEGER_CST
>> +	  || TREE_CODE (arg0) == REAL_CST
>> +	  || TYPE_OVERFLOW_WRAPS (type)
>> +	  || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
>> +	{
>> +	  tem = fold_negate_expr (loc, arg0);
>> +	  if (tem)
>> +	    return fold_convert_loc (loc, type, tem);
>> +	}
>
>a) if arg0 is INTEGER_CST, but e.g. INT_MIN, should we
>   really fold it for -fsanitize=signed-integer-overflow
>   (I'd say in that case we should fold only if it is not
>   the minimum value)

Right. I think you should fix VRP instead.

>b) if the argument is not INTEGRAL_TYPE_P (type), shouldn't
>   we fold no matter what flag_sanitize says?  Is the REAL_CST
>   case needed in that case?

Yes and no I guess.

Richard.

>	Jakub
diff mbox

Patch

diff --git gcc/fold-const.c gcc/fold-const.c
index f3562ff..33311fb 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -7862,9 +7862,15 @@  fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
       return fold_view_convert_expr (type, op0);
 
     case NEGATE_EXPR:
-      tem = fold_negate_expr (loc, arg0);
-      if (tem)
-	return fold_convert_loc (loc, type, tem);
+      if (TREE_CODE (arg0) == INTEGER_CST
+	  || TREE_CODE (arg0) == REAL_CST
+	  || TYPE_OVERFLOW_WRAPS (type)
+	  || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
+	{
+	  tem = fold_negate_expr (loc, arg0);
+	  if (tem)
+	    return fold_convert_loc (loc, type, tem);
+	}
       return NULL_TREE;
 
     case ABS_EXPR:
diff --git gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
index e69de29..cf4f9bc 100644
--- gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
+++ gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
@@ -0,0 +1,17 @@ 
+/* { dg-do run } */
+/* { dg-options "-fsanitize=signed-integer-overflow" } */
+
+#define INT_MIN (-__INT_MAX__ - 1)
+
+int
+main ()
+{
+  volatile int x = INT_MIN;
+  int y = x;
+  y = -(-y);
+  x = y;
+  return 0;
+}
+
+/* { 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]*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)" } */