From 622d0ebde671a4c71fc771ef8f9210ff433c9fbe Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Thu, 7 Mar 2019 13:33:46 +0100 Subject: [PATCH] compiler: implement nil checks This commit implements nil checks for all platforms. These nil checks can be optimized on systems with a MMU, but since a major target is systems without MMU, keep it this way for now. It implements three checks: * Nil checks before dereferencing a pointer. * Nil checks before calculating an address (*ssa.FieldAddr and *ssa.IndexAddr) * Nil checks before calling a function pointer. The first check has by far the biggest impact, with around 5% increase in code size. The other checks only trigger in only some test cases and have a minimal impact on code size. This first nil check is also the one that is easiest to avoid on systems with MMU, if necessary. --- compiler/asserts.go | 31 +++++++++++++++++++++++++++++++ compiler/compiler.go | 22 ++++++++++++++++++++++ src/runtime/panic.go | 5 +++++ 3 files changed, 58 insertions(+) create mode 100644 compiler/asserts.go diff --git a/compiler/asserts.go b/compiler/asserts.go new file mode 100644 index 00000000..4c09264a --- /dev/null +++ b/compiler/asserts.go @@ -0,0 +1,31 @@ +package compiler + +// This file implements functions that do certain safety checks that are +// required by the Go programming language. + +import ( + "tinygo.org/x/go-llvm" +) + +// 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. +func (c *Compiler) emitNilCheck(frame *Frame, ptr llvm.Value, blockPrefix string) { + // Check whether this is a nil pointer. + faultBlock := c.ctx.AddBasicBlock(frame.fn.LLVMFn, blockPrefix+".nil") + nextBlock := c.ctx.AddBasicBlock(frame.fn.LLVMFn, blockPrefix+".next") + frame.blockExits[frame.currentBlock] = nextBlock // adjust outgoing block for phi nodes + + // Compare against nil. + nilptr := llvm.ConstPointerNull(ptr.Type()) + isnil := c.builder.CreateICmp(llvm.IntEQ, ptr, nilptr, "") + c.builder.CreateCondBr(isnil, faultBlock, nextBlock) + + // Fail: this is a nil pointer, exit with a panic. + c.builder.SetInsertPointAtEnd(faultBlock) + c.createRuntimeCall("nilpanic", nil, "") + c.builder.CreateUnreachable() + + // Ok: this is a valid pointer. + c.builder.SetInsertPointAtEnd(nextBlock) +} diff --git a/compiler/compiler.go b/compiler/compiler.go index 26f145b5..e22fadf9 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -344,6 +344,14 @@ func (c *Compiler) Compile(mainPath string) error { c.mod.NamedFunction("runtime.activateTask").SetLinkage(llvm.ExternalLinkage) c.mod.NamedFunction("runtime.scheduler").SetLinkage(llvm.ExternalLinkage) + // Tell the optimizer that runtime.alloc is an allocator, meaning that it + // returns values that are never null and never alias to an existing value. + for _, name := range []string{"noalias", "nonnull"} { + attrKind := llvm.AttributeKindID(name) + attr := c.ctx.CreateEnumAttribute(attrKind, 0) + c.mod.NamedFunction("runtime.alloc").AddAttributeAtIndex(0, attr) + } + // see: https://reviews.llvm.org/D18355 if c.Debug { c.mod.AddNamedMetadataOperand("llvm.module.flags", @@ -1400,6 +1408,7 @@ func (c *Compiler) parseCall(frame *Frame, instr *ssa.CallCommon) (llvm.Value, e // closure: {context, function pointer} context := c.builder.CreateExtractValue(value, 0, "") value = c.builder.CreateExtractValue(value, 1, "") + c.emitNilCheck(frame, value, "fpcall") return c.parseFunctionCall(frame, instr.Args, value, context, false) } } @@ -1578,6 +1587,11 @@ func (c *Compiler) parseExpr(frame *Frame, expr ssa.Value) (llvm.Value, error) { llvm.ConstInt(c.ctx.Int32Type(), 0, false), llvm.ConstInt(c.ctx.Int32Type(), uint64(expr.Field), false), } + // Check for nil pointer before calculating the address, from the spec: + // > For an operand x of type T, the address operation &x generates a + // > pointer of type *T to x. [...] If the evaluation of x would cause a + // > run-time panic, then the evaluation of &x does too. + c.emitNilCheck(frame, val, "gep") return c.builder.CreateGEP(val, indices, ""), nil case *ssa.Function: fn := c.ir.GetFunction(expr) @@ -1637,6 +1651,13 @@ func (c *Compiler) parseExpr(frame *Frame, expr ssa.Value) (llvm.Value, error) { case *types.Array: bufptr = val buflen = llvm.ConstInt(c.uintptrType, uint64(typ.Len()), false) + // Check for nil pointer before calculating the address, from + // the spec: + // > For an operand x of type T, the address operation &x + // > generates a pointer of type *T to x. [...] If the + // > evaluation of x would cause a run-time panic, then the + // > evaluation of &x does too. + c.emitNilCheck(frame, bufptr, "gep") default: return llvm.Value{}, c.makeError(expr.Pos(), "todo: indexaddr: "+typ.String()) } @@ -2695,6 +2716,7 @@ func (c *Compiler) parseUnOp(frame *Frame, unop *ssa.UnOp) (llvm.Value, error) { fn := c.mod.NamedFunction(name) return c.builder.CreateBitCast(fn, c.i8ptrType, ""), nil } else { + c.emitNilCheck(frame, x, "deref") load := c.builder.CreateLoad(x, "") if c.ir.IsVolatile(valType) { // Volatile load, for memory-mapped registers. diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 3a182354..721d80bf 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -27,6 +27,11 @@ func _recover() interface{} { return nil } +// Panic when trying to dereference a nil pointer. +func nilpanic() { + runtimePanic("nil pointer dereference") +} + // Check for bounds in *ssa.Index, *ssa.IndexAddr and *ssa.Lookup. func lookupBoundsCheck(length uintptr, index int) { if index < 0 || index >= int(length) {