Patchwork Go patch committed: Don't crash on recursive consts or variables

login
register
mail settings
Submitter Ian Taylor
Date Dec. 18, 2010, 3:03 a.m.
Message ID <mcrtyibsryh.fsf@google.com>
Download mbox | patch
Permalink /patch/76054/
State New
Headers show

Comments

Ian Taylor - Dec. 18, 2010, 3:03 a.m.
This patch avoids some more crashes for recursive consts and
variables--that is, for cases where the initialization expression of a
global const or variable refers to the const or variable itself.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

Patch

diff -r bce4b61f3652 go/expressions.cc
--- a/go/expressions.cc	Thu Dec 16 22:34:34 2010 -0800
+++ b/go/expressions.cc	Fri Dec 17 18:57:00 2010 -0800
@@ -2285,6 +2285,32 @@ 
   return new Complex_expression(real, imag, type, location);
 }
 
+// Find a named object in an expression.
+
+class Find_named_object : public Traverse
+{
+ public:
+  Find_named_object(Named_object* no)
+    : Traverse(traverse_expressions),
+      no_(no), found_(false)
+  { }
+
+  // Whether we found the object.
+  bool
+  found() const
+  { return this->found_; }
+
+ protected:
+  int
+  expression(Expression**);
+
+ private:
+  // The object we are looking for.
+  Named_object* no_;
+  // Whether we found it.
+  bool found_;
+};
+
 // A reference to a const in an expression.
 
 class Const_expression : public Expression
@@ -2295,6 +2321,10 @@ 
       constant_(constant), type_(NULL), seen_(false)
   { }
 
+  Named_object*
+  named_object()
+  { return this->constant_; }
+
   const std::string&
   name() const
   { return this->constant_->name(); }
@@ -2565,6 +2595,19 @@ 
 void
 Const_expression::do_check_types(Gogo*)
 {
+  if (this->type_ != NULL && this->type_->is_error_type())
+    return;
+
+  Expression* init = this->constant_->const_value()->expr();
+  Find_named_object find_named_object(this->constant_);
+  Expression::traverse(&init, &find_named_object);
+  if (find_named_object.found())
+    {
+      this->report_error(_("constant refers to itself"));
+      this->type_ = Type::make_error_type();
+      return;
+    }
+
   if (this->type_ == NULL || this->type_->is_abstract())
     return;
 
@@ -2682,6 +2725,32 @@ 
   return new Const_expression(constant, location);
 }
 
+// Find a named object in an expression.
+
+int
+Find_named_object::expression(Expression** pexpr)
+{
+  switch ((*pexpr)->classification())
+    {
+    case Expression::EXPRESSION_CONST_REFERENCE:
+      if (static_cast<Const_expression*>(*pexpr)->named_object() == this->no_)
+	break;
+      return TRAVERSE_CONTINUE;
+    case Expression::EXPRESSION_VAR_REFERENCE:
+      if ((*pexpr)->var_expression()->named_object() == this->no_)
+	break;
+      return TRAVERSE_CONTINUE;
+    case Expression::EXPRESSION_FUNC_REFERENCE:
+      if ((*pexpr)->func_expression()->named_object() == this->no_)
+	break;
+      return TRAVERSE_CONTINUE;
+    default:
+      return TRAVERSE_CONTINUE;
+    }
+  this->found_ = true;
+  return TRAVERSE_EXIT;
+}
+
 // The nil value.
 
 class Nil_expression : public Expression
diff -r bce4b61f3652 go/gogo.cc
--- a/go/gogo.cc	Thu Dec 16 22:34:34 2010 -0800
+++ b/go/gogo.cc	Fri Dec 17 18:57:00 2010 -0800
@@ -3056,7 +3056,7 @@ 
   : type_(type), init_(init), preinit_(NULL), location_(location),
     is_global_(is_global), is_parameter_(is_parameter),
     is_receiver_(is_receiver), is_varargs_parameter_(false),
-    is_address_taken_(false), init_is_lowered_(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)
@@ -3090,7 +3090,18 @@ 
 {
   if (this->init_ != NULL && !this->init_is_lowered_)
     {
+      if (this->seen_)
+	{
+	  // We will give an error elsewhere, this is just to prevent
+	  // an infinite loop.
+	  return;
+	}
+      this->seen_ = true;
+
       gogo->lower_expression(function, &this->init_);
+
+      this->seen_ = false;
+
       this->init_is_lowered_ = true;
     }
 }
@@ -3209,7 +3220,7 @@ 
 // with type determination, then this should be unnecessary.
 
 Type*
-Variable::type() const
+Variable::type()
 {
   // A variable in a type switch with a nil case will have the wrong
   // type here.  This gets fixed up in determine_type, below.
@@ -3224,14 +3235,26 @@ 
       type = NULL;
     }
 
+  if (this->seen_)
+    {
+      if (this->type_ == NULL || !this->type_->is_error_type())
+	{
+	  error_at(this->location_, "variable initializer refers to itself");
+	  this->type_ = Type::make_error_type();
+	}
+      return this->type_;
+    }
+
+  this->seen_ = true;
+
   if (type != NULL)
-    return type;
+    ;
   else if (this->type_from_init_tuple_)
-    return this->type_from_tuple(init, false);
+    type = this->type_from_tuple(init, false);
   else if (this->type_from_range_index_ || this->type_from_range_value_)
-    return this->type_from_range(init, this->type_from_range_index_, false);
+    type = this->type_from_range(init, this->type_from_range_index_, false);
   else if (this->type_from_chan_element_)
-    return this->type_from_chan_element(init, false);
+    type = this->type_from_chan_element(init, false);
   else
     {
       gcc_assert(init != NULL);
@@ -3244,9 +3267,21 @@ 
 
       if (type->is_void_type())
 	type = Type::make_error_type();
-
-      return type;
     }
+
+  this->seen_ = false;
+
+  return type;
+}
+
+// Fetch the type from a const pointer, in which case it should have
+// been set already.
+
+Type*
+Variable::type() const
+{
+  gcc_assert(this->type_ != NULL);
+  return this->type_;
 }
 
 // Set the type if necessary.
diff -r bce4b61f3652 go/gogo.h
--- a/go/gogo.h	Thu Dec 16 22:34:34 2010 -0800
+++ b/go/gogo.h	Fri Dec 17 18:57:00 2010 -0800
@@ -1039,6 +1039,9 @@ 
 
   // Get the type of the variable.
   Type*
+  type();
+
+  Type*
   type() const;
 
   // Return whether the type is defined yet.
@@ -1258,6 +1261,8 @@ 
   bool is_varargs_parameter_ : 1;
   // Whether something takes the address of this variable.
   bool is_address_taken_ : 1;
+  // True if we have seen this variable in a traversal.
+  bool seen_ : 1;
   // True if we have lowered the initialization expression.
   bool init_is_lowered_ : 1;
   // True if init is a tuple used to set the type.