This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH]: Fix use of __builtin_eh_pointer in EH_ELSE
- From: Tristan Gingold <gingold at adacore dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Richard Henderson <rth at redhat dot com>
- Date: Tue, 3 Sep 2013 16:08:48 +0200
- Subject: [PATCH]: Fix use of __builtin_eh_pointer in EH_ELSE
- Authentication-results: sourceware.org; auth=none
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 <gingold@adacore.com>
* 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 <gingold@adacore.com>
* 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);