Patchwork Go patch committed: Fix bug with multiple results

login
register
mail settings
Submitter Ian Taylor
Date May 13, 2011, 10:34 p.m.
Message ID <mcrpqnm6xgt.fsf@coign.corp.google.com>
Download mbox | patch
Permalink /patch/95536/
State New
Headers show

Comments

Ian Taylor - May 13, 2011, 10:34 p.m.
This patch to the Go frontend fixes a bug in which a function returns
multiple results and at least one of those results is a named struct.
For multiple results the Go frontend builds a struct.  In some cases the
function type may be built before the named struct type is completed, in
which case the result struct will be laid out incorrectly.  This patch
fixes the problem by postponing building the result struct until all
named types are converted.  I added an assertion to detect this kind of
thing in the future.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


2011-05-13  Ian Lance Taylor  <iant@google.com>

	* go-gcc.cc (Gcc_backend::function_type): When building a struct
	for multiple results, check that all fields types have a size.
	(Gcc_backend::placeholder_pointer_type): Permit name to be empty.

Patch

Index: gcc/go/go-gcc.cc
===================================================================
--- gcc/go/go-gcc.cc	(revision 173712)
+++ gcc/go/go-gcc.cc	(working copy)
@@ -465,6 +465,7 @@  Gcc_backend::function_type(const Btyped_
 	  tree field_type_tree = p->btype->get_tree();
 	  if (field_type_tree == error_mark_node)
 	    return this->error_type();
+	  gcc_assert(TYPE_SIZE(field_type_tree) != NULL_TREE);
 	  tree field = build_decl(location, FIELD_DECL, name_tree,
 				  field_type_tree);
 	  DECL_CONTEXT(field) = result;
@@ -573,10 +574,13 @@  Gcc_backend::placeholder_pointer_type(co
 				      source_location location, bool)
 {
   tree ret = build_variant_type_copy(ptr_type_node);
-  tree decl = build_decl(location, TYPE_DECL,
-			 get_identifier_from_string(name),
-			 ret);
-  TYPE_NAME(ret) = decl;
+  if (!name.empty())
+    {
+      tree decl = build_decl(location, TYPE_DECL,
+			     get_identifier_from_string(name),
+			     ret);
+      TYPE_NAME(ret) = decl;
+    }
   return this->make_type(ret);
 }
 
Index: gcc/go/gofrontend/gogo.cc
===================================================================
--- gcc/go/gofrontend/gogo.cc	(revision 173712)
+++ gcc/go/gofrontend/gogo.cc	(working copy)
@@ -2599,6 +2599,8 @@  Gogo::convert_named_types()
 
   Runtime::convert_types(this);
 
+  Function_type::convert_types(this);
+
   this->named_types_are_converted_ = true;
 }
 
Index: gcc/go/gofrontend/types.h
===================================================================
--- gcc/go/gofrontend/types.h	(revision 173685)
+++ gcc/go/gofrontend/types.h	(working copy)
@@ -1628,6 +1628,10 @@  class Function_type : public Type
   Function_type*
   copy_with_receiver(Type*) const;
 
+  // Finishing converting function types.
+  static void
+  convert_types(Gogo*);
+
   static Type*
   make_function_type_descriptor_type();
 
@@ -1666,6 +1670,16 @@  class Function_type : public Type
   type_descriptor_params(Type*, const Typed_identifier*,
 			 const Typed_identifier_list*);
 
+  Btype*
+  get_function_backend(Gogo*);
+
+  // A list of function types with multiple results and their
+  // placeholder backend representations, used to postpone building
+  // the structs we use for multiple results until all types are
+  // converted.
+  typedef std::vector<std::pair<Function_type*, Btype*> > Placeholders;
+  static Placeholders placeholders;
+
   // The receiver name and type.  This will be NULL for a normal
   // function, non-NULL for a method.
   Typed_identifier* receiver_;
Index: gcc/go/gofrontend/backend.h
===================================================================
--- gcc/go/gofrontend/backend.h	(revision 173712)
+++ gcc/go/gofrontend/backend.h	(working copy)
@@ -113,7 +113,9 @@  class Backend
   // Create a placeholder pointer type.  This is used for a named
   // pointer type, since in Go a pointer type may refer to itself.
   // NAME is the name of the type, and the location is where the named
-  // type is defined.  FOR_FUNCTION is true if this is for a Go
+  // type is defined.  This function is also used for unnamed function
+  // types with multiple results, in which case the type has no name
+  // and NAME will be empty.  FOR_FUNCTION is true if this is for a Go
   // function type, which corresponds to a C/C++ pointer to function
   // type.  The return value will later be passed as the first
   // parameter to set_placeholder_pointer_type or
Index: gcc/go/gofrontend/types.cc
===================================================================
--- gcc/go/gofrontend/types.cc	(revision 173685)
+++ gcc/go/gofrontend/types.cc	(working copy)
@@ -831,7 +831,8 @@  Type::check_int_value(Expression* e, con
   return false;
 }
 
-// A hash table mapping unnamed types to trees.
+// A hash table mapping unnamed types to the backend representation of
+// those types.
 
 Type::Type_btypes Type::type_btypes;
 
@@ -2588,10 +2589,10 @@  Function_type::do_hash_for_method(Gogo* 
   return ret;
 }
 
-// Get the tree for a function type.
+// Get the backend representation for a function type.
 
 Btype*
-Function_type::do_get_backend(Gogo* gogo)
+Function_type::get_function_backend(Gogo* gogo)
 {
   Backend::Btyped_identifier breceiver;
   if (this->receiver_ != NULL)
@@ -2643,6 +2644,46 @@  Function_type::do_get_backend(Gogo* gogo
 					this->location());
 }
 
+// A hash table mapping function types to their backend placeholders.
+
+Function_type::Placeholders Function_type::placeholders;
+
+// Get the backend representation for a function type.  If we are
+// still converting types, and this types has multiple results, return
+// a placeholder instead.  We do this because for multiple results we
+// build a struct, and we need to make sure that all the types in the
+// struct are valid before we create the struct.
+
+Btype*
+Function_type::do_get_backend(Gogo* gogo)
+{
+  if (!gogo->named_types_are_converted()
+      && this->results_ != NULL
+      && this->results_->size() > 1)
+    {
+      Btype* placeholder =
+	gogo->backend()->placeholder_pointer_type("", this->location(), true);
+      Function_type::placeholders.push_back(std::make_pair(this, placeholder));
+      return placeholder;
+    }
+  return this->get_function_backend(gogo);
+}
+
+// Convert function types after all named types are converted.
+
+void
+Function_type::convert_types(Gogo* gogo)
+{
+  for (Placeholders::const_iterator p = Function_type::placeholders.begin();
+       p != Function_type::placeholders.end();
+       ++p)
+    {
+      Btype* bt = p->first->get_function_backend(gogo);
+      if (!gogo->backend()->set_placeholder_function_type(p->second, bt))
+	go_assert(saw_errors());
+    }
+}
+
 // Functions are initialized to NULL.
 
 tree
@@ -7236,7 +7277,7 @@  Named_type::do_get_backend(Gogo* gogo)
       --this->seen_;
       if (this->is_circular_)
 	bt1 = gogo->backend()->circular_pointer_type(bt, true);
-      if (!gogo->backend()->set_placeholder_pointer_type(bt, bt1))
+      if (!gogo->backend()->set_placeholder_function_type(bt, bt1))
 	bt = gogo->backend()->error_type();
       return bt;