This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH]: Fix use of __builtin_eh_pointer in EH_ELSE


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);
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]