diff mbox series

Go patch committed: Check len and cap for append(s, make(T, I)...)

Message ID CAOyqgcUNs2KOaXMq+OsFLGqd2_2jp4LwPoAgS1ocWazutFhF3Q@mail.gmail.com
State New
Headers show
Series Go patch committed: Check len and cap for append(s, make(T, I)...) | expand

Commit Message

Ian Lance Taylor Nov. 30, 2020, 8:22 p.m. UTC
This patch to the Go frontend and runtime checks both len and cap when
handling the case of append(s, make(T, I)...).  The overflow checks
done in growslice always reported an error for the capacity argument,
even if it was the length argument that overflowed.  This change lets
the code pass the current issue4085b.go test.  Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
9ebad4b01c22d4c03a3552fd6b0e86c9de0ce6bd
diff mbox series

Patch

diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index e8cf468d8fc..eec3d0708e5 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@ 
-be1738f1fff0e817d921ed568791f9b0585a6982
+84506e0f6bf282765856cb5aeb17124222f73042
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index d1546300d0e..23caf61db93 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -8893,8 +8893,8 @@  Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function,
           // We will optimize this to directly zeroing the tail,
           // instead of allocating a new slice then copy.
 
-          // Retrieve the length. Cannot reference s2 as we will remove
-          // the makeslice call.
+          // Retrieve the length and capacity. Cannot reference s2 as
+          // we will remove the makeslice call.
           Expression* len_arg = makecall->args()->at(1);
           len_arg = Expression::make_cast(int_type, len_arg, loc);
           l2tmp = Statement::make_temporary(int_type, len_arg, loc);
@@ -8907,28 +8907,19 @@  Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function,
           inserter->insert(c2tmp);
 
           // Check bad len/cap here.
-          // if len2 < 0 { panicmakeslicelen(); }
+	  // checkmakeslice(type, len, cap)
+	  // (Note that if len and cap are constants, we won't see a
+	  // makeslice call here, as it will be rewritten to a stack
+	  // allocated array by Mark_address_taken::expression.)
+	  Expression* elem = Expression::make_type_descriptor(element_type,
+							      loc);
           len2 = Expression::make_temporary_reference(l2tmp, loc);
-          Expression* zero = Expression::make_integer_ul(0, int_type, loc);
-          Expression* cond = Expression::make_binary(OPERATOR_LT, len2,
-                                                     zero, loc);
-	  Expression* call = Runtime::make_call(Runtime::PANIC_MAKE_SLICE_LEN,
-						loc, 0);
-          cond = Expression::make_conditional(cond, call, zero->copy(), loc);
-          gogo->lower_expression(function, inserter, &cond);
-          gogo->flatten_expression(function, inserter, &cond);
-          Statement* s = Statement::make_statement(cond, false);
-          inserter->insert(s);
-
-          // if cap2 < 0 { panicmakeslicecap(); }
           Expression* cap2 = Expression::make_temporary_reference(c2tmp, loc);
-          cond = Expression::make_binary(OPERATOR_LT, cap2,
-                                         zero->copy(), loc);
-	  call = Runtime::make_call(Runtime::PANIC_MAKE_SLICE_CAP, loc, 0);
-          cond = Expression::make_conditional(cond, call, zero->copy(), loc);
-          gogo->lower_expression(function, inserter, &cond);
-          gogo->flatten_expression(function, inserter, &cond);
-          s = Statement::make_statement(cond, false);
+	  Expression* check = Runtime::make_call(Runtime::CHECK_MAKE_SLICE,
+						 loc, 3, elem, len2, cap2);
+          gogo->lower_expression(function, inserter, &check);
+          gogo->flatten_expression(function, inserter, &check);
+          Statement* s = Statement::make_statement(check, false);
           inserter->insert(s);
 
           // Remove the original makeslice call.
diff --git a/gcc/go/gofrontend/runtime.def b/gcc/go/gofrontend/runtime.def
index b608766cdaf..9a3c6809130 100644
--- a/gcc/go/gofrontend/runtime.def
+++ b/gcc/go/gofrontend/runtime.def
@@ -265,6 +265,11 @@  DEF_GO_RUNTIME(GROWSLICE, "runtime.growslice",
                P5(TYPE, POINTER, INT, INT, INT), R1(SLICE))
 
 
+// Check the length and cap passed to make, without making a slice.
+// This is used for apend(s, make([]T, len)...).
+DEF_GO_RUNTIME(CHECK_MAKE_SLICE, "runtime.checkMakeSlice", P3(TYPE, INT, INT),
+	       R1(UINTPTR))
+
 // Register roots (global variables) for the garbage collector.
 DEF_GO_RUNTIME(REGISTER_GC_ROOTS, "runtime.registerGCRoots", P1(POINTER), R0())
 
diff --git a/libgo/go/runtime/slice.go b/libgo/go/runtime/slice.go
index 97b26594dad..ddd588ed5b9 100644
--- a/libgo/go/runtime/slice.go
+++ b/libgo/go/runtime/slice.go
@@ -15,6 +15,7 @@  import (
 //go:linkname panicmakeslicelen
 //go:linkname panicmakeslicecap
 //go:linkname makeslice
+//go:linkname checkMakeSlice
 //go:linkname makeslice64
 //go:linkname growslice
 //go:linkname slicecopy
@@ -91,6 +92,13 @@  func makeslicecopy(et *_type, tolen int, fromlen int, from unsafe.Pointer) unsaf
 }
 
 func makeslice(et *_type, len, cap int) unsafe.Pointer {
+	mem := checkMakeSlice(et, len, cap)
+	return mallocgc(mem, et, true)
+}
+
+// checkMakeSlice is called for append(s, make([]T, len, cap)...) to check
+// the values of len and cap.
+func checkMakeSlice(et *_type, len, cap int) uintptr {
 	mem, overflow := math.MulUintptr(et.size, uintptr(cap))
 	if overflow || mem > maxAlloc || len < 0 || len > cap {
 		// NOTE: Produce a 'len out of range' error instead of a
@@ -104,8 +112,7 @@  func makeslice(et *_type, len, cap int) unsafe.Pointer {
 		}
 		panicmakeslicecap()
 	}
-
-	return mallocgc(mem, et, true)
+	return mem
 }
 
 func makeslice64(et *_type, len64, cap64 int64) unsafe.Pointer {