diff mbox

[PR67921] Convert pointer expr to proper type before negating it

Message ID HE1PR08MB05071C7816FD00F60A9AEBE9E7DB0@HE1PR08MB0507.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Bin Cheng Jan. 29, 2016, 4:21 p.m. UTC 
Hi,
Function fold_binary_loc calls split_tree to split a tree into constant, literal and variable parts.  Function split_tree deals with minus_expr by negating different parts into NEXGATE_EXPR.  Since tree exprs fed to split_tree are with NOP conversions stripped, this could result in illegal expr for pointer expressions.  Given below example as described by PR67921:
  op0:  (4 - (sizetype) &c)
  code: MINUS_EXPR
  op1:  (sizetype)b

fold_binary_loc calls split_tree for both op0 and op1 and gets below from the function calls (it also flips the code):
  op0:  4, -(sizetype)&c
  code: PLUS_EXPR
  op1:  -b

Here we generate NEGATIVE_EXPR of pointer variable (b) which is illegal.  If "-b" can not be canceled by following call to associate_trees, it will be passed along in IR resulting in ICE somewhere.

This patch fixes it by converting pointer expression to proper type before negating it.  Note the proper type is the outer type stripped in fold_binary_loc before calling split_tree.  I also included a test which is heavily reduced from the original ffmpeg code in the original PR.  Considering it's stage4, I restricted the patch to the smallest change.  As a matter of fact, we may need to do the same thing for signed int types because -TYPE_MIN is undefined.  Unfortunately, I failed to create a test in this case.

Bootstrap and test on x64_64, is it OK?

2016-01-27  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/67921
	* fold-const.c (split_tree): New parameters.  Convert pointer
	type variable part to proper type before negating. 
	(fold_binary_loc): Pass new arguments to split_tree.

gcc/testsuite/ChangeLog
2016-01-27  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/67921
	* c-c++-common/ubsan/pr67921.c: New test.

Comments

Richard Biener Feb. 1, 2016, 11:53 a.m. UTC | #1 
On Fri, Jan 29, 2016 at 5:21 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> Function fold_binary_loc calls split_tree to split a tree into constant, literal and variable parts.  Function split_tree deals with minus_expr by negating different parts into NEXGATE_EXPR.  Since tree exprs fed to split_tree are with NOP conversions stripped, this could result in illegal expr for pointer expressions.  Given below example as described by PR67921:
>   op0:  (4 - (sizetype) &c)
>   code: MINUS_EXPR
>   op1:  (sizetype)b
>
> fold_binary_loc calls split_tree for both op0 and op1 and gets below from the function calls (it also flips the code):
>   op0:  4, -(sizetype)&c
>   code: PLUS_EXPR
>   op1:  -b
>
> Here we generate NEGATIVE_EXPR of pointer variable (b) which is illegal.  If "-b" can not be canceled by following call to associate_trees, it will be passed along in IR resulting in ICE somewhere.
>
> This patch fixes it by converting pointer expression to proper type before negating it.  Note the proper type is the outer type stripped in fold_binary_loc before calling split_tree.  I also included a test which is heavily reduced from the original ffmpeg code in the original PR.  Considering it's stage4, I restricted the patch to the smallest change.  As a matter of fact, we may need to do the same thing for signed int types because -TYPE_MIN is undefined.  Unfortunately, I failed to create a test in this case.
>
> Bootstrap and test on x64_64, is it OK?

Ok.

Thanks,
Richard.

> 2016-01-27  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/67921
>         * fold-const.c (split_tree): New parameters.  Convert pointer
>         type variable part to proper type before negating.
>         (fold_binary_loc): Pass new arguments to split_tree.
>
> gcc/testsuite/ChangeLog
> 2016-01-27  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/67921
>         * c-c++-common/ubsan/pr67921.c: New test.
>
>
diff mbox

