compiler, runtime: Fix unexpected GC interfering with closure passing.

The Go frontend passes closures through to functions using the
functions __go_set_closure and __go_get_closure.  The
expectation is that there are no function calls between
set_closure and get_closure.  However, it turns out that there
can be function calls if some of the function arguments
require type conversion to an interface type.  Converting to
an interface type can allocate memory, and that can in turn
trigger a garbage collection, and that can in turn call pool
cleanup functions that may call __go_set_closure.  So the
called function can see the wrong closure value, which is bad.

This patch fixes the problem in two different ways.  First, we
move all type conversions in function arguments into temporary
variables so that they can not appear before the call to
__go_set_closure.  (This required shifting the flatten phase
after the simplify_thunk phase, since the latter expects to
work with unconverted argument types.)  Second, we fix the
memory allocation function to preserve the closure value
across any possible garbage collection.

A test case is the libgo database/sql check run with the
environment variable GOGC set to 1.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@213932 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index adc4fb0..f7a3c57 100644 (file)
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -9013,8 +9013,51 @@ Call_expression::lower_varargs(Gogo* gogo, Named_object* function,
 // Flatten a call with multiple results into a temporary.
 
 Expression*
-Call_expression::do_flatten(Gogo*, Named_object*, Statement_inserter* inserter)
+Call_expression::do_flatten(Gogo* gogo, Named_object*,
+                           Statement_inserter* inserter)
 {
+  if (this->classification() == EXPRESSION_ERROR)
+    return this;
+
+  // Add temporary variables for all arguments that require type
+  // conversion.
+  Function_type* fntype = this->get_function_type();
+  go_assert(fntype != NULL);
+  if (this->args_ != NULL && !this->args_->empty()
+      && fntype->parameters() != NULL && !fntype->parameters()->empty())
+    {
+      bool is_interface_method =
+       this->fn_->interface_field_reference_expression() != NULL;
+
+      Expression_list *args = new Expression_list();
+      Typed_identifier_list::const_iterator pp = fntype->parameters()->begin();
+      Expression_list::const_iterator pa = this->args_->begin();
+      if (!is_interface_method && fntype->is_method())
+       {
+         // The receiver argument.
+         args->push_back(*pa);
+         ++pa;
+       }
+      for (; pa != this->args_->end(); ++pa, ++pp)
+       {
+         go_assert(pp != fntype->parameters()->end());
+         if (Type::are_identical(pp->type(), (*pa)->type(), true, NULL))
+           args->push_back(*pa);
+         else
+           {
+             Location loc = (*pa)->location();
+             Expression* arg =
+               Expression::convert_for_assignment(gogo, pp->type(), *pa, loc);
+             Temporary_statement* temp =
+               Statement::make_temporary(pp->type(), arg, loc);
+             inserter->insert(temp);
+             args->push_back(Expression::make_temporary_reference(temp, loc));
+           }
+       }
+      delete this->args_;
+      this->args_ = args;
+    }
+
   size_t rc = this->result_count();
   if (rc > 1 && this->call_temp_ == NULL)
     {

diff --git a/gcc/go/gofrontend/go.cc b/gcc/go/gofrontend/go.cc
index 222ea90..1772623 100644 (file)
--- a/gcc/go/gofrontend/go.cc
+++ b/gcc/go/gofrontend/go.cc
@@ -124,15 +124,15 @@ go_parse_input_files(const char** filenames, unsigned int filename_count,
   // Convert named types to backend representation.
   ::gogo->convert_named_types();
 
-  // Flatten the parse tree.
-  ::gogo->flatten();
-
   // Build thunks for functions which call recover.
   ::gogo->build_recover_thunks();
 
   // Convert complicated go and defer statements into simpler ones.
   ::gogo->simplify_thunk_statements();
 
+  // Flatten the parse tree.
+  ::gogo->flatten();
+
   // Dump ast, use filename[0] as the base name
   ::gogo->dump_ast(filenames[0]);
 }

diff --git a/libgo/runtime/malloc.goc b/libgo/runtime/malloc.goc
index 0288722..c5e64c8 100644 (file)
--- a/libgo/runtime/malloc.goc
+++ b/libgo/runtime/malloc.goc
@@ -84,6 +84,7 @@ runtime_mallocgc(uintptr size, uintptr typ, uint32 flag)
        MLink *v, *next;
        byte *tiny;
        bool incallback;
+       void *closure;
 
        if(size == 0) {
                // All 0-length allocations use this pointer.
@@ -95,6 +96,10 @@ runtime_mallocgc(uintptr size, uintptr typ, uint32 flag)
        m = runtime_m();
        g = runtime_g();
 
+       // We should not be called in between __go_set_closure and the
+       // actual function call, but cope with it if we are.
+       closure = g->closure;
+
        incallback = false;
        if(m->mcache == nil && g->ncgo > 0) {
                // For gccgo this case can occur when a cgo or SWIG function
@@ -175,6 +180,7 @@ runtime_mallocgc(uintptr size, uintptr typ, uint32 flag)
                                        m->locks--;
                                        if(incallback)
                                                runtime_entersyscall();
+                                       g->closure = closure;
                                        return v;
                                }
                        }
@@ -264,6 +270,8 @@ runtime_mallocgc(uintptr size, uintptr typ, uint32 flag)
        if(incallback)
                runtime_entersyscall();
 
+       g->closure = closure;
+
        return v;
 }