diff mbox

GCC 6 RFA: Go patch: call determine_types even for constant expressions

Message ID CAOyqgcVORxK4XbbZb8t8vNe74eZRmUJVivY=KGJiMZMyVTSORA@mail.gmail.com
State New
Headers show

Commit Message

Ian Lance Taylor Dec. 16, 2016, 12:25 a.m. UTC 
On Thu, Dec 15, 2016 at 2:47 PM, Ian Lance Taylor <iant@golang.org> wrote:
> The Go frontend needs to call determine_types even for constant
> expressions, which it was not doing.  The problem is that a constant
> expression may include code like unsafe.Sizeof(0).  Something needs to
> determine the type of the untyped 0, and that should be the
> determine_types pass.
>
> Implementing that triggered a compiler crash on test/const1.go because
> it permitted some erroneous constants to make it all the way to the
> backend.  Catch that case by checking whether we get a constant
> overflow error, and marking the expression invalid if we do.  This is
> a good change in any case, as previously we reported the same constant
> overflow error multiple times, and now we only report it once.
>
> This fixes GCC PR 78763.
>
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.


In order to fix GCC PR 78763, I'd like to commit this patch to the GCC
6 branch.  I've bootstrapped the test on x86_64-pc-linux-gnu, and
verified that the Go tests continue to pass.

OK for GCC 6 branch?

Ian

Comments

Ian Lance Taylor Jan. 24, 2017, 3:34 a.m. UTC | #1 
On Thu, Dec 15, 2016 at 4:25 PM, Ian Lance Taylor <iant@golang.org> wrote:
> On Thu, Dec 15, 2016 at 2:47 PM, Ian Lance Taylor <iant@golang.org> wrote:
>> The Go frontend needs to call determine_types even for constant
>> expressions, which it was not doing.  The problem is that a constant
>> expression may include code like unsafe.Sizeof(0).  Something needs to
>> determine the type of the untyped 0, and that should be the
>> determine_types pass.
>>
>> Implementing that triggered a compiler crash on test/const1.go because
>> it permitted some erroneous constants to make it all the way to the
>> backend.  Catch that case by checking whether we get a constant
>> overflow error, and marking the expression invalid if we do.  This is
>> a good change in any case, as previously we reported the same constant
>> overflow error multiple times, and now we only report it once.
>>
>> This fixes GCC PR 78763.
>>
>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>> to mainline.
>
>
> In order to fix GCC PR 78763, I'd like to commit this patch to the GCC
> 6 branch.  I've bootstrapped the test on x86_64-pc-linux-gnu, and
> verified that the Go tests continue to pass.

Now committed to GCC 6 branch.

Ian
diff mbox

Patch

Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 243728)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -3639,8 +3639,12 @@ 
       if (expr->numeric_constant_value(&nc))
 	{
 	  Numeric_constant result;
-	  if (Unary_expression::eval_constant(op, &nc, loc, &result))
+	  bool issued_error;
+	  if (Unary_expression::eval_constant(op, &nc, loc, &result,
+					      &issued_error))
 	    return result.expression(loc);
+	  else if (issued_error)
+	    return Expression::make_error(this->location());
 	}
     }
 
@@ -3747,12 +3751,15 @@ 
 }
 
 // Apply unary opcode OP to UNC, setting NC.  Return true if this
-// could be done, false if not.  Issue errors for overflow.
+// could be done, false if not.  On overflow, issues an error and sets
+// *ISSUED_ERROR.
 
 bool
 Unary_expression::eval_constant(Operator op, const Numeric_constant* unc,
-				Location location, Numeric_constant* nc)
+				Location location, Numeric_constant* nc,
+				bool* issued_error)
 {
+  *issued_error = false;
   switch (op)
     {
     case OPERATOR_PLUS:
@@ -3897,7 +3904,12 @@ 
   mpz_clear(uval);
   mpz_clear(val);
 
-  return nc->set_type(unc->type(), true, location);
+  if (!nc->set_type(unc->type(), true, location))
+    {
+      *issued_error = true;
+      return false;
+    }
+  return true;
 }
 
 // Return the integral constant value of a unary expression, if it has one.
@@ -3908,8 +3920,9 @@ 
   Numeric_constant unc;
   if (!this->expr_->numeric_constant_value(&unc))
     return false;
+  bool issued_error;
   return Unary_expression::eval_constant(this->op_, &unc, this->location(),
-					 nc);
+					 nc, &issued_error);
 }
 
 // Return the type of a unary expression.
