Patchwork Go patch committed: Better checks for make arguments

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

Comments

Ian Taylor - Dec. 5, 2012, 12:23 a.m.
This patch to the Go frontend implements better checks for constants
passed to the predefined function make.  It gives an error for using
negative numbers, and when the length is larger than the capacity.
These are required by the language spec, but were never enforced by the
compiler until now.  This required updating one test case.  Bootstrapped
and ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to
mainline.

Ian

Patch

Index: gcc/testsuite/go.test/test/fixedbugs/bug273.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug273.go	(revision 194172)
+++ gcc/testsuite/go.test/test/fixedbugs/bug273.go	(revision 194173)
@@ -11,6 +11,7 @@  package main
 var bug = false
 
 var minus1 = -1
+var five = 5
 var big int64 = 10 | 1<<32
 
 type block [1<<19]byte
@@ -40,7 +41,7 @@  func badcap() {
 }
 
 func badcap1() {
-	g1 = make([]block, 10, 5)
+	g1 = make([]block, 10, five)
 }
 
 func bigcap() {
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 194172)
+++ gcc/go/gofrontend/expressions.cc	(revision 194173)
@@ -6623,7 +6623,7 @@  class Builtin_call_expression : public C
   lower_make();
 
   bool
-  check_int_value(Expression*);
+  check_int_value(Expression*, bool is_length);
 
   // A pointer back to the general IR structure.  This avoids a global
   // variable, or passing it around everywhere.
@@ -6897,11 +6897,8 @@  Builtin_call_expression::lower_make()
   else
     {
       len_arg = *parg;
-      if (!this->check_int_value(len_arg))
-	{
-	  this->report_error(_("bad size for make"));
-	  return Expression::make_error(this->location());
-	}
+      if (!this->check_int_value(len_arg, true))
+	return Expression::make_error(this->location());
       if (len_arg->type()->integer_type() != NULL
 	  && len_arg->type()->integer_type()->bits() > uintptr_bits)
 	have_big_args = true;
@@ -6912,11 +6909,23 @@  Builtin_call_expression::lower_make()
   if (is_slice && parg != args->end())
     {
       cap_arg = *parg;
-      if (!this->check_int_value(cap_arg))
+      if (!this->check_int_value(cap_arg, false))
+	return Expression::make_error(this->location());
+
+      Numeric_constant nclen;
+      Numeric_constant nccap;
+      unsigned long vlen;
+      unsigned long vcap;
+      if (len_arg->numeric_constant_value(&nclen)
+	  && cap_arg->numeric_constant_value(&nccap)
+	  && nclen.to_unsigned_long(&vlen) == Numeric_constant::NC_UL_VALID
+	  && nccap.to_unsigned_long(&vcap) == Numeric_constant::NC_UL_VALID
+	  && vlen > vcap)
 	{
-	  this->report_error(_("bad capacity when making slice"));
+	  this->report_error(_("len larger than cap"));
 	  return Expression::make_error(this->location());
 	}
+
       if (cap_arg->type()->integer_type() != NULL
 	  && cap_arg->type()->integer_type()->bits() > uintptr_bits)
 	have_big_args = true;
@@ -6973,20 +6982,36 @@  Builtin_call_expression::lower_make()
 // function.
 
 bool
-Builtin_call_expression::check_int_value(Expression* e)
+Builtin_call_expression::check_int_value(Expression* e, bool is_length)
 {
-  if (e->type()->integer_type() != NULL)
-    return true;
-
-  // Check for a floating point constant with integer value.
   Numeric_constant nc;
-  mpz_t ival;
-  if (e->numeric_constant_value(&nc) && nc.to_int(&ival))
+  if (e->numeric_constant_value(&nc))
     {
-      mpz_clear(ival);
-      return true;
+      unsigned long v;
+      switch (nc.to_unsigned_long(&v))
+	{
+	case Numeric_constant::NC_UL_VALID:
+	  return true;
+	case Numeric_constant::NC_UL_NOTINT:
+	  error_at(e->location(), "non-integer %s argument to make",
+		   is_length ? "len" : "cap");
+	  return false;
+	case Numeric_constant::NC_UL_NEGATIVE:
+	  error_at(e->location(), "negative %s argument to make",
+		   is_length ? "len" : "cap");
+	  return false;
+	case Numeric_constant::NC_UL_BIG:
+	  // We don't want to give a compile-time error for a 64-bit
+	  // value on a 32-bit target.
+	  return true;
+	}
     }
 
+  if (e->type()->integer_type() != NULL)
+    return true;
+
+  error_at(e->location(), "non-integer %s argument to make",
+	   is_length ? "len" : "cap");
   return false;
 }