Patchwork [gccgo] Register all global variables explicitly

login
register
mail settings
Submitter Ian Taylor
Date Aug. 1, 2010, 10:28 a.m.
Message ID <mcriq3ulj8n.fsf@google.com>
Download mbox | patch
Permalink /patch/60446/
State New
Headers show

Comments

Ian Taylor - Aug. 1, 2010, 10:28 a.m.
This patch changes libgo to use an explicit list of global variables
defined in Go which may contain pointers, rather than scanning the
entire data section of the executable.  Each package registers its own
list of variables.  This has the advantage of only scanning variables
for imported packages in libgo, rather than all packages.  This also
removes the changes to crtstuff.c I added to support finding the data
section at runtime.  Committed to gccgo branch.

Ian

Patch

diff -r db5d6acb8d42 go/gogo-tree.cc
--- a/go/gogo-tree.cc	Sun Aug 01 03:18:13 2010 -0700
+++ b/go/gogo-tree.cc	Sun Aug 01 03:20:48 2010 -0700
@@ -215,6 +215,131 @@ 
     }
 }
 
+// Register global variables with the garbage collector.  We need to
+// register all variables which can hold a pointer value.  They become
+// roots during the mark phase.  We build a struct that is easy to
+// hook into a list of roots.
+
+// struct __go_gc_root_list
+// {
+//   struct __go_gc_root_list* __next;
+//   struct __go_gc_root
+//   {
+//     void* __decl;
+//     size_t __size;
+//   } __roots[];
+// };
+
+// The last entry in the roots array has a NULL decl field.
+
+void
+Gogo::register_gc_vars(const std::vector<Named_object*>& var_gc,
+		       tree* init_stmt_list)
+{
+  if (var_gc.empty())
+    return;
+
+  size_t count = var_gc.size();
+
+  tree root_type = Gogo::builtin_struct(NULL, "__go_gc_root", NULL_TREE, 2,
+					"__next",
+					ptr_type_node,
+					"__size",
+					sizetype);
+
+  tree index_type = build_index_type(size_int(count));
+  tree array_type = build_array_type(root_type, index_type);
+
+  tree root_list_type = make_node(RECORD_TYPE);
+  root_list_type = Gogo::builtin_struct(NULL, "__go_gc_root_list",
+					root_list_type, 2,
+					"__next",
+					build_pointer_type(root_list_type),
+					"__roots",
+					array_type);
+
+  // Build an initialier for the __roots array.
+
+  VEC(constructor_elt,gc)* roots_init = VEC_alloc(constructor_elt, gc,
+						  count + 1);
+
+  size_t i = 0;
+  for (std::vector<Named_object*>::const_iterator p = var_gc.begin();
+       p != var_gc.end();
+       ++p, ++i)
+    {
+      VEC(constructor_elt,gc)* init = VEC_alloc(constructor_elt, gc, 2);
+
+      constructor_elt* elt = VEC_quick_push(constructor_elt, init, NULL);
+      tree field = TYPE_FIELDS(root_type);
+      elt->index = field;
+      tree decl = (*p)->get_tree(this, NULL);
+      gcc_assert(TREE_CODE(decl) == VAR_DECL);
+      elt->value = build_fold_addr_expr(decl);
+
+      elt = VEC_quick_push(constructor_elt, init, NULL);
+      field = TREE_CHAIN(field);
+      elt->index = field;
+      elt->value = DECL_SIZE_UNIT(decl);
+
+      elt = VEC_quick_push(constructor_elt, roots_init, NULL);
+      elt->index = size_int(i);
+      elt->value = build_constructor(root_type, init);
+    }
+
+  // The list ends with a NULL entry.
+
+  VEC(constructor_elt,gc)* init = VEC_alloc(constructor_elt, gc, 2);
+
+  constructor_elt* elt = VEC_quick_push(constructor_elt, init, NULL);
+  tree field = TYPE_FIELDS(root_type);
+  elt->index = field;
+  elt->value = fold_convert(TREE_TYPE(field), null_pointer_node);
+
+  elt = VEC_quick_push(constructor_elt, init, NULL);
+  field = TREE_CHAIN(field);
+  elt->index = field;
+  elt->value = size_zero_node;
+
+  elt = VEC_quick_push(constructor_elt, roots_init, NULL);
+  elt->index = size_int(i);
+  elt->value = build_constructor(root_type, init);
+
+  // Build a constructor for the struct.
+
+  VEC(constructor_elt,gc*) root_list_init = VEC_alloc(constructor_elt, gc, 2);
+
+  elt = VEC_quick_push(constructor_elt, root_list_init, NULL);
+  field = TYPE_FIELDS(root_list_type);
+  elt->index = field;
+  elt->value = fold_convert(TREE_TYPE(field), null_pointer_node);
+
+  elt = VEC_quick_push(constructor_elt, root_list_init, NULL);
+  field = TREE_CHAIN(field);
+  elt->index = field;
+  elt->value = build_constructor(array_type, roots_init);
+
+  // Build a decl to register.
+
+  tree decl = build_decl(BUILTINS_LOCATION, VAR_DECL,
+			 create_tmp_var_name("gc"), root_list_type);
+  DECL_EXTERNAL(decl) = 0;
+  TREE_PUBLIC(decl) = 0;
+  TREE_STATIC(decl) = 1;
+  DECL_ARTIFICIAL(decl) = 1;
+  DECL_INITIAL(decl) = build_constructor(root_list_type, root_list_init);
+  rest_of_decl_compilation(decl, 1, 0);
+
+  static tree register_gc_fndecl;
+  tree call = Gogo::call_builtin(&register_gc_fndecl, BUILTINS_LOCATION,
+				 "__go_register_gc_roots",
+				 1,
+				 void_type_node,
+				 build_pointer_type(root_list_type),
+				 build_fold_addr_expr(decl));
+  append_to_statement_list(call, init_stmt_list);
+}
+
 // Build the decl for the initialization function.
 
 tree
