Go patch committed: Don't crash on invalid parameters/results

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

There are still a couple of cases in which the Go frontend can crash on
invalid parameters and results.  This patch does a better job of
checking errors in the parser, rather than just returning a short list.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

diff -r 660e66f0fa8e go/expressions.cc
--- a/go/expressions.cc	Tue Dec 14 12:52:23 2010 -0800
+++ b/go/expressions.cc	Tue Dec 14 14:45:39 2010 -0800
@@ -8662,8 +8662,7 @@
       ok = false;
     }
   if (!ok)
-    error_at(this->location(),
-	     "number of results does not match number of values");
+    this->report_error(_("number of results does not match number of values"));
 }
 
 // Determine the type.  We have nothing to do here, but the 0 result
diff -r 660e66f0fa8e go/parse.cc
--- a/go/parse.cc	Tue Dec 14 12:52:23 2010 -0800
+++ b/go/parse.cc	Tue Dec 14 14:45:39 2010 -0800
@@ -244,7 +244,10 @@
     {
       source_location location = token->location();
       this->advance_token();
-      return this->signature(NULL, location);
+      Type* type = this->signature(NULL, location);
+      if (type == NULL)
+	return Type::make_error_type();
+      return type;
     }
   else if (token->is_keyword(KEYWORD_MAP))
     return this->map_type();
@@ -662,16 +665,25 @@
 // RECEIVER is the receiver if there is one, or NULL.  LOCATION is the
 // location of the start of the type.
 
+// This returns NULL on a parse error.
+
 Function_type*
 Parse::signature(Typed_identifier* receiver, source_location location)
 {
   bool is_varargs = false;
-  Typed_identifier_list* params = this->parameters(&is_varargs);
+  Typed_identifier_list* params;
+  bool params_ok = this->parameters(&params, &is_varargs);
 
   Typed_identifier_list* result = NULL;
   if (this->peek_token()->is_op(OPERATOR_LPAREN)
       || this->type_may_start_here())
-    result = this->result();
+    {
+      if (!this->result(&result))
+	return NULL;
+    }
+
+  if (!params_ok)
+    return NULL;
 
   Function_type* ret = Type::make_function_type(receiver, params, result,
 						location);
@@ -682,21 +694,28 @@
 
 // Parameters     = "(" [ ParameterList [ "," ] ] ")" .
 
-Typed_identifier_list*
-Parse::parameters(bool* is_varargs)
+// This returns false on a parse error.
+
+bool
+Parse::parameters(Typed_identifier_list** pparams, bool* is_varargs)
 {
+  *pparams = NULL;
+
   if (!this->peek_token()->is_op(OPERATOR_LPAREN))
     {
       error_at(this->location(), "expected %<(%>");
-      return NULL;
+      return false;
     }
 
   Typed_identifier_list* params = NULL;
+  bool saw_error = false;
 
   const Token* token = this->advance_token();
   if (!token->is_op(OPERATOR_RPAREN))
     {
       params = this->parameter_list(is_varargs);
+      if (params == NULL)
+	saw_error = true;
       token = this->peek_token();
     }
 
@@ -707,7 +726,11 @@
   else
     this->advance_token();
 
-  return params;
+  if (saw_error)
+    return false;
+
+  *pparams = params;
+  return true;
 }
 
 // ParameterList  = ParameterDecl { "," ParameterDecl } .
@@ -717,12 +740,16 @@
 
 // We pick up an optional trailing comma.
 
+// This returns NULL if some error is seen.
+
 Typed_identifier_list*
 Parse::parameter_list(bool* is_varargs)
 {
   source_location location = this->location();
   Typed_identifier_list* ret = new Typed_identifier_list();
 
+  bool saw_error = false;
+
   // If we see an identifier and then a comma, then we don't know
   // whether we are looking at a list of identifiers followed by a
   // type, or a list of types given by name.  We have to do an
@@ -834,15 +861,16 @@
 	      else
 		{
 		  error_at(this->location(), "%<...%> only permits one name");
+		  saw_error = true;
 		  this->advance_token();
 		  type = this->type();
 		}
 	      for (size_t i = 0; i < ret->size(); ++i)
 		ret->set_type(i, type);
 	      if (!this->peek_token()->is_op(OPERATOR_COMMA))
-		return ret;
+		return saw_error ? NULL : ret;
 	      if (this->advance_token()->is_op(OPERATOR_RPAREN))
-		return ret;
+		return saw_error ? NULL : ret;
 	    }
 	  else
 	    {
@@ -865,6 +893,7 @@
 		    {
 		      error_at(p->location(), "expected %<%s%> to be a type",
 			       Gogo::message_name(p->name()).c_str());
+		      saw_error = true;
 		      type = Type::make_error_type();
 		    }
 		  tret->push_back(Typed_identifier("", type, p->location()));
@@ -873,7 +902,7 @@
 	      ret = tret;
 	      if (!just_saw_comma
 		  || this->peek_token()->is_op(OPERATOR_RPAREN))
-		return ret;
+		return saw_error ? NULL : ret;
 	    }
 	}
     }
