Go patch committed: Check for nil pointers when taking address

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

In Go 1.2 the language was changed slightly to prohibit taking the
address of a field in a struct given a nil pointer to the struct
(http://golang.org/s/go12nil).  This patch by Chris Manghane implements
that check in gccgo.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

diff -r 4f97ec70a5f5 go/expressions.cc
--- a/go/expressions.cc	Sat Nov 09 08:21:57 2013 -0800
+++ b/go/expressions.cc	Mon Nov 11 09:32:48 2013 -0800
@@ -3633,7 +3633,8 @@
  public:
   Unary_expression(Operator op, Expression* expr, Location location)
     : Expression(EXPRESSION_UNARY, location),
-      op_(op), escapes_(true), create_temp_(false), expr_(expr)
+      op_(op), escapes_(true), create_temp_(false), expr_(expr),
+      issue_nil_check_(false)
   { }
 
   // Return the operator.
@@ -3719,6 +3720,10 @@
   void
   do_dump_expression(Ast_dump_context*) const;
 
+  void
+  do_issue_nil_check()
+  { this->issue_nil_check_ = (this->op_ == OPERATOR_MULT); }
+
  private:
   // The unary operator to apply.
   Operator op_;
@@ -3730,6 +3735,9 @@
   bool create_temp_;
   // The operand.
   Expression* expr_;
+  // Whether or not to issue a nil check for this expression if its address
+  // is being taken.
+  bool issue_nil_check_;
 };
 
 // If we are taking the address of a composite literal, and the
@@ -4107,7 +4115,10 @@
 	    this->report_error(_("invalid operand for unary %<&%>"));
 	}
       else
-	this->expr_->address_taken(this->escapes_);
+        {
+          this->expr_->address_taken(this->escapes_);
+          this->expr_->issue_nil_check();
+        }
       break;
 
     case OPERATOR_MULT:
@@ -4277,12 +4288,13 @@
 	// If we are dereferencing the pointer to a large struct, we
 	// need to check for nil.  We don't bother to check for small
 	// structs because we expect the system to crash on a nil
-	// pointer dereference.
+	// pointer dereference.	 However, if we know the address of this
+	// expression is being taken, we must always check for nil.
 	tree target_type_tree = TREE_TYPE(TREE_TYPE(expr));
 	if (!VOID_TYPE_P(target_type_tree))
 	  {
 	    HOST_WIDE_INT s = int_size_in_bytes(target_type_tree);
-	    if (s == -1 || s >= 4096)
+	    if (s == -1 || s >= 4096 || this->issue_nil_check_)
 	      {
 		if (!DECL_P(expr))
 		  expr = save_expr(expr);
@@ -10402,6 +10414,10 @@
   do_address_taken(bool escapes)
   { this->array_->address_taken(escapes); }
 
+  void
+  do_issue_nil_check()
+  { this->array_->issue_nil_check(); }
+
   tree
   do_get_tree(Translate_context*);
 
diff -r 4f97ec70a5f5 go/expressions.h
--- a/go/expressions.h	Sat Nov 09 08:21:57 2013 -0800
+++ b/go/expressions.h	Mon Nov 11 09:32:48 2013 -0800
@@ -613,6 +613,11 @@
   address_taken(bool escapes)
   { this->do_address_taken(escapes); }
 
+  // Note that a nil check must be issued for this expression.
+  void
+  issue_nil_check()
+  { this->do_issue_nil_check(); }
+
   // Return whether this expression must be evaluated in order
   // according to the order of evaluation rules.  This is basically
   // true of all expressions with side-effects.
@@ -742,6 +747,11 @@
   do_address_taken(bool)
   { }
 
+  // Child class implements issuing a nil check if the address is taken.
+  virtual void
+  do_issue_nil_check()
+  { }
+
   // Child class implements whether this expression must be evaluated
   // in order.
   virtual bool
@@ -1721,6 +1731,9 @@
   void
   do_dump_expression(Ast_dump_context*) const;
 
+  void
+  do_issue_nil_check()
+  { this->left_->issue_nil_check(); }
  private:
   // The expression being indexed.
   Expression* left_;
@@ -2011,6 +2024,10 @@
   do_address_taken(bool escapes)
   { this->expr_->address_taken(escapes); }
 
+  void
+  do_issue_nil_check()
+  { this->expr_->issue_nil_check(); }
+
   tree
   do_get_tree(Translate_context*);
 
diff -r 4f97ec70a5f5 go/types.cc
--- a/go/types.cc	Sat Nov 09 08:21:57 2013 -0800
+++ b/go/types.cc	Mon Nov 11 09:32:48 2013 -0800
@@ -8847,8 +8847,7 @@
       bool is_value_method = (is_embedded_pointer
 			      || !Type::method_expects_pointer(no));
       Method* m = new Named_method(no, field_indexes, depth, is_value_method,
-				   (needs_stub_method
-				    || (depth > 0 && is_value_method)));
+				   (needs_stub_method || depth > 0));
       if (!(*methods)->insert(no->name(), m))
 	delete m;
     }