Patchwork [gccgo] More bounds checks

login
register
mail settings
Submitter Ian Taylor
Date Sept. 8, 2010, 11:26 p.m.
Message ID <mcrtylzg6l1.fsf@google.com>
Download mbox | patch
Permalink /patch/64240/
State New
Headers show

Comments

Ian Taylor - Sept. 8, 2010, 11:26 p.m.
This gccgo patch checks for overflow in an array or string index.  It
also checks for a negative capacity when making a map or channel.
Committed to gccgo branch.

Ian

Patch

diff -r 21cb4368a96a go/expressions.cc
--- a/go/expressions.cc	Wed Sep 08 14:22:32 2010 -0700
+++ b/go/expressions.cc	Wed Sep 08 16:21:47 2010 -0700
@@ -833,6 +833,50 @@ 
     gcc_unreachable();
 }
 
+// Return a tree which evaluates to true if VAL, of arbitrary integer
+// type, is negative or is more than the maximum value of BOUND_TYPE.
+// If SOFAR is not NULL, it is or'red into the result.  The return
+// value may be NULL if SOFAR is NULL.
+
+tree
+Expression::check_bounds(tree val, tree bound_type, tree sofar,
+			 source_location loc)
+{
+  tree val_type = TREE_TYPE(val);
+  tree ret = NULL_TREE;
+
+  if (!TYPE_UNSIGNED(val_type))
+    {
+      ret = fold_build2_loc(loc, LT_EXPR, boolean_type_node, val,
+			    build_int_cst(val_type, 0));
+      if (ret == boolean_false_node)
+	ret = NULL_TREE;
+    }
+
+  if ((TYPE_UNSIGNED(val_type) && !TYPE_UNSIGNED(bound_type))
+      || TYPE_SIZE(val_type) > TYPE_SIZE(bound_type))
+    {
+      tree max = TYPE_MAX_VALUE(bound_type);
+      tree big = fold_build2_loc(loc, GT_EXPR, boolean_type_node, val,
+				 fold_convert_loc(loc, val_type, max));
+      if (big == boolean_false_node)
+	;
+      else if (ret == NULL_TREE)
+	ret = big;
+      else
+	ret = fold_build2_loc(loc, TRUTH_OR_EXPR, boolean_type_node,
+			      ret, big);
+    }
+
+  if (ret == NULL_TREE)
+    return sofar;
+  else if (sofar == NULL_TREE)
+    return ret;
+  else
+    return fold_build2_loc(loc, TRUTH_OR_EXPR, boolean_type_node,
+			   sofar, ret);
+}
+
 // Error expressions.  This are used to avoid cascading errors.
 
 class Error_expression : public Expression
