From 26e7e93478707ec3caa53608303d099d21de3dde Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sun, 27 Jan 2019 14:27:08 +0100 Subject: [PATCH] compiler: make sure make([]T, ...) checks for Ts bigger than 1 Without this, the following code would not panic: func getInt(i int) { return i } make([][1<<18], getInt(1<<18)) Or this code would be allowed to compile for 32-bit systems: make([][1<<18], 1<<18) --- compiler/compiler.go | 26 ++++++++++++++++++++++---- src/runtime/panic.go | 14 ++++++++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/compiler/compiler.go b/compiler/compiler.go index 625dcc77..4bdd9ffd 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -1481,9 +1481,16 @@ func (c *Compiler) parseExpr(frame *Frame, expr ssa.Value) (llvm.Value, error) { } var buf llvm.Value if expr.Heap { + size := c.targetData.TypeAllocSize(typ) + // Calculate ^uintptr(0) + maxSize := llvm.ConstNot(llvm.ConstInt(c.uintptrType, 0, false)).ZExtValue() + if size > maxSize { + // Size would be truncated if truncated to uintptr. + return llvm.Value{}, c.makeError(expr.Pos(), fmt.Sprintf("value is too big (%v bytes)", size)) + } // TODO: escape analysis - size := llvm.ConstInt(c.uintptrType, c.targetData.TypeAllocSize(typ), false) - buf = c.createRuntimeCall("alloc", []llvm.Value{size}, expr.Comment) + sizeValue := llvm.ConstInt(c.uintptrType, size, false) + buf = c.createRuntimeCall("alloc", []llvm.Value{sizeValue}, expr.Comment) buf = c.builder.CreateBitCast(buf, llvm.PointerType(typ, 0), "") } else { buf = c.builder.CreateAlloca(typ, expr.Comment) @@ -1727,6 +1734,15 @@ func (c *Compiler) parseExpr(frame *Frame, expr ssa.Value) (llvm.Value, error) { return llvm.Value{}, nil } elemSize := c.targetData.TypeAllocSize(llvmElemType) + elemSizeValue := llvm.ConstInt(c.uintptrType, elemSize, false) + + // Calculate ^uintptr(0) + maxSize := llvm.ConstNot(llvm.ConstInt(c.uintptrType, 0, false)).ZExtValue() + if elemSize > maxSize { + // This seems to be checked by the typechecker already, but let's + // check it again just to be sure. + return llvm.Value{}, c.makeError(expr.Pos(), fmt.Sprintf("slice element type is too big (%v bytes)", elemSize)) + } // Bounds checking. if !frame.fn.IsNoBounds() { @@ -1754,12 +1770,14 @@ func (c *Compiler) parseExpr(frame *Frame, expr ssa.Value) (llvm.Value, error) { sliceCap = c.builder.CreateSExt(sliceCap, biggestInt, "") } } - c.createRuntimeCall(checkFunc, []llvm.Value{sliceLen, sliceCap}, "") + // Note: the max element size needs to be doubled to make sure it + // fits in an int for, for example, len(). + elemSizeDoubled := c.builder.CreateMul(elemSizeValue, llvm.ConstInt(c.uintptrType, 2, false), "") + c.createRuntimeCall(checkFunc, []llvm.Value{sliceLen, sliceCap, elemSizeDoubled}, "") } // Allocate the backing array. // TODO: escape analysis - elemSizeValue := llvm.ConstInt(c.uintptrType, elemSize, false) sliceCapCast, err := c.parseConvert(expr.Cap.Type(), types.Typ[types.Uintptr], sliceCap, expr.Pos()) if err != nil { return llvm.Value{}, err diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 9140fcd0..a716247c 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -57,15 +57,21 @@ func sliceBoundsCheck64(capacity uintptr, low, high uint64) { } // Check for bounds in *ssa.MakeSlice. -func sliceBoundsCheckMake(length, capacity uint) { - if !(0 <= length && length <= capacity) { +func sliceBoundsCheckMake(length, capacity uintptr, elementSizeDoubled uintptr) { + overflow := uint64(capacity*elementSizeDoubled) != uint64(capacity)*uint64(elementSizeDoubled) + if length > capacity || overflow { runtimePanic("slice size out of range") } } // Check for bounds in *ssa.MakeSlice. Supports 64-bit indexes. -func sliceBoundsCheckMake64(length, capacity uint64) { - if !(0 <= length && length <= capacity) { +func sliceBoundsCheckMake64(length, capacity uint64, elementSizeDoubled uintptr) { + // This function is only ever called on systems where uintptr is smaller + // than uint64 (thus must be 32-bit or less). So multiplying as uint64 will + // never overflow if we know that capacity fits in uintptr. + // That elementSizeDoubled fits in uintptr is checked by the compiler. + overflow := capacity != uint64(uintptr(capacity)) || capacity != uint64(uintptr(capacity*uint64(elementSizeDoubled))) + if length > capacity || overflow { runtimePanic("slice size out of range") } }