@@ -526,6 +651,12 @@ 
   // A list of variable initializations.
   Var_inits var_inits;
 
+  // A list of variables which need to be registered with the garbage
+  // collector.
+  std::vector<Named_object*> var_gc;
+  var_gc.reserve(count);
+
+  tree var_init_stmt_list = NULL_TREE;
   size_t i = 0;
   for (Bindings::const_definitions_iterator p = bindings->begin_definitions();
        p != bindings->end_definitions();
@@ -626,15 +757,25 @@ 
 	    {
 	      if (no->var_value()->init() == NULL
 		  && !no->var_value()->has_pre_init())
-		append_to_statement_list(var_init_tree, &init_stmt_list);
+		append_to_statement_list(var_init_tree, &var_init_stmt_list);
 	      else
 		var_inits.push_back(Var_init(no, var_init_tree));
 	    }
+
+	  if (!is_sink && no->var_value()->type()->has_pointer())
+	    var_gc.push_back(no);
 	}
     }
 
-  // Initialize the variables, first sorting them into a workable
-  // order.
+  // Register global variables with the garbage collector.
+  this->register_gc_vars(var_gc, &init_stmt_list);
+
+  // Simple variable initializations, after all variables are
+  // registered.
+  append_to_statement_list(var_init_stmt_list, &init_stmt_list);
+
+  // Complex variable initializations, first sorting them into a
+  // workable order.
   if (!var_inits.empty())
     {
       sort_var_inits(&var_inits);
diff -r db5d6acb8d42 go/gogo.cc
--- a/go/gogo.cc	Sun Aug 01 03:18:13 2010 -0700
+++ b/go/gogo.cc	Sun Aug 01 03:20:48 2010 -0700
@@ -1326,12 +1326,15 @@ 
       else if ((*p)->is_const())
 	(*p)->const_value()->determine_type();
 
-      // If this is a global variable which requires runtime
-      // initialization, we need an initialization function.  We know
-      // that we will see all global variables here.
-      if ((*p)->is_variable())
+      // See if a variable requires us to build an initialization
+      // function.  We know that we will see all global variables
+      // here.
+      if (!this->need_init_fn_ && (*p)->is_variable())
 	{
 	  Variable* variable = (*p)->var_value();
+
+	  // If this is a global variable which requires runtime
+	  // initialization, we need an initialization function.
 	  if (!variable->is_global() || variable->init() == NULL)
 	    ;
 	  else if (variable->type()->interface_type() != NULL)
@@ -1342,6 +1345,12 @@ 
 	    this->need_init_fn_ = true;
 	  else if (variable->init()->is_nonconstant_composite_literal())
 	    this->need_init_fn_ = true;
+
+	  // If this is a global variable which holds a pointer value,
+	  // then we need an initialization function to register it as a
+	  // GC root.
+	  if (variable->is_global() && variable->type()->has_pointer())
+	    this->need_init_fn_ = true;
 	}
     }
 