@@ -883,13 +912,24 @@
   while (this->peek_token()->is_op(OPERATOR_COMMA))
     {
       if (is_varargs != NULL && *is_varargs)
-	error_at(this->location(), "%<...%> must be last parameter");
+	{
+	  error_at(this->location(), "%<...%> must be last parameter");
+	  saw_error = true;
+	}
       if (this->advance_token()->is_op(OPERATOR_RPAREN))
 	break;
       this->parameter_decl(parameters_have_names, ret, is_varargs, &mix_error);
     }
   if (mix_error)
-    error_at(location, "invalid named/anonymous mix");
+    {
+      error_at(location, "invalid named/anonymous mix");
+      saw_error = true;
+    }
+  if (saw_error)
+    {
+      delete ret;
+      return NULL;
+    }
   return ret;
 }
 
@@ -973,18 +1013,26 @@
 
 // Result         = Parameters | Type .
 
-Typed_identifier_list*
-Parse::result()
+// This returns false on a parse error.
+
+bool
+Parse::result(Typed_identifier_list** presults)
 {
   if (this->peek_token()->is_op(OPERATOR_LPAREN))
-    return this->parameters(NULL);
+    return this->parameters(presults, NULL);
   else
     {
       source_location location = this->location();
+      Type* type = this->type();
+      if (type->is_error_type())
+	{
+	  *presults = NULL;
+	  return false;
+	}
       Typed_identifier_list* til = new Typed_identifier_list();
-      Type* type = this->type();
       til->push_back(Typed_identifier("", type, location));
-      return til;
+      *presults = til;
+      return true;
     }
 }
 
@@ -1108,14 +1156,14 @@
 // MethodName         = identifier .
 // InterfaceTypeName  = TypeName .
 
-bool
+void
 Parse::method_spec(Typed_identifier_list* methods)
 {
   const Token* token = this->peek_token();
   if (!token->is_identifier())
     {
       error_at(this->location(), "expected identifier");
-      return false;
+      return;
     }
 
   std::string name = token->identifier();
@@ -1126,7 +1174,9 @@
     {
       // This is a MethodName.
       name = this->gogo_->pack_hidden_name(name, is_exported);
-      Function_type* type = this->signature(NULL, location);
+      Type* type = this->signature(NULL, location);
+      if (type == NULL)
+	return;
       methods->push_back(Typed_identifier(name, type, location));
     }
   else
@@ -1148,15 +1198,13 @@
 		 && !token->is_op(OPERATOR_SEMICOLON)
 		 && !token->is_op(OPERATOR_RCURLY))
 	    token = this->advance_token();
-	  return false;
+	  return;
 	}
       // This must be an interface type, but we can't check that now.
       // We check it and pull out the methods in
       // Interface_type::do_verify.
       methods->push_back(Typed_identifier("", type, location));
     }
-
-  return false;
 }
 
 // Declaration = ConstDecl | TypeDecl | VarDecl | FunctionDecl | MethodDecl .
@@ -1933,6 +1981,8 @@
   this->advance_token();
 
   Function_type* fntype = this->signature(rec, this->location());
+  if (fntype == NULL)
+    return;
 
   Named_object* named_object = NULL;
 
@@ -2462,6 +2512,11 @@
   hold_enclosing_vars.swap(this->enclosing_vars_);
 
   Function_type* type = this->signature(NULL, location);
+  if (type == NULL)
+    {
+      this->block();
+      return Expression::make_error(location);
+    }
 
   // For a function literal, the next token must be a '{'.  If we
   // don't see that, then we may have a type expression.
diff -r 660e66f0fa8e go/parse.h
--- a/go/parse.h	Tue Dec 14 12:52:23 2010 -0800
+++ b/go/parse.h	Tue Dec 14 14:45:39 2010 -0800
@@ -167,13 +167,13 @@
   Type* pointer_type();
   Type* channel_type();
   Function_type* signature(Typed_identifier*, source_location);
-  Typed_identifier_list* parameters(bool* is_varargs);
+  bool parameters(Typed_identifier_list**, bool* is_varargs);
   Typed_identifier_list* parameter_list(bool* is_varargs);
   void parameter_decl(bool, Typed_identifier_list*, bool*, bool*);
-  Typed_identifier_list* result();
+  bool result(Typed_identifier_list**);
   source_location block();
   Type* interface_type();
-  bool method_spec(Typed_identifier_list*);
+  void method_spec(Typed_identifier_list*);
   void declaration();
   bool declaration_may_start_here();
   void decl(void (Parse::*)(void*), void*);