[trans-mem] PR47573: fix DECL_STRUCT_FUNCTION sharing

Submitted by Aldy Hernandez on Feb. 3, 2011, 4:17 p.m.


Message ID 4D4AD500.2020906@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Feb. 3, 2011, 4:17 p.m.
In the attached testcase, we are incorrectly sharing the 
DECL_STRUCT_FUNCTION field between a TM clone and the original function. 
  The TM clone gets removed from the callgraph, but when we are 
verifying the saneness of the original function, the 
DECL_STRUCT_FUNCTION fields have been swiped from under us, and we end 
up accessing poisoned fields.

This usually doesn't happen because when we create the TM clones we call 
tree_function_versioning() which makes copies of the struct function 
fields.  But this code is predicated by DECL_EXTERNAL which is set for 
the template/base constructor in the testcase.

The documentation for DECL_EXTERNAL specifically states that this flag 
can be set, but we can have a source-level definition for the node in 
the translation unit.  So we could be DECL_EXTERNAL, but yet have code 
associated with the node.

Instead, wouldn't it be better to check the node's availability to make 
sure we are really talking about something outside of this translation 
unit?  Below is a patch doing this, which fixes the ICE.

OK for branch?
	* trans-mem.c (ipa_tm_create_version): Call
	tree_function_versioning when the old node will be available.


Richard Henderson Feb. 3, 2011, 4:30 p.m.
On 02/03/2011 08:17 AM, Aldy Hernandez wrote:
> 	PR/47573
> 	* trans-mem.c (ipa_tm_create_version): Call
> 	tree_function_versioning when the old node will be available.



Patch hide | download patch | download mbox

Index: testsuite/g++.dg/tm/pr47573.C
--- testsuite/g++.dg/tm/pr47573.C	(revision 0)
+++ testsuite/g++.dg/tm/pr47573.C	(revision 0)
@@ -0,0 +1,25 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm" }
+template<typename _Tp> class allocator
+        public:
+        allocator() { }
+extern template class allocator<char>;
+template<typename _Alloc = allocator<char> > class basic_string
+        public:
+        _Alloc _M_dataplus;
+        __attribute__((transaction_safe))    
+        basic_string() : _M_dataplus(_Alloc())
+        {
+        }
+int getStringHeight()
+        basic_string<> tmp;
Index: trans-mem.c
--- trans-mem.c	(revision 169788)
+++ trans-mem.c	(working copy)
@@ -4089,7 +4089,7 @@  ipa_tm_create_version (struct cgraph_nod
   new_node = cgraph_copy_node_for_versioning (old_node, new_decl, NULL);
   get_cg_data (old_node)->clone = new_node;
-  if (!DECL_EXTERNAL (old_decl))
+  if (cgraph_function_body_availability (old_node) >= AVAIL_OVERWRITABLE)
     tree_function_versioning (old_decl, new_decl, NULL, false, NULL);
   /* ?? We should be able to remove DECL_IS_TM_CLONE.  We have enough