diff -r db5d6acb8d42 go/gogo.h
--- a/go/gogo.h	Sun Aug 01 03:18:13 2010 -0700
+++ b/go/gogo.h	Sun Aug 01 03:20:48 2010 -0700
@@ -605,6 +605,10 @@ 
   void
   init_imports(tree*);
 
+  // Register variables with the garbage collector.
+  void
+  register_gc_vars(const std::vector<Named_object*>&, tree*);
+
   // Build a pointer to a Go string constant.  This returns a pointer
   // to the pointer.
   tree
diff -r db5d6acb8d42 libgo/runtime/malloc.h
--- a/libgo/runtime/malloc.h	Sun Aug 01 03:18:13 2010 -0700
+++ b/libgo/runtime/malloc.h	Sun Aug 01 03:20:48 2010 -0700
@@ -375,6 +375,7 @@ 
 
 void	MProf_Malloc(void*, uintptr);
 void	MProf_Free(void*, uintptr);
+void	MProf_Mark(void (*scan)(int32, byte *, int64));
 
 // Malloc profiling settings.
 // Must match definition in extern.go.
diff -r db5d6acb8d42 libgo/runtime/mfinal.c
--- a/libgo/runtime/mfinal.c	Sun Aug 01 03:18:13 2010 -0700
+++ b/libgo/runtime/mfinal.c	Sun Aug 01 03:20:48 2010 -0700
@@ -167,11 +167,12 @@ 
 }
 
 void
-walkfintab(void (*fn)(void*))
+walkfintab(void (*fn)(void*), void (*scan)(int32, byte *, int64))
 {
 	void **key;
 	void **ekey;
 
+	scan(0, (byte*)&fintab, sizeof fintab);
 	lock(&finlock);
 	key = fintab.key;
 	ekey = key + fintab.max;
diff -r db5d6acb8d42 libgo/runtime/mgc0.c
--- a/libgo/runtime/mgc0.c	Sun Aug 01 03:18:13 2010 -0700
+++ b/libgo/runtime/mgc0.c	Sun Aug 01 03:20:48 2010 -0700
@@ -103,80 +103,50 @@ 
 	scanblock(1, v, size);
 }
 