Patch

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index bece8d7..e34bc81 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -109,7 +109,8 @@  enum comparison_code {
 
 static bool negate_expr_p (tree);
 static tree negate_expr (tree);
-static tree split_tree (tree, enum tree_code, tree *, tree *, tree *, int);
+static tree split_tree (location_t, tree, tree, enum tree_code,
+			tree *, tree *, tree *, int);
 static tree associate_trees (location_t, tree, tree, enum tree_code, tree);
 static enum comparison_code comparison_to_compcode (enum tree_code);
 static enum tree_code compcode_to_comparison (enum comparison_code);
@@ -767,7 +768,10 @@  negate_expr (tree t)
    literal for which we use *MINUS_LITP instead.
 
    If NEGATE_P is true, we are negating all of IN, again except a literal
-   for which we use *MINUS_LITP instead.
+   for which we use *MINUS_LITP instead.  If a variable part is of pointer
+   type, it is negated after converting to TYPE.  This prevents us from
+   generating illegal MINUS pointer expression.  LOC is the location of
+   the converted variable part.
 
    If IN is itself a literal or constant, return it as appropriate.
 
@@ -775,8 +779,8 @@  negate_expr (tree t)
    same type as IN, but they will have the same signedness and mode.  */
 
 static tree
-split_tree (tree in, enum tree_code code, tree *conp, tree *litp,
-	    tree *minus_litp, int negate_p)
+split_tree (location_t loc, tree in, tree type, enum tree_code code,
+	    tree *conp, tree *litp, tree *minus_litp, int negate_p)
 {
   tree var = 0;
 
@@ -833,7 +837,12 @@  split_tree (tree in, enum tree_code code, tree *conp, tree *litp,
       if (neg_conp_p)
 	*conp = negate_expr (*conp);
       if (neg_var_p)
-	var = negate_expr (var);
+	{
+	  /* Convert to TYPE before negating a pointer type expr.  */
+	  if (var && POINTER_TYPE_P (TREE_TYPE (var)))
+	    var = fold_convert_loc (loc, type, var);
+	  var = negate_expr (var);
+	}
     }
   else if (TREE_CODE (in) == BIT_NOT_EXPR
 	   && code == PLUS_EXPR)
@@ -854,6 +863,9 @@  split_tree (tree in, enum tree_code code, tree *conp, tree *litp,
       else if (*minus_litp)
 	*litp = *minus_litp, *minus_litp = 0;
       *conp = negate_expr (*conp);
+      /* Convert to TYPE before negating a pointer type expr.  */
+      if (var && POINTER_TYPE_P (TREE_TYPE (var)))
+	var = fold_convert_loc (loc, type, var);
       var = negate_expr (var);
     }
 
@@ -9621,9 +9633,10 @@  fold_binary_loc (location_t loc,
 	     then the result with variables.  This increases the chances of
 	     literals being recombined later and of generating relocatable
 	     expressions for the sum of a constant and literal.  */
-	  var0 = split_tree (arg0, code, &con0, &lit0, &minus_lit0, 0);
-	  var1 = split_tree (arg1, code, &con1, &lit1, &minus_lit1,
-			     code == MINUS_EXPR);
+	  var0 = split_tree (loc, arg0, type, code,
+			     &con0, &lit0, &minus_lit0, 0);
+	  var1 = split_tree (loc, arg1, type, code,
+			     &con1, &lit1, &minus_lit1, code == MINUS_EXPR);
 
 	  /* Recombine MINUS_EXPR operands by using PLUS_EXPR.  */
 	  if (code == MINUS_EXPR)
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr67921.c b/gcc/testsuite/c-c++-common/ubsan/pr67921.c
new file mode 100644
index 0000000..728ff93
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/pr67921.c
@@ -0,0 +1,23 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined" } */
+
+struct s
+{
+  int n;
+  int arr[][6];
+};
+void bar (int);
+void foo (struct s *ptr)
+{
+  int i;
+  for (; i < 2; i++)
+    for (; ptr->n;)
+      {
+	int *a = ptr->arr[i];
+	int b[66];
+	int j = 0;
+
+	for (; j < 56; j++)
+	  bar (a[j] - b[j]);
+    }
+}