From patchwork Tue Sep 3 14:08:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tristan Gingold X-Patchwork-Id: 272270 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "www.sourceware.org", Issuer "StartCom Class 1 Primary Intermediate Server CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id DDB8C2C00A3 for ; Wed, 4 Sep 2013 00:09:02 +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 :content-type:content-transfer-encoding:subject:date:message-id :cc:to:mime-version; q=dns; s=default; b=OxqgrnL9czPvGN1cPnubuUX 4/rPe5iD8ACEfpkczWIuX+Q3k+ZO6SDJ3qKVM/X52QADF7yEP8va/IMB5XAcswZD a8ZEJCpOLdPIx0d/I9T8e9J38/+ZaSwmBKg1CqlEq3F8MSna3+9YzzgcWteChlqO wVxGWxh/wvMkGzRJG2gY= 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 :content-type:content-transfer-encoding:subject:date:message-id :cc:to:mime-version; s=default; bh=wIdwrkFOH5VcsLwHDqGABWAO2mc=; b= hmXl1SnId+34IahSbfMkpNjct20EZOZmsMq14IcktOfsk8eZiIk74NsxdlD5DRUU NkHVF3nFUoyA2mfGJj1LAI4YjA0X7gnFiXxNsWOHe7kkYIHV0Y3bgyA5vxUZWoJk b9YYW83LvlJM4ZaKQC/FXBVBPEBzDEJy0e4z+gL2Hvs= Received: (qmail 3558 invoked by alias); 3 Sep 2013 14:08:53 -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 3549 invoked by uid 89); 3 Sep 2013 14:08:52 -0000 Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 03 Sep 2013 14:08:52 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=ALL_TRUSTED, AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: smtp.eu.adacore.com Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 6CAEE2660AEC; Tue, 3 Sep 2013 16:08:46 +0200 (CEST) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jtLs8hahWOZM; Tue, 3 Sep 2013 16:08:46 +0200 (CEST) Received: from ulanbator.act-europe.fr (ulanbator.act-europe.fr [10.10.1.67]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 1B2AD2660AD1; Tue, 3 Sep 2013 16:08:45 +0200 (CEST) From: Tristan Gingold Subject: [PATCH]: Fix use of __builtin_eh_pointer in EH_ELSE Date: Tue, 3 Sep 2013 16:08:48 +0200 Message-Id: Cc: Richard Henderson To: GCC Patches Mime-Version: 1.0 (Mac OS X Mail 6.5 \(1508\)) X-IsSubscribed: yes Hi, The field state->ehp_region wasn't updated before lowering constructs in the eh path of EH_ELSE. As a consequence, __builtin_eh_pointer is lowered to 0 (or possibly to a wrong region number) in this path. The only user of EH_ELSE looks to be trans-mem.c:lower_transaction, and the consequence of that is a memory leak. Furthermore, according to calls.c:flags_from_decl_or_type, the "transaction_pure" attribute must be set on the function type, not on the function declaration. Hence the change to declare __builtin_eh_pointer. (I don't understand the guard condition to set the attribute for it in tree.c. Why is 'builtin_decl_explicit_p (BUILT_IN_TM_LOAD_1)' needed in addition to flag_tm ?) No regressions (check-host) on x86-64 GNU/Linux. Ok for trunk ? Tristan. 2013-09-03 Tristan Gingold * tree.c (set_call_expr_flags): Reject ECF_TM_PURE. (build_common_builtin_nodes): Set "transaction_pure" attribute on __builtin_eh_pointer function type (and not on its declaration). * tree-eh.c (lower_try_finally_nofallthru): Set and revert ehp_region before callinf lower_eh_constructs_1. (lower_try_finally_onedest): Likewise. (lower_try_finally_copy): Likewise. (lower_try_finally_switch): Likewise. testsuite/ 2013-09-03 Tristan Gingold * gcc.dg/tm/except.c: New testcase. diff --git a/gcc/testsuite/gcc.dg/tm/except.c b/gcc/testsuite/gcc.dg/tm/except.c new file mode 100644 index 0000000..ed84bb3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tm/except.c @@ -0,0 +1,36 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -O1 -fexceptions -fdump-tree-optimized" } */ + +typedef struct node { + int val; + struct node *next; +} node_t; + +node_t *head; + +__attribute__((transaction_safe)) +node_t *new_node(int val, node_t *next); + +int add(int val) +{ + int result; + node_t *prev, *next; + + __transaction_atomic { + prev = head; + next = prev->next; + while (next->val < val) { + prev = next; + next = prev->next; + } + result = (next->val != val); + if (result) { + prev->next = new_node(val, next); + } + } + return result; +} + +/* { dg-final { scan-tree-dump-not "ITM_commitTransactionEH \\(0B\\)" "optimized" } } */ + +/* { dg-final { cleanup-tree-dump "optimized" } } */ diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index 6ffbd26..86ee77e 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -1093,8 +1093,12 @@ lower_try_finally_nofallthru (struct leh_state *state, if (tf->may_throw) { + eh_region prev_ehp_region = state->ehp_region; + finally = gimple_eh_else_e_body (eh_else); + state->ehp_region = tf->region; lower_eh_constructs_1 (state, &finally); + state->ehp_region = prev_ehp_region; emit_post_landing_pad (&eh_seq, tf->region); gimple_seq_add_seq (&eh_seq, finally); @@ -1129,6 +1133,7 @@ lower_try_finally_onedest (struct leh_state *state, struct leh_tf_state *tf) gimple_stmt_iterator gsi; tree finally_label; location_t loc = gimple_location (tf->try_finally_expr); + eh_region prev_ehp_region = state->ehp_region; finally = gimple_try_cleanup (tf->top_p); tf->top_p_seq = gimple_try_eval (tf->top_p); @@ -1140,12 +1145,16 @@ lower_try_finally_onedest (struct leh_state *state, struct leh_tf_state *tf) if (x) { if (tf->may_throw) - finally = gimple_eh_else_e_body (x); + { + state->ehp_region = tf->region; + finally = gimple_eh_else_e_body (x); + } else finally = gimple_eh_else_n_body (x); } lower_eh_constructs_1 (state, &finally); + state->ehp_region = prev_ehp_region; for (gsi = gsi_start (finally); !gsi_end_p (gsi); gsi_next (&gsi)) { @@ -1255,13 +1264,19 @@ lower_try_finally_copy (struct leh_state *state, struct leh_tf_state *tf) if (tf->may_throw) { + eh_region prev_ehp_region = state->ehp_region; + /* We don't need to copy the EH path of EH_ELSE, since it is only emitted once. */ if (eh_else) - seq = gimple_eh_else_e_body (eh_else); + { + seq = gimple_eh_else_e_body (eh_else); + state->ehp_region = tf->region; + } else seq = lower_try_finally_dup_block (finally, state, tf_loc); lower_eh_constructs_1 (state, &seq); + state->ehp_region = prev_ehp_region; emit_post_landing_pad (&eh_seq, tf->region); gimple_seq_add_seq (&eh_seq, seq); @@ -1432,8 +1447,12 @@ lower_try_finally_switch (struct leh_state *state, struct leh_tf_state *tf) { if (tf->may_throw) { + eh_region prev_ehp_region = state->ehp_region; + + state->ehp_region = tf->region; finally = gimple_eh_else_e_body (eh_else); lower_eh_constructs_1 (state, &finally); + state->ehp_region = prev_ehp_region; emit_post_landing_pad (&eh_seq, tf->region); gimple_seq_add_seq (&eh_seq, finally); diff --git a/gcc/tree.c b/gcc/tree.c index f0ee309..e4be24d 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -9817,9 +9817,11 @@ set_call_expr_flags (tree decl, int flags) if (flags & ECF_LEAF) DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("leaf"), NULL, DECL_ATTRIBUTES (decl)); - if ((flags & ECF_TM_PURE) && flag_tm) - DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("transaction_pure"), - NULL, DECL_ATTRIBUTES (decl)); + + /* The "transaction_pure" attribute must be set on the function type, not + on the declaration. */ + gcc_assert (!(flags & ECF_TM_PURE)); + /* Looping const or pure is implied by noreturn. There is currently no way to declare looping const or looping pure alone. */ gcc_assert (!(flags & ECF_LOOPING_CONST_OR_PURE) @@ -10018,8 +10020,9 @@ build_common_builtin_nodes (void) integer_type_node, NULL_TREE); ecf_flags = ECF_PURE | ECF_NOTHROW | ECF_LEAF; /* Only use TM_PURE if we we have TM language support. */ - if (builtin_decl_explicit_p (BUILT_IN_TM_LOAD_1)) - ecf_flags |= ECF_TM_PURE; + if (flag_tm && builtin_decl_explicit_p (BUILT_IN_TM_LOAD_1)) + TYPE_ATTRIBUTES (ftype) = tree_cons (get_identifier ("transaction_pure"), + NULL, TYPE_ATTRIBUTES (ftype)); local_define_builtin ("__builtin_eh_pointer", ftype, BUILT_IN_EH_POINTER, "__builtin_eh_pointer", ecf_flags);