-struct globals {
-	byte *start;
-	uintptr size;
+struct root_list {
+	struct root_list *next;
+	struct root {
+		void *decl;
+		size_t size;
+	} roots[];
 };
 
-// FIXME: This needs to grow as needed.
-#define GLOBALS_ENTRIES 16
+static struct root_list* roots;
 
-static struct globals globals[GLOBALS_ENTRIES];
-
-// Called by runtime.
 void
-__go_register_mem(void *start, void *end)
+__go_register_gc_roots (struct root_list* r)
 {
-	int i;
-
-	if(start == nil || end == nil)
-		throw("__go_register_mem");
-	if(start == end)
-		return;
-	for(i = 0; i < GLOBALS_ENTRIES; ++i) {
-		if(globals[i].start == nil) {
-			globals[i].start = (byte*)start;
-			globals[i].size = (byte*)end - (byte*)start;
-			return;
-		}
-	}
-	throw("__go_register_mem out of space");
-}
-
-// Called by runtime for dlclose.
-void
-__go_deregister_mem(void *start, void *end)
-{
-	int i;
-
-	if(start == end)
-		return;
-	for(i = 0; i < GLOBALS_ENTRIES; ++i) {
-		if(globals[i].start == (byte*)start
-		   && globals[i].size == (size_t)((byte*)end - (byte*)start)) {
-			globals[i].start = nil;
-			return;
-		}
-	}
-	throw("__go_deregister_mem not found");
+	// FIXME: This needs locking if multiple goroutines can call
+	// dlopen simultaneously.
+	r->next = roots;
+	roots = r;
 }
 
 static void
 mark(void)
 {
-	int i;
+	struct root_list *pl;
 
-	// mark data+bss.
-	// skip mheap itself, which has no interesting pointers
-	// and is mostly zeroed and would not otherwise be paged in.
-	for(i = 0; i < GLOBALS_ENTRIES; ++i) {
-		if (globals[i].start == nil)
-			continue;
-		if ((byte*)&mheap >= globals[i].start
-		    && (byte*)&mheap < globals[i].start + globals[i].size) {
-			scanblock(0, globals[i].start, (byte*)&mheap - globals[i].start);
-			scanblock(0, (byte*)(&mheap+1),
-				  globals[i].start + globals[i].size - (byte*)(&mheap+1));
+	for(pl = roots; pl != nil; pl = pl->next) {
+		struct root* pr = &pl->roots[0];
+		while(1) {
+			void *decl = pr->decl;
+			if(decl == nil)
+				break;
+			scanblock(0, decl, pr->size);
+			pr++;
 		}
-		else
-			scanblock(0, globals[i].start, globals[i].size);
 	}
 
+	scanblock(0, (byte*)&m0, sizeof m0);
+	scanblock(0, (byte*)&finq, sizeof finq);
+	MProf_Mark(scanblock);
+
 	// mark stacks
 	__go_scanstacks(scanblock);
 
 	// mark things pointed at by objects with finalizers
-	walkfintab(markfin);
+	walkfintab(markfin, scanblock);
 }
 
 // free RefNone, free & queue finalizers for RefNone|RefHasFinalizer, reset RefSome
diff -r db5d6acb8d42 libgo/runtime/mprof.goc
--- a/libgo/runtime/mprof.goc	Sun Aug 01 03:18:13 2010 -0700
+++ b/libgo/runtime/mprof.goc	Sun Aug 01 03:20:48 2010 -0700
@@ -288,3 +288,12 @@ 
 	if(__sync_bool_compare_and_swap(&m->gcing, 1, 0))
 		__go_run_goroutine_gc(102);
 }
+
+void
+MProf_Mark(void (*scan)(int32, byte *, int64))
+{
+	// buckhash is not allocated via mallocgc.
+	scan(0, (byte*)&buckets, sizeof buckets);
+	scan(0, (byte*)&addrhash, sizeof addrhash);
+	scan(0, (byte*)&addrfree, sizeof addrfree);
+}
diff -r db5d6acb8d42 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h	Sun Aug 01 03:18:13 2010 -0700
+++ b/libgo/runtime/runtime.h	Sun Aug 01 03:20:48 2010 -0700
@@ -72,6 +72,8 @@ 
 
 extern __thread		M* 	m;
 
+extern M	m0;
+
 #ifdef __rtems__
 #undef __thread
 #endif
@@ -164,7 +166,7 @@ 
 void	free(void *v);
 struct __go_func_type;
 void	addfinalizer(void*, void(*fn)(void*), const struct __go_func_type *);
-void	walkfintab(void (*fn)(void*));
+void	walkfintab(void (*fn)(void*), void (*scan)(int32, byte *, int64));
 #define runtime_mmap mmap
 #define cas(pval, old, new) __sync_bool_compare_and_swap (pval, old, new)
 
Index: gcc/crtstuff.c
===================================================================
--- gcc/crtstuff.c	(revision 162490)
+++ gcc/crtstuff.c	(working copy)
@@ -116,10 +116,6 @@  call_ ## FUNC (void)					\
 # define HIDDEN_DTOR_LIST_END
 #endif
 
-/* If the Go runtime is present, we currently always register memory
-   with it.  */
-#define GO_REGISTER_MEM
-
 /* We do not want to add the weak attribute to the declarations of these
    routines in unwind-dw2-fde.h because that will cause the definition of
    these symbols to be weak as well.
@@ -156,12 +152,6 @@  extern void __do_global_ctors_1 (void);
 /* Likewise for _Jv_RegisterClasses.  */
 extern void _Jv_RegisterClasses (void *) TARGET_ATTRIBUTE_WEAK;
 
-/* And for __go_register_mem.  */
-extern char _end[] TARGET_ATTRIBUTE_WEAK
-  __attribute__ ((__visibility__ ("hidden")));
-extern void __go_register_mem (void *, void *) TARGET_ATTRIBUTE_WEAK;
-extern void __go_deregister_mem (void *, void *) TARGET_ATTRIBUTE_WEAK;
-
 #ifdef OBJECT_FORMAT_ELF
 
 /*  Declare a pointer to void function type.  */
@@ -338,11 +328,6 @@  __do_global_dtors_aux (void)
 #endif
 #endif
 
-#ifdef GO_REGISTER_MEM
-  if (__go_deregister_mem)
-    __go_deregister_mem (__CTOR_LIST__, _end);
-#endif
-
   completed = 1;
 }
 
@@ -362,7 +347,7 @@  __do_global_dtors_aux_1 (void)
 CRT_CALL_STATIC_FUNCTION (INIT_SECTION_ASM_OP, __do_global_dtors_aux_1)
 #endif
 
-#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) || defined(GO_REGISTER_MEM)
+#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME)
 /* Stick a call to __register_frame_info into the .init section.  For some
    reason calls with no arguments work more reliably in .init, so stick the
    call in another function.  */
@@ -392,10 +377,6 @@  frame_dummy (void)
 	register_classes (__JCR_LIST__);
     }
 #endif /* JCR_SECTION_NAME */
