From 79dae62c781bc3fe6df76d5a609f62fcc7a43310 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Tue, 10 Mar 2020 21:58:34 +0100 Subject: [PATCH] compiler,runtime: check for channel size limits This patch is a combination of two related changes: 1. The compiler now allows other types than `int` when specifying the size of a channel in a make(chan ..., size) call. 2. The compiler now checks for maximum allowed channel sizes. Such checks are trivially optimized out in the vast majority of cases as channel sizes are usually constant. I discovered this issue when trying out channels on AVR. --- compiler/asserts.go | 73 ++++++++++++++++++++++++++++++++++++++++++++ compiler/channel.go | 6 ++++ src/runtime/panic.go | 5 +++ testdata/channel.go | 10 ++++++ 4 files changed, 94 insertions(+) diff --git a/compiler/asserts.go b/compiler/asserts.go index ad79c0a1..ab519842 100644 --- a/compiler/asserts.go +++ b/compiler/asserts.go @@ -4,6 +4,8 @@ package compiler // required by the Go programming language. import ( + "fmt" + "go/token" "go/types" "tinygo.org/x/go-llvm" @@ -122,6 +124,77 @@ func (c *Compiler) emitSliceBoundsCheck(frame *Frame, capacity, low, high, max l c.builder.SetInsertPointAtEnd(nextBlock) } +// emitChanBoundsCheck emits a bounds check before creating a new channel to +// check that the value is not too big for runtime.chanMake. +func (c *Compiler) emitChanBoundsCheck(frame *Frame, elementSize uint64, bufSize llvm.Value, bufSizeType *types.Basic, pos token.Pos) { + if frame.fn.IsNoBounds() { + // The //go:nobounds pragma was added to the function to avoid bounds + // checking. + return + } + + // Check whether the bufSize parameter must be cast to a wider integer for + // comparison. + if bufSize.Type().IntTypeWidth() < c.uintptrType.IntTypeWidth() { + if bufSizeType.Info()&types.IsUnsigned != 0 { + // Unsigned, so zero-extend to uint type. + bufSizeType = types.Typ[types.Uint] + bufSize = c.builder.CreateZExt(bufSize, c.intType, "") + } else { + // Signed, so sign-extend to int type. + bufSizeType = types.Typ[types.Int] + bufSize = c.builder.CreateSExt(bufSize, c.intType, "") + } + } + + // Calculate (^uintptr(0)) >> 1, which is the max value that fits in an + // uintptr if uintptrs were signed. + maxBufSize := llvm.ConstLShr(llvm.ConstNot(llvm.ConstInt(c.uintptrType, 0, false)), llvm.ConstInt(c.uintptrType, 1, false)) + if elementSize > maxBufSize.ZExtValue() { + c.addError(pos, fmt.Sprintf("channel element type is too big (%v bytes)", elementSize)) + return + } + // Avoid divide-by-zero. + if elementSize == 0 { + elementSize = 1 + } + // Make the maxBufSize actually the maximum allowed value (in number of + // elements in the channel buffer). + maxBufSize = llvm.ConstUDiv(maxBufSize, llvm.ConstInt(c.uintptrType, elementSize, false)) + + // Make sure maxBufSize has the same type as bufSize. + if maxBufSize.Type() != bufSize.Type() { + maxBufSize = llvm.ConstZExt(maxBufSize, bufSize.Type()) + } + + bufSizeTooBig := c.builder.CreateICmp(llvm.IntUGE, bufSize, maxBufSize, "") + // Check whether we can resolve this check at compile time. + if !bufSizeTooBig.IsAConstantInt().IsNil() { + val := bufSizeTooBig.ZExtValue() + if val == 0 { + // Everything is constant so the check does not have to be emitted + // in IR. This avoids emitting some redundant IR in the vast + // majority of cases. + return + } + } + + faultBlock := c.ctx.AddBasicBlock(frame.fn.LLVMFn, "chan.outofbounds") + nextBlock := c.ctx.AddBasicBlock(frame.fn.LLVMFn, "chan.next") + frame.blockExits[frame.currentBlock] = nextBlock // adjust outgoing block for phi nodes + + // Now branch to the out-of-bounds or the regular block. + c.builder.CreateCondBr(bufSizeTooBig, faultBlock, nextBlock) + + // Fail: this channel is created with an invalid size parameter. + c.builder.SetInsertPointAtEnd(faultBlock) + c.createRuntimeCall("chanMakePanic", nil, "") + c.builder.CreateUnreachable() + + // Ok: this channel value is not too big. + c.builder.SetInsertPointAtEnd(nextBlock) +} + // emitNilCheck checks whether the given pointer is nil, and panics if it is. It // has no effect in well-behaved programs, but makes sure no uncaught nil // pointer dereferences exist in valid Go code. diff --git a/compiler/channel.go b/compiler/channel.go index ac16510c..de7b12e2 100644 --- a/compiler/channel.go +++ b/compiler/channel.go @@ -15,6 +15,12 @@ func (c *Compiler) emitMakeChan(frame *Frame, expr *ssa.MakeChan) llvm.Value { elementSize := c.targetData.TypeAllocSize(c.getLLVMType(expr.Type().(*types.Chan).Elem())) elementSizeValue := llvm.ConstInt(c.uintptrType, elementSize, false) bufSize := c.getValue(frame, expr.Size) + c.emitChanBoundsCheck(frame, elementSize, bufSize, expr.Size.Type().Underlying().(*types.Basic), expr.Pos()) + if bufSize.Type().IntTypeWidth() < c.uintptrType.IntTypeWidth() { + bufSize = c.builder.CreateZExt(bufSize, c.uintptrType, "") + } else if bufSize.Type().IntTypeWidth() > c.uintptrType.IntTypeWidth() { + bufSize = c.builder.CreateTrunc(bufSize, c.uintptrType, "") + } return c.createRuntimeCall("chanMake", []llvm.Value{elementSizeValue, bufSize}, "") } diff --git a/src/runtime/panic.go b/src/runtime/panic.go index e8aafa74..8f8d7366 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -50,6 +50,11 @@ func slicePanic() { runtimePanic("slice out of range") } +// Panic when trying to create a new channel that is too big. +func chanMakePanic() { + runtimePanic("new channel is too big") +} + func blockingPanic() { runtimePanic("trying to do blocking operation in exported function") } diff --git a/testdata/channel.go b/testdata/channel.go index a504b690..db5a8631 100644 --- a/testdata/channel.go +++ b/testdata/channel.go @@ -54,6 +54,16 @@ func main() { n, ok = <-ch println("recv from closed channel:", n, ok) + // Test various channel size types. + _ = make(chan int, int8(2)) + _ = make(chan int, int16(2)) + _ = make(chan int, int32(2)) + _ = make(chan int, int64(2)) + _ = make(chan int, uint8(2)) + _ = make(chan int, uint16(2)) + _ = make(chan int, uint32(2)) + _ = make(chan int, uint64(2)) + // Test bigger values ch2 := make(chan complex128) wg.add(1)