Re: [PATCH]: Fix use of __builtin_eh_pointer in EH_ELSE

[Tristan Gingold <gingold at adacore dot com>]

On Sep 24, 2013, at 8:51 PM, Richard Henderson <rth@redhat.com> wrote:

> On 09/03/2013 07:08 AM, Tristan Gingold wrote:
>> 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 ?)
> 
> Clearly these are totally unrelated and should not be in the same patch.

This wasn't clear to me, as I got 'unsafe function call __builtin_eh_pointer in
atomic transaction' before fixing the transaction_pure.

So here is the 'transaction_pure' part.
No check-host regressions on x86_64-linux-gnu.

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

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