Patchwork [PING] for for c/PR57541

login
register
mail settings
Submitter Iyer, Balaji V
Date July 2, 2013, 5:07 p.m.
Message ID <BF230D13CA30DD48930C31D4099330003A43CBDA@FMSMSX101.amr.corp.intel.com>
Download mbox | patch
Permalink /patch/256466/
State New
Headers show

Comments

Iyer, Balaji V - July 2, 2013, 5:07 p.m.
Hello everyone,
	Is this Patch OK for trunk?

-Balaji V. Iyer.

> -----Original Message-----
> From: Iyer, Balaji V
> Sent: Monday, June 17, 2013 8:10 AM
> To: gcc-patches@gcc.gnu.org
> Subject: [PING][PATCH] for for c/PR57541
> 
> Hello Everyone,
> 	Is this patch OK for trunk?
> 
> Thanks,
> 
> Balaji V. Iyer.
> 
> > -----Original Message-----
> > From: Iyer, Balaji V
> > Sent: Wednesday, June 12, 2013 1:22 PM
> > To: gcc-patches@gcc.gnu.org
> > Cc: anna.m.tikhonova@gmail.com
> > Subject: [PATCH] for for c/PR57541
> >
> > 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.

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;