@@ -8723,7 +8767,7 @@ 
 {
   if (this->type_ == NULL)
     {
-      Array_type* type = this->array_->type()->array_type();
+     Array_type* type = this->array_->type()->array_type();
       if (type == NULL)
 	this->type_ = Type::make_error_type();
       else if (this->end_ == NULL)
@@ -8770,6 +8814,9 @@ 
   Array_type* array_type = this->array_->type()->array_type();
   gcc_assert(array_type != NULL);
 
+  unsigned int int_bits =
+    Type::lookup_integer_type("int")->integer_type()->bits();
+
   Type* dummy;
   mpz_t lval;
   mpz_init(lval);
@@ -8782,6 +8829,7 @@ 
   if (this->start_->integer_constant_value(true, ival, &dummy))
     {
       if (mpz_sgn(ival) < 0
+	  || mpz_sizeinbase(ival, 2) >= int_bits
 	  || (lval_valid
 	      && (this->end_ == NULL
 		  ? mpz_cmp(ival, lval) >= 0
@@ -8795,7 +8843,9 @@ 
     {
       if (this->end_->integer_constant_value(true, ival, &dummy))
 	{
-	  if (mpz_sgn(ival) < 0 || (lval_valid && mpz_cmp(ival, lval) > 0))
+	  if (mpz_sgn(ival) < 0
+	      || mpz_sizeinbase(ival, 2) >= int_bits
+	      || (lval_valid && mpz_cmp(ival, lval) > 0))
 	    {
 	      error_at(this->end_->location(), "array index out of bounds");
 	      this->set_is_error();
@@ -8848,19 +8898,26 @@ 
   if (array_tree == error_mark_node)
     return error_mark_node;
 
+  if (array_type->length() == NULL && !DECL_P(array_tree))
+    array_tree = save_expr(array_tree);
+  tree length_tree = array_type->length_tree(gogo, array_tree);
+  length_tree = save_expr(length_tree);
+  tree length_type = TREE_TYPE(length_tree);
+
   tree bad_index = boolean_false_node;
+
   tree start_tree = this->start_->get_tree(context);
   if (start_tree == error_mark_node)
     return error_mark_node;
-
-  if (array_type->length() == NULL && !DECL_P(array_tree))
-    array_tree = save_expr(array_tree);
   if (!DECL_P(start_tree))
     start_tree = save_expr(start_tree);
-  tree length_tree = array_type->length_tree(gogo, array_tree);
-  if (this->end_ != NULL && this->end_->is_nil_expression())
-    length_tree = save_expr(length_tree);
-  start_tree = fold_convert_loc(loc, TREE_TYPE(length_tree), start_tree);
+  if (!INTEGRAL_TYPE_P(TREE_TYPE(start_tree)))
+    start_tree = convert_to_integer(length_type, start_tree);
+
+  bad_index = Expression::check_bounds(start_tree, length_type, bad_index,
+				       loc);
+
+  start_tree = fold_convert_loc(loc, length_type, start_tree);
   bad_index = fold_build2_loc(loc, TRUTH_OR_EXPR, boolean_type_node, bad_index,
 			      fold_build2_loc(loc,
 					      (this->end_ == NULL
@@ -8911,8 +8968,7 @@ 
   // Array slice.
 
   tree capacity_tree = array_type->capacity_tree(gogo, array_tree);
-  capacity_tree = fold_convert_loc(loc, TREE_TYPE(length_tree),
-				   capacity_tree);
+  capacity_tree = fold_convert_loc(loc, length_type, capacity_tree);
 
   tree end_tree;
   if (this->end_->is_nil_expression())
@@ -8922,10 +8978,16 @@ 
       end_tree = this->end_->get_tree(context);
       if (end_tree == error_mark_node)
 	return error_mark_node;
-      end_tree = fold_convert_loc(loc, TREE_TYPE(length_tree), end_tree);
-
       if (!DECL_P(end_tree))
 	end_tree = save_expr(end_tree);
+      if (!INTEGRAL_TYPE_P(TREE_TYPE(end_tree)))
+	end_tree = convert_to_integer(length_type, end_tree);
+
+      bad_index = Expression::check_bounds(end_tree, length_type, bad_index,
+					   loc);
+
+      end_tree = fold_convert_loc(loc, length_type, end_tree);
+
       capacity_tree = save_expr(capacity_tree);
       tree bad_end = fold_build2_loc(loc, TRUTH_OR_EXPR, boolean_type_node,
 				     fold_build2_loc(loc, LT_EXPR,
@@ -8951,12 +9013,10 @@ 
 				  TREE_TYPE(value_pointer),
 				  value_pointer, offset);
 
-  tree result_length_tree = fold_build2_loc(loc, MINUS_EXPR,
-					    TREE_TYPE(length_tree),
+  tree result_length_tree = fold_build2_loc(loc, MINUS_EXPR, length_type,
 					    end_tree, start_tree);
 
-  tree result_capacity_tree = fold_build2_loc(loc, MINUS_EXPR,
-					      TREE_TYPE(length_tree),
+  tree result_capacity_tree = fold_build2_loc(loc, MINUS_EXPR, length_type,
 					      capacity_tree, start_tree);
 
   tree struct_tree = this->type()->get_tree(gogo);
@@ -9139,81 +9199,110 @@ 
 tree
 String_index_expression::do_get_tree(Translate_context* context)
 {
+  source_location loc = this->location();
+
   tree string_tree = this->string_->get_tree(context);
   if (string_tree == error_mark_node)
     return error_mark_node;
 
   if (this->string_->type()->points_to() != NULL)
     string_tree = build_fold_indirect_ref(string_tree);
+  if (!DECL_P(string_tree))
+    string_tree = save_expr(string_tree);
+  tree string_type = TREE_TYPE(string_tree);
+
+  tree length_tree = String_type::length_tree(context->gogo(), string_tree);
+  length_tree = save_expr(length_tree);
+  tree length_type = TREE_TYPE(length_tree);
+
+  tree bad_index = boolean_false_node;
 
   tree start_tree = this->start_->get_tree(context);
   if (start_tree == error_mark_node)
     return error_mark_node;
-  start_tree = fold_convert(integer_type_node, start_tree);
-
-  tree string_type = TREE_TYPE(string_tree);
+  if (!DECL_P(start_tree))
+    start_tree = save_expr(start_tree);
+  if (!INTEGRAL_TYPE_P(TREE_TYPE(start_tree)))
+    start_tree = convert_to_integer(length_type, start_tree);
+
+  bad_index = Expression::check_bounds(start_tree, length_type, bad_index,
+				       loc);
+
+  start_tree = fold_convert_loc(loc, length_type, start_tree);
+
+  // FIXME: Duplicates Array_index::do_get_tree.
+  static tree bad_index_fndecl;
+  tree crash = Gogo::call_builtin(&bad_index_fndecl,
+				  this->location(),
+				  "__go_bad_index",
+				  0,
+				  void_type_node);
+  TREE_NOTHROW(bad_index_fndecl) = 0;
+  TREE_THIS_VOLATILE(bad_index_fndecl) = 1;
 
   if (this->end_ == NULL)
     {
-      if (!DECL_P(string_tree))
-	string_tree = save_expr(string_tree);
-      if (!DECL_P(start_tree))
-	start_tree = save_expr(start_tree);
-
-      tree length_tree = String_type::length_tree(context->gogo(),
-						  string_tree);
-      tree bad_index = fold_build2(GE_EXPR, boolean_type_node,
-				   start_tree, length_tree);
-
-      // FIXME: Duplicates Array_index::do_get_tree.
-      static tree bad_index_fndecl;
-      tree crash = Gogo::call_builtin(&bad_index_fndecl,
-				      this->location(),
-				      "__go_bad_index",
-				      0,
-				      void_type_node);
-      TREE_NOTHROW(bad_index_fndecl) = 0;
-      TREE_THIS_VOLATILE(bad_index_fndecl) = 1;
+      bad_index = fold_build2_loc(loc, TRUTH_OR_EXPR, boolean_type_node,
+				  bad_index,
+				  fold_build2_loc(loc, GE_EXPR,
+						  boolean_type_node,
+						  start_tree, length_tree));
 
       tree bytes_tree = String_type::bytes_tree(context->gogo(), string_tree);
-      tree ptr = fold_build2(POINTER_PLUS_EXPR, TREE_TYPE(bytes_tree),
-			     bytes_tree,
-			     fold_convert(sizetype, start_tree));
-      tree index = build_fold_indirect_ref(ptr);
-
-      return fold_build2(COMPOUND_EXPR, TREE_TYPE(index),
-			 build3(COND_EXPR, void_type_node,
-				bad_index, crash, NULL_TREE),
-			 index);
+      tree ptr = fold_build2_loc(loc, POINTER_PLUS_EXPR, TREE_TYPE(bytes_tree),
+				 bytes_tree,
+				 fold_convert_loc(loc, sizetype, start_tree));
+      tree index = build_fold_indirect_ref_loc(loc, ptr);
+
+      return build2(COMPOUND_EXPR, TREE_TYPE(index),
+		    build3(COND_EXPR, void_type_node,
+			   bad_index, crash, NULL_TREE),
+		    index);
     }
   else
     {
       tree end_tree;
       if (this->end_->is_nil_expression())
-	end_tree = integer_minus_one_node;
+	end_tree = build_int_cst(length_type, -1);
       else
 	{
 	  end_tree = this->end_->get_tree(context);
 	  if (end_tree == error_mark_node)
 	    return error_mark_node;
-	}
-      end_tree = fold_convert(integer_type_node, end_tree);
+	  if (!DECL_P(end_tree))
+	    end_tree = save_expr(end_tree);
+	  if (!INTEGRAL_TYPE_P(TREE_TYPE(end_tree)))
+	    end_tree = convert_to_integer(length_type, end_tree);
+
+	  bad_index = Expression::check_bounds(end_tree, length_type,
+					       bad_index, loc);
+
+	  end_tree = fold_convert_loc(loc, length_type, end_tree);
+	}
+
       static tree strslice_fndecl;
       tree ret = Gogo::call_builtin(&strslice_fndecl,
-				    this->location(),
+				    loc,
 				    "__go_string_slice",
 				    3,
 				    string_type,
 				    string_type,
 				    string_tree,
-				    integer_type_node,
+				    length_type,
 				    start_tree,
-				    integer_type_node,
+				    length_type,
 				    end_tree);
       // This will panic if the bounds are out of range for the
       // string.
       TREE_NOTHROW(strslice_fndecl) = 0;
-      return ret;
+
+      if (bad_index == boolean_false_node)
+	return ret;
+      else
+	return build2(COMPOUND_EXPR, TREE_TYPE(ret),
+		      build3(COND_EXPR, void_type_node,
+			     bad_index, crash, NULL_TREE),
+		      ret);
     }
 }
 
diff -r 21cb4368a96a go/expressions.h
--- a/go/expressions.h	Wed Sep 08 14:22:32 2010 -0700
+++ b/go/expressions.h	Wed Sep 08 16:21:47 2010 -0700
@@ -589,6 +589,13 @@ 
   static Expression*
   import_expression(Import*);
 
+  // Return a tree which checks that VAL, of arbitrary integer type,
+  // is non-negative and is not more than the maximum value of
+  // BOUND_TYPE.  If SOFAR is not NULL, it is or'red into the result.
+  // The return value may be NULL if SOFAR is NULL.
+  static tree
+  check_bounds(tree val, tree bound_type, tree sofar, source_location);
+
  protected:
   // May be implemented by child class: traverse the expressions.
   virtual int
diff -r 21cb4368a96a go/types.cc
--- a/go/types.cc	Wed Sep 08 14:22:32 2010 -0700
+++ b/go/types.cc	Wed Sep 08 16:21:47 2010 -0700
@@ -3634,46 +3634,6 @@ 
     }
 }
 
-// Check that LEN, which has an arbitrary integer type, is
-// non-negative and not more than the maximum value of BOUND_TYPE.
-// Return a tree which performs this check.  The return value may be
-// NULL_TREE.  CHECK is a set of checks so far which are or'ed into
-// the result.
-
-tree
-Array_type::check_make_arg(tree len, tree check, tree bound_type,
-			   source_location location)
-{
-  tree len_type = TREE_TYPE(len);
-  tree ret = NULL_TREE;
-  if (!TYPE_UNSIGNED(len_type))
-    ret = fold_build2_loc(location, LT_EXPR, boolean_type_node,
-			  len,
-			  fold_convert_loc(location, len_type,
-					   integer_zero_node));
-  if ((TYPE_UNSIGNED(len_type) && !TYPE_UNSIGNED(bound_type))
-      || TYPE_SIZE(len_type) > TYPE_SIZE(bound_type))
-    {
-      tree max = TYPE_MAX_VALUE(bound_type);
-      tree big = fold_build2_loc(location, GT_EXPR, boolean_type_node,
-				 len,
-				 fold_convert_loc(location, len_type, max));
-      if (ret == NULL_TREE)
-	ret = big;
-      else
-	ret = fold_build2_loc(location, TRUTH_OR_EXPR, boolean_type_node,
-			      ret, big);
-    }
-
-  if (ret == NULL_TREE)
-    return check;
-  else if (check == NULL_TREE)
-    return ret;
-  else
-    return fold_build2_loc(location, TRUTH_OR_EXPR, boolean_type_node,
-			   ret, check);
-}
-
 // Handle the builtin make function for a slice.
 
 tree
@@ -3715,9 +3675,9 @@ 
   if (!INTEGRAL_TYPE_P(TREE_TYPE(length_tree)))
     length_tree = convert_to_integer(TREE_TYPE(count_field), length_tree);
 
-  tree bad_index = Array_type::check_make_arg(length_tree, NULL_TREE,
-					      TREE_TYPE(count_field),
-					      location);
+  tree bad_index = Expression::check_bounds(length_tree,
+					    TREE_TYPE(count_field),
+					    NULL_TREE, location);
 
   length_tree = fold_convert_loc(location, TREE_TYPE(count_field), length_tree);
   tree capacity_tree;
@@ -3734,9 +3694,9 @@ 
 	capacity_tree = convert_to_integer(TREE_TYPE(count_field),
 					   capacity_tree);
 
-      bad_index = Array_type::check_make_arg(capacity_tree, bad_index,
-					     TREE_TYPE(count_field),
-					     location);
+      bad_index = Expression::check_bounds(capacity_tree,
+					   TREE_TYPE(count_field),
+					   bad_index, location);
 
       tree chktype = (((TYPE_SIZE(TREE_TYPE(capacity_tree))
 			> TYPE_SIZE(TREE_TYPE(length_tree)))
@@ -4163,6 +4123,8 @@ 
 				  Expression_list* args,
 				  source_location location)
 {
+  tree bad_index = NULL_TREE;
+
   tree expr_tree;
   if (args == NULL || args->empty())
     expr_tree = size_zero_node;
@@ -4171,21 +4133,48 @@ 
       expr_tree = args->front()->get_tree(context);
       if (expr_tree == error_mark_node)
 	return error_mark_node;
-      expr_tree = ::convert(sizetype, expr_tree);
+      if (!DECL_P(expr_tree))
+	expr_tree = save_expr(expr_tree);
+      if (!INTEGRAL_TYPE_P(TREE_TYPE(expr_tree)))
+	expr_tree = convert_to_integer(sizetype, expr_tree);
+      bad_index = Expression::check_bounds(expr_tree, sizetype, bad_index,
+					   location);
     }
 
   tree map_type = this->get_tree(context->gogo());
 
   static tree new_map_fndecl;
-  return Gogo::call_builtin(&new_map_fndecl,
-			    location,
-			    "__go_new_map",
-			    2,
-			    map_type,
-			    TREE_TYPE(TYPE_FIELDS(TREE_TYPE(map_type))),
-			    context->gogo()->map_descriptor(this),
-			    sizetype,
-			    expr_tree);
+  tree ret = Gogo::call_builtin(&new_map_fndecl,
+				location,
+				"__go_new_map",
+				2,
+				map_type,
+				TREE_TYPE(TYPE_FIELDS(TREE_TYPE(map_type))),
+				context->gogo()->map_descriptor(this),
+				sizetype,
+				expr_tree);
+  // This can panic if the capacity is out of range.
+  TREE_NOTHROW(new_map_fndecl) = 0;
+
+  if (bad_index == NULL_TREE)
+    return ret;
+  else
+    {
+      // FIXME: Wrong message.
+      static tree bad_index_fndecl;
+      tree crash = Gogo::call_builtin(&bad_index_fndecl,
+				      location,
+				      "__go_bad_index",
+				      0,
+				      void_type_node);
+      TREE_NOTHROW(bad_index_fndecl) = 0;
+      TREE_THIS_VOLATILE(bad_index_fndecl) = 1;
+
+      return build2(COMPOUND_EXPR, TREE_TYPE(ret),
+		    build3(COND_EXPR, void_type_node,
+			   bad_index, crash, NULL_TREE),
+		    ret);
+    }
 }
 
 // Type descriptor.
@@ -4343,22 +4332,56 @@ 
   tree element_tree = this->element_type_->get_tree(gogo);
   tree element_size_tree = size_in_bytes(element_tree);
 
+  tree bad_index = NULL_TREE;
+
   tree expr_tree;
-  if (args != NULL && !args->empty())
-    expr_tree = ::convert(sizetype, args->front()->get_tree(context));
+  if (args == NULL || args->empty())
+    expr_tree = size_zero_node;
   else
-    expr_tree = size_zero_node;
+    {
+      expr_tree = args->front()->get_tree(context);
+      if (expr_tree == error_mark_node)
+	return error_mark_node;
+      if (!DECL_P(expr_tree))
+	expr_tree = save_expr(expr_tree);
+      if (!INTEGRAL_TYPE_P(TREE_TYPE(expr_tree)))
+	expr_tree = convert_to_integer(sizetype, expr_tree);
+      bad_index = Expression::check_bounds(expr_tree, sizetype, bad_index,
+					   location);
+    }
 
   static tree new_channel_fndecl;
-  return Gogo::call_builtin(&new_channel_fndecl,
-			    location,
-			    "__go_new_channel",
-			    2,
-			    channel_type,
-			    sizetype,
-			    element_size_tree,
-			    sizetype,
-			    expr_tree);
+  tree ret = Gogo::call_builtin(&new_channel_fndecl,
+				location,
+				"__go_new_channel",
+				2,
+				channel_type,
+				sizetype,
+				element_size_tree,
+				sizetype,
+				expr_tree);
+  // This can panic if the capacity is out of range.
+  TREE_NOTHROW(new_channel_fndecl) = 0;
+
+  if (bad_index == NULL_TREE)
+    return ret;
+  else
+    {
+      // FIXME: Wrong message.
+      static tree bad_index_fndecl;
+      tree crash = Gogo::call_builtin(&bad_index_fndecl,
+				      location,
+				      "__go_bad_index",
+				      0,
+				      void_type_node);
+      TREE_NOTHROW(bad_index_fndecl) = 0;
+      TREE_THIS_VOLATILE(bad_index_fndecl) = 1;
+
+      return build2(COMPOUND_EXPR, TREE_TYPE(ret),
+		    build3(COND_EXPR, void_type_node,
+			   bad_index, crash, NULL_TREE),
+		    ret);
+    }
 }
 
 // Type descriptor.
diff -r 21cb4368a96a go/types.h
--- a/go/types.h	Wed Sep 08 14:22:32 2010 -0700
+++ b/go/types.h	Wed Sep 08 16:21:47 2010 -0700
@@ -1997,9 +1997,6 @@ 
   tree
   get_length_tree(Gogo*);
 
-  static tree
-  check_make_arg(tree, tree, tree, source_location);
-
   // A mapping from Type to tree, used to ensure that arrays of
   // identical types are identical.
   typedef std::tr1::unordered_map<const Type*, tree, Type_hash_identical,