Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-04-25 Thread Jeff Law 

On 02/17/14 10:51, Iyer, Balaji V wrote:

I don't know Cilk stuff at all, but it seems that _Cilk_spawn is forbidden in a
return statement.  But then only checking TREE_CODE (retval) ==
CILK_SPAWN_STMT isn't sufficient, because CILK_SPAWN_STMT can be
wrapped e.g. in MINUS_EXPR, C_MAYBE_CONST_EXPR, EQ_EXPR, and a
bunch of others.  I used walk_tree for checking whether a return statement
contains any CILK_SPAWN_STMT.

Regtested/bootstrapped on x86_64-linux, ok for 5.0?



5.0? you mean 4.9 right?... since this is a minor bug-fix.
Fixes go onto the trunk with selected fixes being backported to the 4.9 
branch at the discretion of the release managers.


jeff

Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-03-05 Thread Jeff Law 

On 02/26/14 08:21, Marek Polacek wrote:

Ping.

Based on Balaji's comments, OK.

jeff

RE: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-03-04 Thread Iyer, Balaji V 


 -Original Message-
 From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
 ow...@gcc.gnu.org] On Behalf Of Marek Polacek
 Sent: Wednesday, February 19, 2014 8:39 AM
 To: Iyer, Balaji V
 Cc: gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR
 c/60197)
 
 On Tue, Feb 18, 2014 at 10:06:14PM +, Iyer, Balaji V wrote:
  This is invalid.
 
 Thanks.  In that case, this patch should error out on such invalid uses as 
 well,
 instead of ICEing.
 
 Regtested/bootstrapped on x86_64-linux.
 
 2014-02-19  Marek Polacek  pola...@redhat.com
 
   PR c/60197
 c-family/
   * cilk.c (contains_cilk_spawn_stmt): New function.
   (contains_cilk_spawn_stmt_walker): Likewise.
   (recognize_spawn): Give error on invalid use of _Cilk_spawn.
   * c-common.h (contains_cilk_spawn_stmt): Add declaration.
 c/
   * c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
   of checking tree code.
 cp/
   * typeck.c (check_return_expr): Call contains_cilk_spawn_stmt
 instead
   of checking tree code.
 testsuite/
   * c-c++-common/cilk-plus/CK/pr60197.c: New test.
   * c-c++-common/cilk-plus/CK/pr60197-2.c: New test.
 
 diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index
 f074ab1..1099b10 100644
 --- gcc/c-family/c-common.h
 +++ gcc/c-family/c-common.h
 @@ -1389,4 +1389,5 @@ extern tree make_cilk_frame (tree);  extern tree
 create_cilk_function_exit (tree, bool, bool);  extern tree
 cilk_install_body_pedigree_operations (tree);  extern void cilk_outline (tree,
 tree *, void *);
 +extern bool contains_cilk_spawn_stmt (tree);
  #endif /* ! GCC_C_COMMON_H */
 diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c index f2179dfc..6a7bf4f 
 100644
 --- gcc/c-family/cilk.c
 +++ gcc/c-family/cilk.c
 @@ -235,6 +235,9 @@ recognize_spawn (tree exp, tree *exp0)
walk_tree (exp0, unwrap_cilk_spawn_stmt, NULL, NULL);
spawn_found = true;
  }
 +  /* _Cilk_spawn can't be wrapped in expression such as PLUS_EXPR.  */
 + else if (contains_cilk_spawn_stmt (exp))
 +error (invalid use of %_Cilk_spawn%);
return spawn_found;
  }
 
 @@ -1292,3 +1295,25 @@ build_cilk_sync (void)
