diff options
author | Ayke van Laethem <[email protected]> | 2020-05-28 01:29:45 +0200 |
---|---|---|
committer | Ron Evans <[email protected]> | 2020-05-28 13:42:36 +0200 |
commit | 734613c20e4695acb11aea0c569dc0fef9455c22 (patch) | |
tree | 3e8b7e322053917fe4ca091af0122aeda4328d48 /transform | |
parent | 1570adac1c28cf0f492e35fe62af7da6b57e42dc (diff) | |
download | tinygo-734613c20e4695acb11aea0c569dc0fef9455c22.tar.gz tinygo-734613c20e4695acb11aea0c569dc0fef9455c22.zip |
transform: introduce check for method calls on nil interfaces
I ran into an issue where I did a method call on a nil interface and it
resulted in a HardFault. Luckily I quickly realized what was going on so
I could fix it, but I think undefined behavior is definitely the wrong
behavior in this case. This commit therefore changes such calls to cause
a nil panic instead of introducing undefined behavior.
This does have a code size impact. It's relatively minor, much lower
than I expected. When comparing the before and after of the drivers
smoke tests (probably the most representative sample available), I found
that most did not change at all and those that did change, normally not
more than 100 bytes (16 or 32 byte changes are typical).
Right now the pattern is the following:
switch typecode {
case 1:
call method 1
case 2:
call method 2
default:
nil panic
}
I also tried the following (in the hope that it would be easier to
optimize), but it didn't really result in a code size reduction:
switch typecode {
case 1:
call method 1
case 2:
call method 2
case 0:
nil panic
default:
unreachable
}
Some code got smaller, while other code (the majority) got bigger. Maybe
this can be improved once range[1] is finally allowed[2] on function
parameters, but it'll probably take a while before that is implemented.
[1]: https://llvm.org/docs/LangRef.html#range-metadata
[2]: https://github.com/rust-lang/rust/issues/50156
Diffstat (limited to 'transform')
-rw-r--r-- | transform/interface-lowering.go | 13 | ||||
-rw-r--r-- | transform/testdata/interface.ll | 1 | ||||
-rw-r--r-- | transform/testdata/interface.out.ll | 3 |
3 files changed, 15 insertions, 2 deletions
diff --git a/transform/interface-lowering.go b/transform/interface-lowering.go index d64f2731d..6aa3bb134 100644 --- a/transform/interface-lowering.go +++ b/transform/interface-lowering.go @@ -616,10 +616,19 @@ func (p *lowerInterfacesPass) createInterfaceMethodFunc(itf *interfaceInfo, sign // Create entry block. entry := p.ctx.AddBasicBlock(fn, "entry") - // Create default block and make it unreachable (which it is, because all - // possible types are checked). + // Create default block and call runtime.nilPanic. + // The only other possible value remaining is nil for nil interfaces. We + // could panic with a different message here such as "nil interface" but + // that would increase code size and "nil panic" is close enough. Most + // importantly, it avoids undefined behavior when accidentally calling a + // method on a nil interface. defaultBlock := p.ctx.AddBasicBlock(fn, "default") p.builder.SetInsertPointAtEnd(defaultBlock) + nilPanic := p.mod.NamedFunction("runtime.nilPanic") + p.builder.CreateCall(nilPanic, []llvm.Value{ + llvm.Undef(llvm.PointerType(p.ctx.Int8Type(), 0)), + llvm.Undef(llvm.PointerType(p.ctx.Int8Type(), 0)), + }, "") p.builder.CreateUnreachable() // Create type switch in entry block. diff --git a/transform/testdata/interface.ll b/transform/testdata/interface.ll index ee059c959..1e2ce61b6 100644 --- a/transform/testdata/interface.ll +++ b/transform/testdata/interface.ll @@ -24,6 +24,7 @@ declare void @runtime.printuint8(i8) declare void @runtime.printint32(i32) declare void @runtime.printptr(i32) declare void @runtime.printnl() +declare void @runtime.nilPanic(i8*, i8*) define void @printInterfaces() { call void @printInterface(i32 ptrtoint (%runtime.typeInInterface* @"typeInInterface:reflect/types.type:basic:int" to i32), i8* inttoptr (i32 5 to i8*)) diff --git a/transform/testdata/interface.out.ll b/transform/testdata/interface.out.ll index 80f71ee4e..4bb9ec1bb 100644 --- a/transform/testdata/interface.out.ll +++ b/transform/testdata/interface.out.ll @@ -25,6 +25,8 @@ declare void @runtime.printptr(i32) declare void @runtime.printnl() +declare void @runtime.nilPanic(i8*, i8*) + define void @printInterfaces() { call void @printInterface(i32 4, i8* inttoptr (i32 5 to i8*)) call void @printInterface(i32 16, i8* inttoptr (i8 120 to i8*)) @@ -83,6 +85,7 @@ entry: ] default: ; preds = %entry + call void @runtime.nilPanic(i8* undef, i8* undef) unreachable "reflect/types.type:named:Number": ; preds = %entry |