From patchwork Thu Feb  3 16:17:04 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [trans-mem] PR47573: fix DECL_STRUCT_FUNCTION sharing
Date: Thu, 03 Feb 2011 06:17:04 -0000
From: Aldy Hernandez <aldyh@redhat.com>
X-Patchwork-Id: 81677
Message-Id: <4D4AD500.2020906@redhat.com>
To: Richard Henderson <rth@redhat.com>,
 Patrick MARLIER <patrick.marlier@unine.ch>,
 gcc-patches <gcc-patches@gcc.gnu.org>

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?
PR/47573
	* trans-mem.c (ipa_tm_create_version): Call
	tree_function_versioning when the old node will be available.


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