Patchwork Go patch committed: Fix crash on go/defer of some builtin functions

login
register
mail settings
Submitter Ian Taylor
Date Dec. 4, 2012, 5:17 a.m.
Message ID <mcrobias8e1.fsf@google.com>
Download mbox | patch
Permalink /patch/203559/
State New
Headers show

Comments

Ian Taylor - Dec. 4, 2012, 5:17 a.m.
This patch to the Go compiler fixes a crash when using go or defer with
some builtin functions, namely those that have no side-effects.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.7 branch.

Ian

Patch

diff -r 952fc7825cfa go/expressions.cc
--- a/go/expressions.cc	Mon Dec 03 16:27:21 2012 -0800
+++ b/go/expressions.cc	Mon Dec 03 20:57:51 2012 -0800
@@ -80,10 +80,11 @@ 
 // expression is being discarded.  By default, we give an error.
 // Expressions with side effects override.
 
-void
+bool
 Expression::do_discarding_value()
 {
   this->unused_value_error();
+  return false;
 }
 
 // This virtual function is called to export expressions.  This will
@@ -100,7 +101,7 @@ 
 void
 Expression::unused_value_error()
 {
-  error_at(this->location(), "value computed is not used");
+  this->report_error(_("value computed is not used"));
 }
 
 // Note that this expression is an error.  This is called by children
@@ -786,9 +787,9 @@ 
     return true;
   }
 
-  void
+  bool
   do_discarding_value()
-  { }
+  { return true; }
 
   Type*
   do_type()
@@ -1149,9 +1150,9 @@ 
   { }
 
  protected:
-  void
+  bool
   do_discarding_value()
-  { }
+  { return true; }
 
   Type*
   do_type();
@@ -5326,13 +5327,19 @@ 
 
 // Note that the value is being discarded.
 
-void
+bool
 Binary_expression::do_discarding_value()
 {
   if (this->op_ == OPERATOR_OROR || this->op_ == OPERATOR_ANDAND)
-    this->right_->discarding_value();
-  else
-    this->unused_value_error();
+    {
+      this->right_->discarding_value();
+      return true;
+    }
+  else
+    {
+      this->unused_value_error();
+      return false;
+    }
 }
 
 // Get type.
@@ -6536,7 +6543,7 @@ 
   bool
   do_numeric_constant_value(Numeric_constant*) const;
 
-  void
+  bool
   do_discarding_value();
 
   Type*
@@ -7338,7 +7345,7 @@ 
 // discarding the value of an ordinary function call, but we do for
 // builtin functions, purely for consistency with the gc compiler.
 
-void
+bool
 Builtin_call_expression::do_discarding_value()
 {
   switch (this->code_)
@@ -7359,7 +7366,7 @@ 
     case BUILTIN_OFFSETOF:
     case BUILTIN_SIZEOF:
       this->unused_value_error();
-      break;
+      return false;
 
     case BUILTIN_CLOSE:
     case BUILTIN_COPY:
@@ -7368,7 +7375,7 @@ 
     case BUILTIN_PRINT:
     case BUILTIN_PRINTLN:
     case BUILTIN_RECOVER:
-      break;
+      return true;
     }
 }
 
diff -r 952fc7825cfa go/expressions.h
--- a/go/expressions.h	Mon Dec 03 16:27:21 2012 -0800
+++ b/go/expressions.h	Mon Dec 03 20:57:51 2012 -0800
@@ -360,10 +360,11 @@ 
 
   // This is called if the value of this expression is being
   // discarded.  This issues warnings about computed values being
-  // unused.
-  void
+  // unused.  This returns true if all is well, false if it issued an
+  // error message.
+  bool
   discarding_value()
-  { this->do_discarding_value(); }
+  { return this->do_discarding_value(); }
 
   // Return whether this is an error expression.
   bool
@@ -689,7 +690,7 @@ 
   { return false; }
 
   // Called by the parser if the value is being discarded.
-  virtual void
+  virtual bool
   do_discarding_value();
 
   // Child class holds type.
@@ -1205,7 +1206,7 @@ 
   bool
   do_numeric_constant_value(Numeric_constant*) const;
 
-  void
+  bool
   do_discarding_value();
 
   Type*
@@ -1373,9 +1374,9 @@ 
   virtual Expression*
   do_lower(Gogo*, Named_object*, Statement_inserter*, int);
 
-  void
+  bool
   do_discarding_value()
-  { }
+  { return true; }
 
   virtual Type*
   do_type();
@@ -2056,9 +2057,9 @@ 
   do_traverse(Traverse* traverse)
   { return Expression::traverse(&this->channel_, traverse); }
 
-  void
+  bool
   do_discarding_value()
-  { }
+  { return true; }
 
   Type*
   do_type();
diff -r 952fc7825cfa go/statements.cc
--- a/go/statements.cc	Mon Dec 03 16:27:21 2012 -0800
+++ b/go/statements.cc	Mon Dec 03 20:57:51 2012 -0800
@@ -2006,6 +2006,8 @@ 
 void
 Thunk_statement::do_check_types(Gogo*)
 {
+  if (!this->call_->discarding_value())
+    return;
   Call_expression* ce = this->call_->call_expression();
   if (ce == NULL)
     {
@@ -2471,11 +2473,15 @@ 
       Expression_statement* es =
 	static_cast<Expression_statement*>(call_statement);
       Call_expression* ce = es->expr()->call_expression();
-      go_assert(ce != NULL);
-      if (may_call_recover)
-	ce->set_is_deferred();
-      if (recover_arg != NULL)
-	ce->set_recover_arg(recover_arg);
+      if (ce == NULL)
+	go_assert(saw_errors());
+      else
+	{
+	  if (may_call_recover)
+	    ce->set_is_deferred();
+	  if (recover_arg != NULL)
+	    ce->set_recover_arg(recover_arg);
+	}
     }
 
   // That is all the thunk has to do.