TREE_SIDE_EFFECTS (sync) = 1;
return sync;
  }
 +
 +/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
 +   non-null tree if TP contains CILK_SPAWN_STMT.  */
 +
 +static tree
 +contains_cilk_spawn_stmt_walker (tree *tp, int *, void *) {
 +  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
 +return *tp;
 +  else
 +return NULL_TREE;
 +}
 +
 +/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
 +   node.  */
 +
 +bool
 +contains_cilk_spawn_stmt (tree expr)
 +{
 +  return walk_tree (expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
 +  != NULL_TREE;
 +}
 diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 2b54290..7c4ba0e 100644
 --- gcc/c/c-typeck.c
 +++ gcc/c/c-typeck.c
 @@ -9140,7 +9140,7 @@ c_finish_return (location_t loc, tree retval, tree
 origtype)
 return error_mark_node;
   }
  }
 -  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
 +  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
  {
error_at (loc, use of %_Cilk_spawn% in a return statement is not 
   allowed);
 diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 5fc0e6b..566411f 100644
 --- gcc/cp/typeck.c
 +++ gcc/cp/typeck.c
 @@ -8328,7 +8328,7 @@ check_return_expr (tree retval, bool *no_warning)
 
*no_warning = false;
 
 -  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
 +  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
  {
error_at (EXPR_LOCATION (retval), use of %_Cilk_spawn% in a return
 
   statement is not allowed);
 diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 index e69de29..1e5ca00 100644
 --- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 +++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 @@ -0,0 +1,35 @@
 +/* PR c/60197 */
 +/* { dg-do compile } */
 +/* { dg-options -fcilkplus } */
 +
 +extern int foo (void);
 +
 +int
 +fn1 (void)
 +{
 +  int i;
 +  i = (_Cilk_spawn foo ()) + 1; /* { dg-error invalid use of } */
 +  return i;
 +}
 +
 +int
 +fn2 (void)
 +{
 +  int i = (_Cilk_spawn foo ()) + 1; /* { dg-error invalid use of } */
 +  return i;
 +}
 +
 +int
 +fn3 (int j, int k, int l)
 +{
 +  int i = (_Cilk_spawn foo ()) + 1) - l) * k) / j); /* { dg-error
 +invalid use of } */
 +  return i;
 +}
 +
 +int
 +fn4 (int j, int k, int l)
 +{
 +  int i;
 +  i = (_Cilk_spawn foo ()) + 1) - l) * k) / j); /* { dg-error
 +invalid use of } */
 +  return i;
 +}
 diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c gcc/testsuite/c-
 c++-common/cilk-plus/CK/pr60197.c
 index e69de29..2b47d1e 100644
 --- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 +++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 @@ -0,0

Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-02-26 Thread Marek Polacek 
Ping.

On Wed, Feb 19, 2014 at 02:38:50PM +0100, Marek Polacek wrote:
 On Tue, Feb 18, 2014 at 10:06:14PM +, Iyer, Balaji V wrote:
  This is invalid.
 
 Thanks.  In that case, this patch should error out on such invalid uses as
 well, instead of ICEing.
 
 Regtested/bootstrapped on x86_64-linux.
 
 2014-02-19  Marek Polacek  pola...@redhat.com
 
   PR c/60197
 c-family/
   * cilk.c (contains_cilk_spawn_stmt): New function.
   (contains_cilk_spawn_stmt_walker): Likewise.
   (recognize_spawn): Give error on invalid use of _Cilk_spawn.
   * c-common.h (contains_cilk_spawn_stmt): Add declaration.
 c/
   * c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
   of checking tree code.
 cp/
   * typeck.c (check_return_expr): Call contains_cilk_spawn_stmt instead
   of checking tree code.
 testsuite/
   * c-c++-common/cilk-plus/CK/pr60197.c: New test.
   * c-c++-common/cilk-plus/CK/pr60197-2.c: New test.
 
 diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
 index f074ab1..1099b10 100644
 --- gcc/c-family/c-common.h
 +++ gcc/c-family/c-common.h
 @@ -1389,4 +1389,5 @@ extern tree make_cilk_frame (tree);
  extern tree create_cilk_function_exit (tree, bool, bool);
  extern tree cilk_install_body_pedigree_operations (tree);
  extern void cilk_outline (tree, tree *, void *);
 +extern bool contains_cilk_spawn_stmt (tree);
  #endif /* ! GCC_C_COMMON_H */
 diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c
 index f2179dfc..6a7bf4f 100644
 --- gcc/c-family/cilk.c
 +++ gcc/c-family/cilk.c
 @@ -235,6 +235,9 @@ recognize_spawn (tree exp, tree *exp0)