@@ -4539,13 +4552,15 @@ 
 
 // Apply binary opcode OP to LEFT_NC and RIGHT_NC, setting NC.  Return
 // true if this could be done, false if not.  Issue errors at LOCATION
-// as appropriate.
+// as appropriate, and sets *ISSUED_ERROR if it did.
 
 bool
 Binary_expression::eval_constant(Operator op, Numeric_constant* left_nc,
 				 Numeric_constant* right_nc,
-				 Location location, Numeric_constant* nc)
+				 Location location, Numeric_constant* nc,
+				 bool* issued_error)
 {
+  *issued_error = false;
   switch (op)
     {
     case OPERATOR_OROR:
@@ -4594,7 +4609,11 @@ 
     r = Binary_expression::eval_integer(op, left_nc, right_nc, location, nc);
 
   if (r)
-    r = nc->set_type(type, true, location);
+    {
+      r = nc->set_type(type, true, location);
+      if (!r)
+	*issued_error = true;
+    }
 
   return r;
 }
@@ -4917,9 +4936,15 @@ 
 	else
 	  {
 	    Numeric_constant nc;
+	    bool issued_error;
 	    if (!Binary_expression::eval_constant(op, &left_nc, &right_nc,
-						  location, &nc))
+						  location, &nc,
+						  &issued_error))
+	      {
+		if (issued_error)
+		  return Expression::make_error(location);
                 return this;
+	      }
 	    return nc.expression(location);
 	  }
       }
@@ -5254,8 +5279,9 @@ 
   Numeric_constant right_nc;
   if (!this->right_->numeric_constant_value(&right_nc))
     return false;
+  bool issued_error;
   return Binary_expression::eval_constant(this->op_, &left_nc, &right_nc,
-					  this->location(), nc);
+					  this->location(), nc, &issued_error);
 }
 
 // Note that the value is being discarded.
@@ -5354,8 +5380,13 @@ 
 
   Type_context subcontext(*context);
 
-  if (is_comparison)
+  if (is_constant_expr)
     {
+      subcontext.type = NULL;
+      subcontext.may_be_abstract = true;
+    }
+  else if (is_comparison)
+    {
       // In a comparison, the context does not determine the types of
       // the operands.
       subcontext.type = NULL;
@@ -5396,8 +5427,7 @@ 
 	subcontext.type = subcontext.type->make_non_abstract_type();
     }
 
-  if (!is_constant_expr)
-    this->left_->determine_type(&subcontext);
+  this->left_->determine_type(&subcontext);
 
   if (is_shift_op)
     {
@@ -5417,8 +5447,7 @@ 
       subcontext.may_be_abstract = false;
     }
 
-  if (!is_constant_expr)
-    this->right_->determine_type(&subcontext);
+  this->right_->determine_type(&subcontext);
 
   if (is_comparison)
     {
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 243728)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -1633,10 +1633,11 @@ 
   }
 
   // Apply unary opcode OP to UNC, setting NC.  Return true if this
-  // could be done, false if not.  Issue errors for overflow.
+  // could be done, false if not.  On overflow, issues an error and
+  // sets *ISSUED_ERROR.
   static bool
   eval_constant(Operator op, const Numeric_constant* unc,
-		Location, Numeric_constant* nc);
+		Location, Numeric_constant* nc, bool *issued_error);
 
   static Expression*
   do_import(Import*);
@@ -1755,11 +1756,11 @@ 
 
   // Apply binary opcode OP to LEFT_NC and RIGHT_NC, setting NC.
   // Return true if this could be done, false if not.  Issue errors at
-  // LOCATION as appropriate.
+  // LOCATION as appropriate, and sets *ISSUED_ERROR if it did.
   static bool
   eval_constant(Operator op, Numeric_constant* left_nc,
 		Numeric_constant* right_nc, Location location,
-		Numeric_constant* nc);
+		Numeric_constant* nc, bool* issued_error);
 
   // Compare constants LEFT_NC and RIGHT_NC according to OP, setting
   // *RESULT.  Return true if this could be done, false if not.  Issue