diff mbox

for for c/PR57541

Message ID BF230D13CA30DD48930C31D4099330003A42E351@FMSMSX101.amr.corp.intel.com
State New
Headers show

Commit Message

Iyer, Balaji V June 12, 2013, 5:21 p.m. UTC
Hello Everyone,
	Attach, please find a patch that will fix the issues in C/PR57541. The issue reported was that the parameters passed into the builtin array notation reduction functions were not checked correctly. This patch should fix that issue. It is tested on x86 and x86_64 and it seem to pass all the tests. I have also included a testsuite. 

Here are the ChangeLog entries:

gcc/c/ChangeLog
2013-06-12  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-array-notation.c (fix_builtin_array_notation_fn): Added a call to
        valid_no_reduce_fn_params_p and valid_reduce_fn_params_p.
        (build_array_notation_expr): Added a check for capturing the return
        value (i.e. void) of __sec_reduce_mutating function.


gcc/c-family/ChangeLog:
2013-06-12  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * array-notation-common.c (valid_reduce_fn_params_p): New function.
        (valid_no_reduce_fn_params_p): Likewise.
        * c-common.h (valid_reduce_fn_params_p): Added a new prototype.
        (valid_no_reduce_fn_params_p): Likewise.

gcc/testsuite/ChangeLog
2013-06-12  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        PR c/57541
        * c-c++-common/cilk-plus/AN/pr57541-2.c: New test.
        * c-c++-common/cilk-plus/AN/rank_mismatch2.c: Fixed a bug by replacing
        a comma with an operation.


Thanks,

Balaji V. Iyer.
diff mbox

Patch

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
old mode 100644
new mode 100755
index 3d8f68f..52a58a6
Binary files a/gcc/c-family/ChangeLog and b/gcc/c-family/ChangeLog differ
diff --git a/gcc/c-family/array-notation-common.c b/gcc/c-family/array-notation-common.c
old mode 100644
new mode 100755
index 489b67c..6c1c7e2
--- a/gcc/c-family/array-notation-common.c
+++ b/gcc/c-family/array-notation-common.c
@@ -560,3 +560,125 @@  find_correct_array_notation_type (tree op)
     } 
   return return_type;
 }
+
+/* Returns true if the function call in BUILTIN_FN (of type CALL_EXPR) if the
+   number of parameters for the array notation reduction functions are
+   correct.  */
+
+bool
+valid_no_reduce_fn_params_p (tree builtin_fn)
+{
+  switch (is_cilkplus_reduce_builtin (CALL_EXPR_FN (builtin_fn)))
+    {
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ADD:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MUL:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
+      if (call_expr_nargs (builtin_fn) != 1)
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "builtin function %qE can only have one argument",
+		    CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      break;
+    case BUILT_IN_CILKPLUS_SEC_REDUCE:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING:
+      if (call_expr_nargs (builtin_fn) != 3)
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "builtin function %qE must have 3 arguments",
+		    CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      break;
+    default:
+      /* If it is not a builtin function, then no reason for us do any checking
+	 here.  */
+      return true;
+    }
+  return true;
+}
+
+/* Returns true if the parameters of BUILTIN_FN (array notation builtin
+   function): IDENTITY_VALUE and FUNC_PARM are valid.  */
+
+bool
+valid_reduce_fn_params_p (tree builtin_fn, tree identity_value, tree func_parm)
+{
+  switch (is_cilkplus_reduce_builtin (CALL_EXPR_FN (builtin_fn)))
+    {
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ADD:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MUL:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
+      func_parm = CALL_EXPR_ARG (builtin_fn, 0);
+      if (!contains_array_notation_expr (func_parm))
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "builtin function %qE must contain one argument with "
+		    "array notations", CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      if (TREE_CODE (func_parm) == CALL_EXPR
+	  && is_cilkplus_reduce_builtin (CALL_EXPR_FN (func_parm)) !=
+	  BUILT_IN_NONE)
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "builtin functions cannot be used as arguments for %qE",
+		    CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      return true;
+    case BUILT_IN_CILKPLUS_SEC_REDUCE:
+      if (!contains_array_notation_expr (func_parm))
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "second argument of %qE must contain array notation "
+		    "expressions", CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      if (TREE_CODE (func_parm) == CALL_EXPR
+	  && is_cilkplus_reduce_builtin (CALL_EXPR_FN (func_parm)) !=
+	  BUILT_IN_NONE)
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "builtin functions cannot be used as arguments for %qE",
+		    CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      return true;
+    case BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING:
+      if (!contains_array_notation_expr (func_parm))
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "builtin function %qE must contain one argument with array "
+		    "notations", CALL_EXPR_FN (builtin_fn));
+	  return false;
+	}
+      if (TREE_CODE (identity_value) != ADDR_EXPR 
+	  && !POINTER_TYPE_P (TREE_TYPE (identity_value)))
+	{
+	  error_at (EXPR_LOCATION (builtin_fn),
+		    "identity value of __sec_reduce function must be a "
+		    "pointer or reference");
+	  return false;
+	}      
+      return true;
+    default:
+      return true;
+    }
+  return true;
+}
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
old mode 100644
new mode 100755
index 8eaf54f..74a1c6f
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1179,4 +1179,6 @@  extern void replace_array_notations (tree *, bool, vec<tree, va_gc> *,
 extern tree find_inv_trees (tree *, int *, void *);
 extern tree replace_inv_trees (tree *, int *, void *);
 extern tree find_correct_array_notation_type (tree op);
+extern bool valid_no_reduce_fn_params_p (tree);
+extern bool valid_reduce_fn_params_p (tree, tree, tree);
 #endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
old mode 100644
new mode 100755
index 72d182b..ea79745
Binary files a/gcc/c/ChangeLog and b/gcc/c/ChangeLog differ
diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
old mode 100644
new mode 100755
index 3285969..22ebd5f
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -139,6 +139,11 @@  fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
   if (an_type == BUILT_IN_NONE)
     return NULL_TREE;
 
+  /* Checking if the number of parameters passed into the reduction function
+     is valid.  */
+  if (!valid_no_reduce_fn_params_p (an_builtin_fn))
+    return error_mark_node;
+
   if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE
       || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
     {
@@ -154,7 +159,12 @@  fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
   /* Fully fold any EXCESSIVE_PRECISION EXPR that can occur in the function
      parameter.  */
   func_parm = c_fully_fold (func_parm, false, NULL);
-  
+
+  /* Checking if the parameter types passed into the reduction function is
+     valid.  */
+  if (!valid_reduce_fn_params_p (an_builtin_fn, identity_value, func_parm))
+    return error_mark_node;
+
   location = EXPR_LOCATION (an_builtin_fn);
   
   if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank))