walk_tree (exp0, unwrap_cilk_spawn_stmt, NULL, NULL);
spawn_found = true;
  }
 +  /* _Cilk_spawn can't be wrapped in expression such as PLUS_EXPR.  */
 +  else if (contains_cilk_spawn_stmt (exp))
 +error (invalid use of %_Cilk_spawn%);
return spawn_found;
  }
  
 @@ -1292,3 +1295,25 @@ build_cilk_sync (void)
TREE_SIDE_EFFECTS (sync) = 1;
return sync;
  }
 +
 +/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
 +   non-null tree if TP contains CILK_SPAWN_STMT.  */
 +
 +static tree
 +contains_cilk_spawn_stmt_walker (tree *tp, int *, void *)
 +{
 +  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
 +return *tp;
 +  else
 +return NULL_TREE;
 +}
 +
 +/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
 +   node.  */
 +
 +bool
 +contains_cilk_spawn_stmt (tree expr)
 +{
 +  return walk_tree (expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
 +  != NULL_TREE;
 +}
 diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
 index 2b54290..7c4ba0e 100644
 --- gcc/c/c-typeck.c
 +++ gcc/c/c-typeck.c
 @@ -9140,7 +9140,7 @@ c_finish_return (location_t loc, tree retval, tree 
 origtype)
 return error_mark_node;
   }
  }
 -  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
 +  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
  {
error_at (loc, use of %_Cilk_spawn% in a return statement is not 
   allowed);
 diff --git gcc/cp/typeck.c gcc/cp/typeck.c
 index 5fc0e6b..566411f 100644
 --- gcc/cp/typeck.c
 +++ gcc/cp/typeck.c
 @@ -8328,7 +8328,7 @@ check_return_expr (tree retval, bool *no_warning)
  
*no_warning = false;
  
 -  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
 +  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
  {
error_at (EXPR_LOCATION (retval), use of %_Cilk_spawn% in a return 
   statement is not allowed);
 diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c 
 gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 index e69de29..1e5ca00 100644
 --- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 +++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
 @@ -0,0 +1,35 @@
 +/* PR c/60197 */
 +/* { dg-do compile } */
 +/* { dg-options -fcilkplus } */
 +
 +extern int foo (void);
 +
 +int
 +fn1 (void)
 +{
 +  int i;
 +  i = (_Cilk_spawn foo ()) + 1; /* { dg-error invalid use of } */
 +  return i;
 +}
 +
 +int
 +fn2 (void)
 +{
 +  int i = (_Cilk_spawn foo ()) + 1; /* { dg-error invalid use of } */
 +  return i;
 +}
 +
 +int
 +fn3 (int j, int k, int l)
 +{
 +  int i = (_Cilk_spawn foo ()) + 1) - l) * k) / j); /* { dg-error 
 invalid use of } */
 +  return i;
 +}
 +
 +int
 +fn4 (int j, int k, int l)
 +{
 +  int i;
 +  i = (_Cilk_spawn foo ()) + 1) - l) * k) / j); /* { dg-error invalid 
 use of } */
 +  return i;
 +}
 diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c 
 gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 index e69de29..2b47d1e 100644
 --- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 +++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 @@ -0,0 +1,66 @@
 +/* PR c/60197 */
 +/* { dg-do compile } */
 +/* { dg-options -fcilkplus } */
 +
 +extern int foo (void);
 +extern int bar (int);
 +
 +int
 +fn1 (void)
 +{
 +  return (_Cilk_spawn foo ()) * 2; /* { dg-error in a return

Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-02-19 Thread Marek Polacek 
On Tue, Feb 18, 2014 at 10:06:14PM +, Iyer, Balaji V wrote:
 This is invalid.

Thanks.  In that case, this patch should error out on such invalid uses as
well, instead of ICEing.

Regtested/bootstrapped on x86_64-linux.

2014-02-19  Marek Polacek  pola...@redhat.com

PR c/60197
c-family/
* cilk.c (contains_cilk_spawn_stmt): New function.
(contains_cilk_spawn_stmt_walker): Likewise.
(recognize_spawn): Give error on invalid use of _Cilk_spawn.
* c-common.h (contains_cilk_spawn_stmt): Add declaration.
c/
* c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
of checking tree code.
cp/
* typeck.c (check_return_expr): Call contains_cilk_spawn_stmt instead
of checking tree code.
testsuite/
* c-c++-common/cilk-plus/CK/pr60197.c: New test.
* c-c++-common/cilk-plus/CK/pr60197-2.c: New test.

diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index f074ab1..1099b10 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1389,4 +1389,5 @@ extern tree make_cilk_frame (tree);
 extern tree create_cilk_function_exit (tree, bool, bool);
 extern tree cilk_install_body_pedigree_operations (tree);
 extern void cilk_outline (tree, tree *, void *);
+extern bool contains_cilk_spawn_stmt (tree);
 #endif /* ! GCC_C_COMMON_H */
diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c
index f2179dfc..6a7bf4f 100644
--- gcc/c-family/cilk.c
+++ gcc/c-family/cilk.c
@@ -235,6 +235,9 @@ recognize_spawn (tree exp, tree *exp0)
   walk_tree (exp0, unwrap_cilk_spawn_stmt, NULL, NULL);
   spawn_found = true;
 }
