From patchwork Thu Oct 2 04:26:55 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andi Kleen X-Patchwork-Id: 395794 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4E3A7140161 for ; Thu, 2 Oct 2014 14:28:58 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references; q=dns; s= default; b=VkbzEm39osObl2TjTNm00mAf02+hleMnd6rbT2cMbCIzOt41g3DY5 /2yDvbUSPK4Tt8zWoQ7+oqVinsgpTUmGfAXS/0+F3tRKYEHGIhuA6Z+pxEjUxkdl gfkb26FX/Kkba0o+yhipaMbZNCbVUHvpmKRzmiCOhh//vEPXo5e4Ok= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references; s= default; bh=IeZerfcRB5uPCXY5gBBsea3CFFQ=; b=aE7e7ClwJtDJjAAMlyap nfCvZ8dYRJzPEagiTPvPcvT2A0m4z+OqcmlbFNPkGDGn55Omizx0cmq44kVECEyu bw6nDK6CZ4hx1RSJ/bkk10jz1+XNJC+NinaMbzhhojuPR+TqdFtadFlRKSwW+b3v QTIc/oi5gZ7NC2Mhq9IReLM= Received: (qmail 24897 invoked by alias); 2 Oct 2014 04:28:14 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 24663 invoked by uid 89); 2 Oct 2014 04:28:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: one.firstfloor.org Received: from one.firstfloor.org (HELO one.firstfloor.org) (193.170.194.197) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 02 Oct 2014 04:28:08 +0000 Received: from basil.firstfloor.org (184-100-254-193.ptld.qwest.net [184.100.254.193]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by one.firstfloor.org (Postfix) with ESMTPSA id BF4B4869AC; Thu, 2 Oct 2014 06:28:01 +0200 (CEST) Received: by basil.firstfloor.org (Postfix, from userid 1000) id DD934A1FC6; Wed, 1 Oct 2014 21:26:59 -0700 (PDT) From: Andi Kleen To: gcc-patches@gcc.gnu.org Cc: Andi Kleen Subject: [PATCH 2/5] Error out for Cilk_spawn or array expression in forbidden places Date: Wed, 1 Oct 2014 21:26:55 -0700 Message-Id: <1412224018-25368-2-git-send-email-andi@firstfloor.org> In-Reply-To: <1412224018-25368-1-git-send-email-andi@firstfloor.org> References: <1412224018-25368-1-git-send-email-andi@firstfloor.org> From: Andi Kleen _Cilk_spawn or Cilk array expressions are only allowed on their own, but not in for(), if(), switch, do, while, goto, etc. The C parser didn't always check for that, which lead to ICEs earlier for invalid code. Add a generic helper that checks this and call it where needed in the C frontend. I chose to allow spawn/array for for init and increment expressions. While the Cilk spec could be interpreted to forbid it there too there didn't seem any reason to not allow it. One dark corner is spawn, array in statement expressions not at the end. Right now that's forbidden too. gcc/c-family/: 2014-09-30 Andi Kleen PR c/60804 * c-common.h (check_no_cilk): Declare. * cilk.c (get_error_location): New function. (check_no_cilk): Dito. gcc/c/: 2014-09-30 Andi Kleen PR c/60804 * c-parser.c (c_parser_statement_after_labels): Call check_no_cilk. (c_parser_if_statement): Dito. (c_parser_switch_statement): Dito. (c_parser_while_statement): Dito. (c_parser_do_statement): Dito. (c_parser_for_statement): Dito. * c-typeck.c (c_finish_loop): Dito. --- gcc/c-family/c-common.h | 1 + gcc/c-family/cilk.c | 37 +++++++++++++++++++++++++++++++++ gcc/c/c-parser.c | 54 ++++++++++++++++++++++++------------------------- gcc/c/c-typeck.c | 8 ++------ 4 files changed, 66 insertions(+), 34 deletions(-) diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 1e3477f..37ee156 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1395,4 +1395,5 @@ extern tree cilk_install_body_pedigree_operations (tree); extern void cilk_outline (tree, tree *, void *); extern bool contains_cilk_spawn_stmt (tree); extern tree cilk_for_number_of_iterations (tree); +extern bool check_no_cilk (tree, const char *, location_t loc = UNKNOWN_LOCATION); #endif /* ! GCC_C_COMMON_H */ diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c index 5b6684a..e33ef2b 100644 --- a/gcc/c-family/cilk.c +++ b/gcc/c-family/cilk.c @@ -1312,3 +1312,40 @@ contains_cilk_spawn_stmt (tree expr) return walk_tree (&expr, contains_cilk_spawn_stmt_walker, NULL, NULL) != NULL_TREE; } + +/* Return a error location for EXPR if LOC is not set. */ + +static location_t +get_error_location (tree expr, location_t loc) +{ + if (loc == UNKNOWN_LOCATION) + { + if (TREE_CODE (expr) == MODIFY_EXPR) + expr = TREE_OPERAND (expr, 0); + loc = EXPR_LOCATION (expr); + } + return loc; +} + +/* Check that no array notation or spawn statement is in EXPR. + If not true gemerate an error at LOC for WHAT. */ + +bool +check_no_cilk (tree expr, const char *what, location_t loc) +{ + if (!flag_cilkplus) + return false; + if (contains_array_notation_expr (expr)) + { + loc = get_error_location (expr, loc); + error_at (loc, "Cilk array notation cannot be used %s", what); + return true; + } + if (walk_tree (&expr, contains_cilk_spawn_stmt_walker, NULL, NULL)) + { + loc = get_error_location (expr, loc); + error_at (loc, "%<_Cilk_spawn%> statement cannot be used %s", what); + return true; + } + return false; +} diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 0d159fd..aa5a6cd 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -4926,6 +4926,8 @@ c_parser_statement_after_labels (c_parser *parser) c_parser_consume_token (parser); val = c_parser_expression (parser); + if (check_no_cilk (val.value, "as a computed goto expression", loc)) + val.value = error_mark_node; val = convert_lvalue_to_rvalue (loc, val, false, true); stmt = c_finish_goto_ptr (loc, val.value); } @@ -4979,8 +4981,13 @@ c_parser_statement_after_labels (c_parser *parser) { struct c_expr expr = c_parser_expression (parser); expr = convert_lvalue_to_rvalue (loc, expr, false, false); - expr.value = c_fully_fold (expr.value, false, NULL); - stmt = objc_build_throw_stmt (loc, expr.value); + if (check_no_cilk (expr.value, "for a throw expression")) + expr.value = error_mark_node; + else + { + expr.value = c_fully_fold (expr.value, false, NULL); + stmt = objc_build_throw_stmt (loc, expr.value); + } goto expect_semicolon; } break; @@ -5163,6 +5170,11 @@ c_parser_if_statement (c_parser *parser) block = c_begin_compound_stmt (flag_isoc99); loc = c_parser_peek_token (parser)->location; cond = c_parser_paren_condition (parser); + if (flag_cilkplus && contains_cilk_spawn_stmt (cond)) + { + error_at (loc, "if statement cannot contain %"); + cond = error_mark_node; + } in_if_block = parser->in_if_block; parser->in_if_block = true; first_body = c_parser_if_body (parser, &first_if); @@ -5209,13 +5221,10 @@ c_parser_switch_statement (c_parser *parser) ce = c_parser_expression (parser); ce = convert_lvalue_to_rvalue (switch_cond_loc, ce, true, false); expr = ce.value; - if (flag_cilkplus && contains_array_notation_expr (expr)) - { - error_at (switch_cond_loc, - "array notations cannot be used as a condition for switch " - "statement"); - expr = error_mark_node; - } + /* ??? expr has no valid location? */ + if (check_no_cilk (expr, "as a condition for switch statement", + switch_cond_loc)) + expr = error_mark_node; c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); } else @@ -5255,13 +5264,8 @@ c_parser_while_statement (c_parser *parser, bool ivdep) block = c_begin_compound_stmt (flag_isoc99); loc = c_parser_peek_token (parser)->location; cond = c_parser_paren_condition (parser); - if (flag_cilkplus && contains_array_notation_expr (cond)) - { - error_at (loc, "array notations cannot be used as a condition for while " - "statement"); - cond = error_mark_node; - } - + if (check_no_cilk (cond, "as a condition for while statement")) + cond = error_mark_node; if (ivdep && cond != error_mark_node) cond = build2 (ANNOTATE_EXPR, TREE_TYPE (cond), cond, build_int_cst (integer_type_node, @@ -5307,12 +5311,8 @@ c_parser_do_statement (c_parser *parser, bool ivdep) new_cont = c_cont_label; c_cont_label = save_cont; cond = c_parser_paren_condition (parser); - if (flag_cilkplus && contains_array_notation_expr (cond)) - { - error_at (loc, "array notations cannot be used as a condition for a " - "do-while statement"); - cond = error_mark_node; - } + if (check_no_cilk (cond, "as a condition for a do-while statement")) + cond = error_mark_node; if (ivdep && cond != error_mark_node) cond = build2 (ANNOTATE_EXPR, TREE_TYPE (cond), cond, build_int_cst (integer_type_node, @@ -5464,6 +5464,8 @@ c_parser_for_statement (c_parser *parser, bool ivdep) struct c_expr ce; tree init_expression; ce = c_parser_expression (parser); + /* In theory we could forbid _Cilk_spawn here, as the spec says "only in top + level statement", but it works just fine, so allow it. */ init_expression = ce.value; parser->objc_could_be_foreach_context = false; if (c_parser_next_token_is_keyword (parser, RID_IN)) @@ -5505,12 +5507,8 @@ c_parser_for_statement (c_parser *parser, bool ivdep) else { cond = c_parser_condition (parser); - if (flag_cilkplus && contains_array_notation_expr (cond)) - { - error_at (loc, "array notations cannot be used in a " - "condition for a for-loop"); - cond = error_mark_node; - } + if (check_no_cilk (cond, "in a condition for a for-loop")) + cond = error_mark_node; c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>"); } diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index f69c28b..9df1b94 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -9600,12 +9600,8 @@ c_finish_loop (location_t start_locus, tree cond, tree incr, tree body, { tree entry = NULL, exit = NULL, t; - if (flag_cilkplus && contains_array_notation_expr (cond)) - { - error_at (start_locus, "array notation expression cannot be used in a " - "loop%'s condition"); - return; - } + /* In theory could forbid cilk spawn for loop increment expression, + but it should work just fine. */ /* If the condition is zero don't generate a loop construct. */ if (cond && integer_zerop (cond))