-#ifdef GO_REGISTER_MEM
-  if (__go_register_mem)
-    __go_register_mem (__CTOR_LIST__, _end);
-#endif
 }
 
 #ifdef INIT_SECTION_ASM_OP
@@ -466,14 +447,9 @@  __do_global_dtors (void)
   if (__deregister_frame_info)
     __deregister_frame_info (__EH_FRAME_BEGIN__);
 #endif
-
-#ifdef GO_REGISTER_MEM
-  if (__go_deregister_mem)
-    __go_deregister_mem (__CTOR_LIST__, _end);
-#endif
 }
 
-#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) || defined(GO_REGISTER_MEM)
+#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME)
 /* A helper function for __do_global_ctors, which is in crtend.o.  Here
    in crtbegin.o, we can reference a couple of symbols not visible there.
    Plus, since we're before libgcc.a, we have no problems referencing
@@ -495,10 +471,6 @@  __do_global_ctors_1(void)
 	register_classes (__JCR_LIST__);
     }
 #endif
-#ifdef GO_REGISTER_MEM
-  if (__go_register_mem)
-    __go_register_mem (__CTOR_LIST__, _end);
-#endif
 }
 #endif /* USE_EH_FRAME_REGISTRY || JCR_SECTION_NAME */
 
@@ -644,7 +616,7 @@  void
 __do_global_ctors (void)
 {
   func_ptr *p;
-#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) || defined(GO_REGISTER_MEM)
+#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME)
   __do_global_ctors_1();
 #endif
   for (p = __CTOR_END__ - 1; *p != (func_ptr) -1; p--)