+  /* _Cilk_spawn can't be wrapped in expression such as PLUS_EXPR.  */
+  else if (contains_cilk_spawn_stmt (exp))
+error (invalid use of %_Cilk_spawn%);
   return spawn_found;
 }
 
@@ -1292,3 +1295,25 @@ build_cilk_sync (void)
   TREE_SIDE_EFFECTS (sync) = 1;
   return sync;
 }
+
+/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
+   non-null tree if TP contains CILK_SPAWN_STMT.  */
+
+static tree
+contains_cilk_spawn_stmt_walker (tree *tp, int *, void *)
+{
+  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
+return *tp;
+  else
+return NULL_TREE;
+}
+
+/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
+   node.  */
+
+bool
+contains_cilk_spawn_stmt (tree expr)
+{
+  return walk_tree (expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
+!= NULL_TREE;
+}
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 2b54290..7c4ba0e 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9140,7 +9140,7 @@ c_finish_return (location_t loc, tree retval, tree 
origtype)
  return error_mark_node;
}
 }
-  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
 {
   error_at (loc, use of %_Cilk_spawn% in a return statement is not 
allowed);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 5fc0e6b..566411f 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -8328,7 +8328,7 @@ check_return_expr (tree retval, bool *no_warning)
 
   *no_warning = false;
 
-  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
 {
   error_at (EXPR_LOCATION (retval), use of %_Cilk_spawn% in a return 
statement is not allowed);
diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c 
gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
index e69de29..1e5ca00 100644
--- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
+++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197-2.c
@@ -0,0 +1,35 @@
+/* PR c/60197 */
+/* { dg-do compile } */
+/* { dg-options -fcilkplus } */
+
+extern int foo (void);
+
+int
+fn1 (void)
+{
+  int i;
+  i = (_Cilk_spawn foo ()) + 1; /* { dg-error invalid use of } */
+  return i;
+}
+
+int
+fn2 (void)
+{
+  int i = (_Cilk_spawn foo ()) + 1; /* { dg-error invalid use of } */
+  return i;
+}
+
+int
+fn3 (int j, int k, int l)
+{
+  int i = (_Cilk_spawn foo ()) + 1) - l) * k) / j); /* { dg-error invalid 
use of } */
+  return i;
+}
+
+int
+fn4 (int j, int k, int l)
+{
+  int i;
+  i = (_Cilk_spawn foo ()) + 1) - l) * k) / j); /* { dg-error invalid use 
of } */
+  return i;
+}
diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c 
gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
index e69de29..2b47d1e 100644
--- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
+++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
@@ -0,0 +1,66 @@
+/* PR c/60197 */
+/* { dg-do compile } */
+/* { dg-options -fcilkplus } */
+
+extern int foo (void);
+extern int bar (int);
+
+int
+fn1 (void)
+{
+  return (_Cilk_spawn foo ()) * 2; /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn2 (void)
+{
+  return (_Cilk_spawn foo ())  2; /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn3 (int i, int j, int k)
+{
+  return

RE: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-02-18 Thread Iyer, Balaji V 
 Yeah, it passed regtesting.  Note that we also ICE on e.g.
 int
 foo (void)
 {
   int i;
   i = (_Cilk_spawn foo ()) + 1;
   return i;
 }
 
 I don't know whether this is valid use of _Cilk_spawn though.  In any case,
 this patch addresses only _Cilk_spawn in return statements.

This is invalid.

Thanks,

Balaji V. Iyer.

[PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-02-17 Thread Marek Polacek 
I don't know Cilk stuff at all, but it seems that _Cilk_spawn is
forbidden in a return statement.  But then only checking
TREE_CODE (retval) == CILK_SPAWN_STMT isn't sufficient, because
CILK_SPAWN_STMT can be wrapped e.g. in MINUS_EXPR, C_MAYBE_CONST_EXPR,
EQ_EXPR, and a bunch of others.  I used walk_tree for checking whether
a return statement contains any CILK_SPAWN_STMT.

Regtested/bootstrapped on x86_64-linux, ok for 5.0?

2014-02-17  Marek Polacek  pola...@redhat.com

PR c/60197
c-family/
* array-notation-common.c (contains_cilk_spawn_stmt): New function.
(contains_cilk_spawn_stmt_walker): Likewise.
* c-common.h (contains_cilk_spawn_stmt): Add declaration.
c/
* c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
of checking tree code.
cp/
* typeck.c (check_return_expr): Call contains_cilk_spawn_stmt instead
of checking tree code.
testsuite/
* c-c++-common/cilk-plus/CK/pr60197.c: New test.

diff --git gcc/c-family/array-notation-common.c 
gcc/c-family/array-notation-common.c
index c010039..5d1e2c8 100644
--- gcc/c-family/array-notation-common.c
+++ gcc/c-family/array-notation-common.c
@@ -657,3 +657,25 @@ fix_sec_implicit_args (location_t loc, vec tree, va_gc 
*list,
   vec_safe_push (array_operand, (*list)[ii]);
   return array_operand;
 }
+
+/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
+   non-null tree if TP contains CILK_SPAWN_STMT.  */
+
+static tree
+contains_cilk_spawn_stmt_walker (tree *tp, int *, void *)
+{
+  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
+return *tp;
+  else
+return NULL_TREE;
+}
+
+/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
+   node.  */
+
+bool
+contains_cilk_spawn_stmt (tree expr)
+{
+  return walk_tree (expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
+!= NULL_TREE;
+}
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index f074ab1..6f2c6b7 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1375,6 +1375,7 @@ extern void cilkplus_extract_an_triplets (vectree, 
va_gc *, size_t, size_t,
  vecvecan_parts  *);
 extern vec tree, va_gc *fix_sec_implicit_args
   (location_t, vec tree, va_gc *, vecan_loop_parts, size_t, tree);
+extern bool contains_cilk_spawn_stmt (tree);
 
 /* In cilk.c.  */
 extern tree insert_cilk_frame (tree);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index da6a6fc..23cdb0e 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9134,7 +9134,7 @@ c_finish_return (location_t loc, tree retval, tree 
origtype)
  return error_mark_node;
}
 }
-  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
 {
   error_at (loc, use of %_Cilk_spawn% in a return statement is not 
allowed);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 5fc0e6b..566411f 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -8328,7 +8328,7 @@ check_return_expr (tree retval, bool *no_warning)
 
   *no_warning = false;
 
-  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
 {
   error_at (EXPR_LOCATION (retval), use of %_Cilk_spawn% in a return 
statement is not allowed);
diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c 
gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
index e69de29..2b47d1e 100644
--- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
+++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
@@ -0,0 +1,66 @@
+/* PR c/60197 */
+/* { dg-do compile } */
+/* { dg-options -fcilkplus } */
+
+extern int foo (void);
+extern int bar (int);
+
+int
+fn1 (void)
+{
+  return (_Cilk_spawn foo ()) * 2; /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn2 (void)
+{
+  return (_Cilk_spawn foo ())  2; /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn3 (int i, int j, int k)
+{
+  return ((_Cilk_spawn foo () + i) - j) * k) / j) | i) ^ k) ; /* { 
dg-error in a return statement is not allowed } */
+}
+
+int
+fn4 (int i, int j, int k)
+{
+  return (i - _Cilk_spawn foo ()) * k) / j) | i) ^ k); /* { dg-error in a 
return statement is not allowed } */
+}
+
+int
+fn5 (void)
+{
+  return _Cilk_spawn foo (); /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn6 (void)
+{
+  return _Cilk_spawn foo () + _Cilk_spawn foo (); /* { dg-error in a return 
statement is not allowed } */
+}
+
+int
+fn7 (void)
+{
+  return 5 % _Cilk_spawn foo (); /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn8 (void)
+{
+  return !_Cilk_spawn foo (); /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn9 (void)
+{
+  return foo ()  _Cilk_spawn foo (); /* { dg-error in a return statement is 
not allowed } */
+}
+
+int
+fn10 (void)
+{
+  return bar (_Cilk_spawn foo ()); /* { dg-error

RE: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-02-17 Thread Iyer, Balaji V 
Hi Marek,
Thanks for working on this. Please see my comments below.

 -Original Message-
 From: Marek Polacek [mailto:pola...@redhat.com]
 Sent: Monday, February 17, 2014 12:43 PM
 To: GCC Patches
 Cc: Iyer, Balaji V
 Subject: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)
 
 I don't know Cilk stuff at all, but it seems that _Cilk_spawn is forbidden in 
 a
 return statement.  But then only checking TREE_CODE (retval) ==
 CILK_SPAWN_STMT isn't sufficient, because CILK_SPAWN_STMT can be
 wrapped e.g. in MINUS_EXPR, C_MAYBE_CONST_EXPR, EQ_EXPR, and a
 bunch of others.  I used walk_tree for checking whether a return statement
 contains any CILK_SPAWN_STMT.
 
 Regtested/bootstrapped on x86_64-linux, ok for 5.0?


5.0? you mean 4.9 right?... since this is a minor bug-fix.

 
 2014-02-17  Marek Polacek  pola...@redhat.com
 
   PR c/60197
 c-family/
   * array-notation-common.c (contains_cilk_spawn_stmt): New
 function.
   (contains_cilk_spawn_stmt_walker): Likewise.
   * c-common.h (contains_cilk_spawn_stmt): Add declaration.
 c/
   * c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
   of checking tree code.
 cp/
   * typeck.c (check_return_expr): Call contains_cilk_spawn_stmt
 instead
   of checking tree code.
 testsuite/
   * c-c++-common/cilk-plus/CK/pr60197.c: New test.
 
 diff --git gcc/c-family/array-notation-common.c gcc/c-family/array-notation-
 common.c
 index c010039..5d1e2c8 100644
 --- gcc/c-family/array-notation-common.c
 +++ gcc/c-family/array-notation-common.c
 @@ -657,3 +657,25 @@ fix_sec_implicit_args (location_t loc, vec tree,
 va_gc *list,
vec_safe_push (array_operand, (*list)[ii]);
return array_operand;
  }
 +
 +/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
 +   non-null tree if TP contains CILK_SPAWN_STMT.  */
 +
 +static tree
 +contains_cilk_spawn_stmt_walker (tree *tp, int *, void *) {
 +  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
 +return *tp;
 +  else
 +return NULL_TREE;
 +}
 +
 +/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
 +   node.  */
 +
 +bool
 +contains_cilk_spawn_stmt (tree expr)
 +{
 +  return walk_tree (expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
 +  != NULL_TREE;
 +}

I would move these two functions to cilk.c file instead of 
array-notations-common.c since cilk.c contains all Cilk keyword handling 
routines.

 diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index
 f074ab1..6f2c6b7 100644
 --- gcc/c-family/c-common.h
 +++ gcc/c-family/c-common.h
 @@ -1375,6 +1375,7 @@ extern void cilkplus_extract_an_triplets (vectree,
 va_gc *, size_t, size_t,
 vecvecan_parts  *);
  extern vec tree, va_gc *fix_sec_implicit_args
(location_t, vec tree, va_gc *, vecan_loop_parts, size_t, tree);
 +extern bool contains_cilk_spawn_stmt (tree);
 

...and put the prototype under /* In Cilk.c.  */ part).

  /* In cilk.c.  */
  extern tree insert_cilk_frame (tree);
 diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index da6a6fc..23cdb0e 100644
 --- gcc/c/c-typeck.c
 +++ gcc/c/c-typeck.c
 @@ -9134,7 +9134,7 @@ c_finish_return (location_t loc, tree retval, tree
 origtype)
 return error_mark_node;
   }
  }
 -  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
 +  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
  {
error_at (loc, use of %_Cilk_spawn% in a return statement is not 
   allowed);
 diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 5fc0e6b..566411f 100644
 --- gcc/cp/typeck.c
 +++ gcc/cp/typeck.c
 @@ -8328,7 +8328,7 @@ check_return_expr (tree retval, bool *no_warning)
 
*no_warning = false;
 
 -  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
 +  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
  {
error_at (EXPR_LOCATION (retval), use of %_Cilk_spawn% in a return
 
   statement is not allowed);
 diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c gcc/testsuite/c-
 c++-common/cilk-plus/CK/pr60197.c
 index e69de29..2b47d1e 100644
 --- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 +++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
 @@ -0,0 +1,66 @@
 +/* PR c/60197 */
 +/* { dg-do compile } */
 +/* { dg-options -fcilkplus } */
 +
 +extern int foo (void);
 +extern int bar (int);
 +
 +int
 +fn1 (void)
 +{
 +  return (_Cilk_spawn foo ()) * 2; /* { dg-error in a return statement
 +is not allowed } */ }
 +
 +int
 +fn2 (void)
 +{
 +  return (_Cilk_spawn foo ())  2; /* { dg-error in a return statement
 +is not allowed } */ }
 +
 +int
 +fn3 (int i, int j, int k)
 +{
 +  return ((_Cilk_spawn foo () + i) - j) * k) / j) | i) ^ k) ; /* {
 +dg-error in a return statement is not allowed } */ }
 +
 +int
 +fn4 (int i, int j, int k)
 +{
 +  return (i - _Cilk_spawn foo ()) * k) / j) | i) ^ k); /* {
 +dg-error in a return statement is not allowed

Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197)

2014-02-17 Thread Marek Polacek 
On Mon, Feb 17, 2014 at 05:51:08PM +, Iyer, Balaji V wrote:
  Regtested/bootstrapped on x86_64-linux, ok for 5.0?
 
 5.0? you mean 4.9 right?... since this is a minor bug-fix.
 
No, I meant 5.0, since this isn't a regression.  But maybe it could go
even into 4.9.  RM's call.
  
 I would move these two functions to cilk.c file instead of 
 array-notations-common.c since cilk.c contains all Cilk keyword handling 
 routines.
 
Ok, moved.

  diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index
  f074ab1..6f2c6b7 100644
  --- gcc/c-family/c-common.h
  +++ gcc/c-family/c-common.h
  @@ -1375,6 +1375,7 @@ extern void cilkplus_extract_an_triplets (vectree,
  va_gc *, size_t, size_t,
vecvecan_parts  *);
   extern vec tree, va_gc *fix_sec_implicit_args
 (location_t, vec tree, va_gc *, vecan_loop_parts, size_t, tree);
  +extern bool contains_cilk_spawn_stmt (tree);
  
 
 ...and put the prototype under /* In Cilk.c.  */ part).
 
