Go patch committed: Fix bug taking address of variable

[Ian Lance Taylor <iant at google dot com>]

This patch to the Go frontend fixes a bug when taking the address of a
variable when the address of the variable does not escape the function.
When the address does escape the variable is moved onto the heap, to
avoid danling pointers.  When the address does not escape this is not
necessary.  However, gcc requires that TREE_ADDRESSABLE be set in that
case, and the Go frontend was not doing that.  This patch fixes that
bug.  Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian


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

	* go-gcc.cc (Gcc_backend::local_variable): Add is_address_taken
	parameter.
	(Gcc_backend::parameter_variable): Likewise.


Index: gcc/go/go-gcc.cc
===================================================================
--- gcc/go/go-gcc.cc	(revision 173685)
+++ gcc/go/go-gcc.cc	(working copy)
@@ -260,11 +260,11 @@ class Gcc_backend : public Backend
   global_variable_set_init(Bvariable*, Bexpression*);
 
   Bvariable*
-  local_variable(Bfunction*, const std::string& name, Btype* type,
+  local_variable(Bfunction*, const std::string&, Btype*, bool,
 		 source_location);
 
   Bvariable*
-  parameter_variable(Bfunction*, const std::string& name, Btype* type,
+  parameter_variable(Bfunction*, const std::string&, Btype*, bool,
 		     source_location);
 
   Bvariable*
@@ -1074,7 +1074,8 @@ Gcc_backend::global_variable_set_init(Bv
 
 Bvariable*
 Gcc_backend::local_variable(Bfunction* function, const std::string& name,
-			    Btype* btype, source_location location)
+			    Btype* btype, bool is_address_taken,
+			    source_location location)
 {
   tree type_tree = btype->get_tree();
   if (type_tree == error_mark_node)
@@ -1084,6 +1085,8 @@ Gcc_backend::local_variable(Bfunction* f
 			 type_tree);
   DECL_CONTEXT(decl) = function->get_tree();
   TREE_USED(decl) = 1;
+  if (is_address_taken)
+    TREE_ADDRESSABLE(decl) = 1;
   go_preserve_from_gc(decl);
   return new Bvariable(decl);
 }
@@ -1092,7 +1095,8 @@ Gcc_backend::local_variable(Bfunction* f
 
 Bvariable*
 Gcc_backend::parameter_variable(Bfunction* function, const std::string& name,
-				Btype* btype, source_location location)
+				Btype* btype, bool is_address_taken,
+				source_location location)
 {
   tree type_tree = btype->get_tree();
   if (type_tree == error_mark_node)
@@ -1103,6 +1107,8 @@ Gcc_backend::parameter_variable(Bfunctio
   DECL_CONTEXT(decl) = function->get_tree();
   DECL_ARG_TYPE(decl) = type_tree;
   TREE_USED(decl) = 1;
+  if (is_address_taken)
+    TREE_ADDRESSABLE(decl) = 1;
   go_preserve_from_gc(decl);
   return new Bvariable(decl);
 }
Index: gcc/go/gofrontend/gogo.cc
===================================================================
--- gcc/go/gofrontend/gogo.cc	(revision 173685)
+++ gcc/go/gofrontend/gogo.cc	(working copy)
@@ -3333,10 +3333,11 @@ Variable::Variable(Type* type, Expressio
   : type_(type), init_(init), preinit_(NULL), location_(location),
     backend_(NULL), is_global_(is_global), is_parameter_(is_parameter),
     is_receiver_(is_receiver), is_varargs_parameter_(false),
-    is_address_taken_(false), seen_(false), init_is_lowered_(false),
-    type_from_init_tuple_(false), type_from_range_index_(false),
-    type_from_range_value_(false), type_from_chan_element_(false),
-    is_type_switch_var_(false), determined_type_(false)
+    is_address_taken_(false), is_non_escaping_address_taken_(false),
+    seen_(false), init_is_lowered_(false), type_from_init_tuple_(false),
+    type_from_range_index_(false), type_from_range_value_(false),
+    type_from_chan_element_(false), is_type_switch_var_(false),
+    determined_type_(false)
 {
   go_assert(type != NULL || init != NULL);
   go_assert(!is_parameter || init == NULL);
@@ -3722,11 +3723,15 @@ Variable::get_backend_variable(Gogo* gog
 	    {
 	      tree fndecl = function->func_value()->get_decl();
 	      Bfunction* bfunction = tree_to_function(fndecl);
+	      bool is_address_taken = (this->is_non_escaping_address_taken_
+				       && !this->is_in_heap());
 	      if (is_parameter)
 		bvar = backend->parameter_variable(bfunction, n, btype,
+						   is_address_taken,
 						   this->location_);
 	      else
 		bvar = backend->local_variable(bfunction, n, btype,
+					       is_address_taken,
 					       this->location_);
 	    }
 	  this->backend_ = bvar;
@@ -3757,7 +3762,10 @@ Result_variable::get_backend_variable(Go
 	  tree fndecl = function->func_value()->get_decl();
 	  Bfunction* bfunction = tree_to_function(fndecl);
 	  std::string n = Gogo::unpack_hidden_name(name);
+	  bool is_address_taken = (this->is_non_escaping_address_taken_
+				   && !this->is_in_heap());
 	  this->backend_ = backend->local_variable(bfunction, n, btype,
+						   is_address_taken,
 						   this->location_);
 	}
     }
Index: gcc/go/gofrontend/gogo.h
===================================================================
--- gcc/go/gofrontend/gogo.h	(revision 173685)
+++ gcc/go/gofrontend/gogo.h	(working copy)
@@ -1160,6 +1160,22 @@ class Variable
   is_in_heap() const
   { return this->is_address_taken_ && !this->is_global_; }
 
+  // Note that something takes the address of this variable.
+  void
+  set_address_taken()
+  { this->is_address_taken_ = true; }
+
+  // Return whether the address is taken but does not escape.
+  bool
+  is_non_escaping_address_taken() const
+  { return this->is_non_escaping_address_taken_; }
+
+  // Note that something takes the address of this variable such that
+  // the address does not escape the function.
+  void
+  set_non_escaping_address_taken()
+  { this->is_non_escaping_address_taken_ = true; }
+
   // Get the source location of the variable's declaration.
   source_location
   location() const
@@ -1252,11 +1268,6 @@ class Variable
   void
   determine_type();
 
-  // Note that something takes the address of this variable.
-  void
-  set_address_taken()
-  { this->is_address_taken_ = true; }
-
   // Get the backend representation of the variable.
   Bvariable*
   get_backend_variable(Gogo*, Named_object*, const Package*,
@@ -1314,8 +1325,13 @@ class Variable
   bool is_receiver_ : 1;
   // Whether this is the varargs parameter of a function.
   bool is_varargs_parameter_ : 1;
-  // Whether something takes the address of this variable.
+  // Whether something takes the address of this variable.  For a
+  // local variable this implies that the variable has to be on the
+  // heap.
   bool is_address_taken_ : 1;
+  // Whether something takes the address of this variable such that
+  // the address does not escape the function.
+  bool is_non_escaping_address_taken_ : 1;
   // True if we have seen this variable in a traversal.
   bool seen_ : 1;
   // True if we have lowered the initialization expression.
@@ -1343,7 +1359,8 @@ class Result_variable
   Result_variable(Type* type, Function* function, int index,
 		  source_location location)
     : type_(type), function_(function), index_(index), location_(location),
-      backend_(NULL), is_address_taken_(false)
+      backend_(NULL), is_address_taken_(false),
+      is_non_escaping_address_taken_(false)
   { }
 
   // Get the type of the result variable.
@@ -1376,6 +1393,17 @@ class Result_variable
   set_address_taken()
   { this->is_address_taken_ = true; }
 
+  // Return whether the address is taken but does not escape.
+  bool
+  is_non_escaping_address_taken() const
+  { return this->is_non_escaping_address_taken_; }
+
+  // Note that something takes the address of this variable such that
+  // the address does not escape the function.
+  void
+  set_non_escaping_address_taken()
+  { this->is_non_escaping_address_taken_ = true; }
+
   // Whether this variable should live in the heap.
   bool
   is_in_heap() const
@@ -1404,6 +1432,9 @@ class Result_variable
   Bvariable* backend_;
   // Whether something takes the address of this variable.
   bool is_address_taken_;
+  // Whether something takes the address of this variable such that
+  // the address does not escape the function.
+  bool is_non_escaping_address_taken_;
 };
 
 // The value we keep for a named constant.  This lets us hold a type
Index: gcc/go/gofrontend/backend.h
===================================================================
--- gcc/go/gofrontend/backend.h	(revision 173685)
+++ gcc/go/gofrontend/backend.h	(working copy)
@@ -317,19 +317,23 @@ class Backend
   // Create a local variable.  The frontend will create the local
   // variables first, and then create the block which contains them.
   // FUNCTION is the function in which the variable is defined.  NAME
-  // is the name of the variable.  TYPE is the type.  LOCATION is
-  // where the variable is defined.  For each local variable the
-  // frontend will call init_statement to set the initial value.
+  // is the name of the variable.  TYPE is the type.  IS_ADDRESS_TAKEN
+  // is true if the address of this variable is taken (this implies
+  // that the address does not escape the function, as otherwise the
+  // variable would be on the heap).  LOCATION is where the variable
+  // is defined.  For each local variable the frontend will call
+  // init_statement to set the initial value.
   virtual Bvariable*
   local_variable(Bfunction* function, const std::string& name, Btype* type,
-		 source_location location) = 0;
+		 bool is_address_taken, source_location location) = 0;
 
   // Create a function parameter.  This is an incoming parameter, not
   // a result parameter (result parameters are treated as local
   // variables).  The arguments are as for local_variable.
   virtual Bvariable*
   parameter_variable(Bfunction* function, const std::string& name,
-		     Btype* type, source_location location) = 0;
+		     Btype* type, bool is_address_taken,
+		     source_location location) = 0;
 
   // Create a temporary variable.  A temporary variable has no name,
   // just a type.  We pass in FUNCTION and BLOCK in case they are
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 173685)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -958,13 +958,23 @@ void
 Var_expression::do_address_taken(bool escapes)
 {
   if (!escapes)
-    ;
-  else if (this->variable_->is_variable())
-    this->variable_->var_value()->set_address_taken();
-  else if (this->variable_->is_result_variable())
-    this->variable_->result_var_value()->set_address_taken();
+    {
+      if (this->variable_->is_variable())
+	this->variable_->var_value()->set_non_escaping_address_taken();
+      else if (this->variable_->is_result_variable())
+	this->variable_->result_var_value()->set_non_escaping_address_taken();
+      else
+	go_unreachable();
+    }
   else
-    go_unreachable();
+    {
+      if (this->variable_->is_variable())
+	this->variable_->var_value()->set_address_taken();
+      else if (this->variable_->is_result_variable())
+	this->variable_->result_var_value()->set_address_taken();
+      else
+	go_unreachable();
+    }
 }
 
 // Get the tree for a reference to a variable.