diff mbox

Go patch committed: Fix reloc info for circular immutable structs

Message ID mcry50qn01u.fsf@iant-glaptop.roam.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor March 4, 2014, 2:57 a.m. UTC 
In Go, unlike C, there can be mutually recursive static initializers.
For type descriptors, those initializers can be common, in the sense
that they can appear in multiple object files.  This is done using
make_decl_one_only.  That function only computes relocation information
correctly when the initializer is set, so we only call it at that point.
(When the relocation information is incorrect, on some systems the
variable will be put in a writable .rodata section, which makes little
sense.)  If there is a mutually recursive reference, then one of the
initializers need to be computed before the other is set.  To work
around this issue, we temporarily mark the variable as weak after it has
been created but before the initializer is set, which should ensure that
the right relocation is computed.  This is definitely a hack, but it
should suffice for GCC 4.9.

Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian


2014-03-03  Ian Lance Taylor  <iant@google.com>

	* go-gcc.cc (Gcc_backend::immutable_struct): If IS_COMMON, set
	DECL_WEAK.
	(GCC_backend::immutable_struct_set_init): If IS_COMMON, clear
	DECL_WEAK.
diff mbox

Patch

Index: go-gcc.cc
===================================================================
--- go-gcc.cc	(revision 208300)
+++ go-gcc.cc	(working copy)
@@ -1871,7 +1871,7 @@ 
 
 Bvariable*
 Gcc_backend::immutable_struct(const std::string& name, bool is_hidden,
-			      bool, Btype* btype, Location location)
+			      bool is_common, Btype* btype, Location location)
 {
   tree type_tree = btype->get_tree();
   if (type_tree == error_mark_node)
@@ -1888,6 +1888,21 @@ 
   if (!is_hidden)
     TREE_PUBLIC(decl) = 1;
 
+  // When the initializer for one immutable_struct refers to another,
+  // it needs to know the visibility of the referenced struct so that
+  // compute_reloc_for_constant will return the right value.  On many
+  // systems calling make_decl_one_only will mark the decl as weak,
+  // which will change the return value of compute_reloc_for_constant.
+  // We can't reliably call make_decl_one_only yet, because we don't
+  // yet know the initializer.  This issue doesn't arise in C because
+  // Go initializers, unlike C initializers, can be indirectly
+  // recursive.  To ensure that compute_reloc_for_constant computes
+  // the right value if some other initializer refers to this one, we
+  // mark this symbol as weak here.  We undo that below in
+  // immutable_struct_set_init before calling mark_decl_one_only.
+  if (is_common)
+    DECL_WEAK(decl) = 1;
+
   // We don't call rest_of_decl_compilation until we have the
   // initializer.
 
@@ -1910,9 +1925,13 @@ 
 
   DECL_INITIAL(decl) = init_tree;
 
-  // We can't call make_decl_one_only until we set DECL_INITIAL.
+  // Now that DECL_INITIAL is set, we can't call make_decl_one_only.
+  // See the comment where DECL_WEAK is set in immutable_struct.
   if (is_common)
-    make_decl_one_only(decl, DECL_ASSEMBLER_NAME(decl));
+    {
+      DECL_WEAK(decl) = 0;
+      make_decl_one_only(decl, DECL_ASSEMBLER_NAME(decl));
+    }
 
   // These variables are often unneeded in the final program, so put
   // them in their own section so that linker GC can discard them.