This as well.

 Other than the two above comments, the patch looks OK to me. But I don't have 
 approval rights. I assume it doesn't break any existing regressions in the 
 cilk-plus suite (doesn't look like it though)?

Yeah, it passed regtesting.  Note that we also ICE on e.g.
int
foo (void)
{
  int i;
  i = (_Cilk_spawn foo ()) + 1;
  return i;
}

I don't know whether this is valid use of _Cilk_spawn though.  In any case, this
patch addresses only _Cilk_spawn in return statements.

2014-02-17  Marek Polacek  pola...@redhat.com

PR c/60197
c-family/
* cilk.c (contains_cilk_spawn_stmt): New function.
(contains_cilk_spawn_stmt_walker): Likewise.
* c-common.h (contains_cilk_spawn_stmt): Add declaration.
c/
* c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
of checking tree code.
cp/
* typeck.c (check_return_expr): Call contains_cilk_spawn_stmt instead
of checking tree code.
testsuite/
* c-c++-common/cilk-plus/CK/pr60197.c: New test.

diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index f074ab1..1099b10 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1389,4 +1389,5 @@ extern tree make_cilk_frame (tree);
 extern tree create_cilk_function_exit (tree, bool, bool);
 extern tree cilk_install_body_pedigree_operations (tree);
 extern void cilk_outline (tree, tree *, void *);
+extern bool contains_cilk_spawn_stmt (tree);
 #endif /* ! GCC_C_COMMON_H */
diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c
index f2179dfc..13cebf8 100644
--- gcc/c-family/cilk.c
+++ gcc/c-family/cilk.c
@@ -1292,3 +1292,25 @@ build_cilk_sync (void)
   TREE_SIDE_EFFECTS (sync) = 1;
   return sync;
 }