@@ -721,7 +731,14 @@  build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
       tree rhs_node = (*rhs_list)[ii];
       if (TREE_CODE (rhs_node) == CALL_EXPR)
 	{
-	  builtin_loop = fix_builtin_array_notation_fn (rhs_node, &new_var);
+	  if (is_cilkplus_reduce_builtin (CALL_EXPR_FN (rhs_node)) ==
+	      BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
+	    { 
+	      error_at (location, "void value not ignored as it ought to be");
+	      builtin_loop = error_mark_node;
+	    }
+	  else
+	    builtin_loop = fix_builtin_array_notation_fn (rhs_node, &new_var);
 	  if (builtin_loop == error_mark_node)
 	    {
 	      pop_stmt_list (an_init); 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
old mode 100644
new mode 100755
index 187f2ef..65d7576
Binary files a/gcc/testsuite/ChangeLog and b/gcc/testsuite/ChangeLog differ
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c
new file mode 100644
index 0000000..b0e67bf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c
@@ -0,0 +1,22 @@ 
+int A[10];
+int j = 0;
+int main () {
+  int a;
+  int *q;
+  const int c = 5;
+  int my_func (int, int);
+  int my_func2 (int *, int);
+  a = __sec_reduce_add (1); /* { dg-error "must contain one argument with" } */
+  a = __sec_reduce_mul (A[5]); /* { dg-error "must contain one argument with" } */
+  a = __sec_reduce_max (A[:], 2); /* { dg-error "can only have one argument" } */
+  a = __sec_reduce (1);  /* { dg-error "must have 3 arguments" } */
+  a = __sec_reduce (1, 2, 3); /* { dg-error "must contain array notation" } */
+  a = __sec_reduce (j, A[:], my_func);        /*  This is OK.  */
+  a = __sec_reduce (c, A[:], my_func);        /*  This is OK.  */
+  __sec_reduce_mutating (&j, A[:], my_func2); /*  This IS OK.  */
+  __sec_reduce_mutating (j, A[:], my_func2); /* { dg-error "pointer or" } */
+  __sec_reduce_mutating (*q, A[:], my_func2); /* { dg-error "pointer or" } */
+  a =  __sec_reduce_mutating (&j, A[:], my_func2);  /*  { dg-error "void value not ignored as it ought to be" } */
+  a = __sec_reduce_add (__sec_reduce_add (A[:])); /* { dg-error "builtin functions cannot be used as arguments for" } */
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c
index 4a4882d..9346726 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c
@@ -20,7 +20,7 @@  int main (void)
 
   argc += function_call (function_call (array[:] + array2[5:10:2][:])); /* { dg-error "rank mismatch between" } */
 
-   argc += __sec_reduce_add (array[:], array2[:][:]); /* { dg-error "rank mismatch between" } */
+   argc += __sec_reduce_add (array[:] / array2[:][:]); /* { dg-error "rank mismatch between" } */
 
    argc += __sec_reduce_add (array2[:][:]) + argc; /* This is OK.  */
   return argc;