Patchwork Go patch committed: Copy initializer to heap if it may have pointers

login
register
mail settings
Submitter Ian Taylor
Date Jan. 26, 2011, 7:48 p.m.
Message ID <mcrtygvv42m.fsf@google.com>
Download mbox | patch
Permalink /patch/80546/
State New
Headers show

Comments

Ian Taylor - Jan. 26, 2011, 7:48 p.m.
If an initializer for a slice may have pointers, then we need to copy it
to the heap even if it is constant, as otherwise the garbage collector
won't see it.  If the garbage collector doesn't see it, then if it is
changed, the new elements may be discarded by the collector.  This patch
implements that.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

Patch

diff -r 5b51b216b4b0 go/expressions.cc
--- a/go/expressions.cc	Mon Jan 24 21:44:03 2011 -0800
+++ b/go/expressions.cc	Wed Jan 26 11:34:22 2011 -0800
@@ -11138,7 +11138,15 @@ 
     return error_mark_node;
 
   bool is_constant_initializer = TREE_CONSTANT(values);
-  bool is_in_function = context->function() != NULL;
+
+  // We have to copy the initial values into heap memory if we are in
+  // a function or if the values are not constants.  We also have to
+  // copy them if they may contain pointers in a non-constant context,
+  // as otherwise the garbage collector won't see them.
+  bool copy_to_heap = (context->function() != NULL
+		       || !is_constant_initializer
+		       || (element_type->has_pointer()
+			   && !context->is_const()));
 
   if (is_constant_initializer)
     {
@@ -11148,12 +11156,12 @@ 
       TREE_PUBLIC(tmp) = 0;
       TREE_STATIC(tmp) = 1;
       DECL_ARTIFICIAL(tmp) = 1;
-      if (is_in_function)
-	{
-	  // If this is not a function, we will only initialize the
-	  // value once, so we can use this directly rather than
-	  // copying it.  In that case we can't make it read-only,
-	  // because the program is permitted to change it.
+      if (copy_to_heap)
+	{
+	  // If we are not copying the value to the heap, we will only
+	  // initialize the value once, so we can use this directly
+	  // rather than copying it.  In that case we can't make it
+	  // read-only, because the program is permitted to change it.
 	  TREE_READONLY(tmp) = 1;
 	  TREE_CONSTANT(tmp) = 1;
 	}
@@ -11164,10 +11172,9 @@ 
 
   tree space;
   tree set;
-  if (!is_in_function && is_constant_initializer)
-    {
-      // Outside of a function, we know the initializer will only run
-      // once.
+  if (!copy_to_heap)
+    {
+      // the initializer will only run once.
       space = build_fold_addr_expr(values);
       set = NULL_TREE;
     }
@@ -11214,7 +11221,7 @@ 
   tree constructor = build_constructor(type_tree, init);
   if (constructor == error_mark_node)
     return error_mark_node;
-  if (!is_in_function && is_constant_initializer)
+  if (!copy_to_heap)
     TREE_CONSTANT(constructor) = 1;
 
   if (set == NULL_TREE)