+
+/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
+   non-null tree if TP contains CILK_SPAWN_STMT.  */
+
+static tree
+contains_cilk_spawn_stmt_walker (tree *tp, int *, void *)
+{
+  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
+return *tp;
+  else
+return NULL_TREE;
+}
+
+/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
+   node.  */
+
+bool
+contains_cilk_spawn_stmt (tree expr)
+{
+  return walk_tree (expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
+!= NULL_TREE;
+}
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index da6a6fc..23cdb0e 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9134,7 +9134,7 @@ c_finish_return (location_t loc, tree retval, tree 
origtype)
  return error_mark_node;
}
 }
-  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
 {
   error_at (loc, use of %_Cilk_spawn% in a return statement is not 
allowed);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 5fc0e6b..566411f 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -8328,7 +8328,7 @@ check_return_expr (tree retval, bool *no_warning)
 
   *no_warning = false;
 
-  if (flag_cilkplus  retval  TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus  retval  contains_cilk_spawn_stmt (retval))
 {
   error_at (EXPR_LOCATION (retval), use of %_Cilk_spawn% in a return 
statement is not allowed);
diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c 
gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
index e69de29..2b47d1e 100644
--- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
+++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
@@ -0,0 +1,66 @@
+/* PR c/60197 */
+/* { dg-do compile } */
+/* { dg-options -fcilkplus } */
+
+extern int foo (void);
+extern int bar (int);
+
+int
+fn1 (void)
+{
+  return (_Cilk_spawn foo ()) * 2; /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn2 (void)
+{
+  return (_Cilk_spawn foo ())  2; /* { dg-error in a return statement is not 
allowed } */
+}
+
+int
+fn3 (int i, int j, int k)
+{
+  return ((_Cilk_spawn foo () + i) - j) * k) / j) | i) ^ k) ; /* { 
dg-error in a return statement is not allowed } */
+}
+