Message ID | 20140219133850.GB10901@redhat.com |
---|---|
State | New |
Headers | show |
Ping. On Wed, Feb 19, 2014 at 02:38:50PM +0100, Marek Polacek wrote: > On Tue, Feb 18, 2014 at 10:06:14PM +0000, 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 <polacek@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 ((((((_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 "in a return statement is not allowed" } */ > +} > > Marek Marek
> -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@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 +0000, 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 <polacek@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 ((((((_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 "in a return statement > +is not allowed" } */ } > Hi Marek, This patch looks OK to me. I don't have approval rights, so I can't approve it. Thanks, Balaji V. Iyer. > Marek
On 02/26/14 08:21, Marek Polacek wrote:
> Ping.
Based on Balaji's comments, OK.
jeff
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 ((((((_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 "in a return statement is not allowed" } */ +}
On Tue, Feb 18, 2014 at 10:06:14PM +0000, 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 <polacek@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. Marek