From 5308e8903e8c1a79151c8262fc2992eb405ce049 Mon Sep 17 00:00:00 2001
From: Jaden Weiss <jaden@jadendw.dev>
Date: Wed, 8 Apr 2020 18:25:42 -0400
Subject: [PATCH] compiler: pass interface typecode through defer frames

Previously, the typecode was passed via a direct reference, which results in invalid IR when the defer is not reached in all return paths.
It also results in incorrect behavior if the defer is in a loop, causing all defers to use the typecode of the last iteration.
---
 compiler/defer.go     | 12 ++++++++----
 compiler/interface.go | 22 +++++++++++++---------
 testdata/calls.go     | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/compiler/defer.go b/compiler/defer.go
index fa7d7623..4a2a480a 100644
--- a/compiler/defer.go
+++ b/compiler/defer.go
@@ -92,9 +92,10 @@ func (b *builder) createDefer(instr *ssa.Defer) {
 		// Collect all values to be put in the struct (starting with
 		// runtime._defer fields, followed by the call parameters).
 		itf := b.getValue(instr.Call.Value) // interface
+		typecode := b.CreateExtractValue(itf, 0, "invoke.func.typecode")
 		receiverValue := b.CreateExtractValue(itf, 1, "invoke.func.receiver")
-		values = []llvm.Value{callback, next, receiverValue}
-		valueTypes = append(valueTypes, b.i8ptrType)
+		values = []llvm.Value{callback, next, typecode, receiverValue}
+		valueTypes = append(valueTypes, b.uintptrType, b.i8ptrType)
 		for _, arg := range instr.Call.Args {
 			val := b.getValue(arg)
 			values = append(values, val)
@@ -248,7 +249,7 @@ func (b *builder) createRunDefers() {
 			}
 
 			// Get the real defer struct type and cast to it.
-			valueTypes := []llvm.Type{b.uintptrType, llvm.PointerType(b.getLLVMRuntimeType("_defer"), 0), b.i8ptrType}
+			valueTypes := []llvm.Type{b.uintptrType, llvm.PointerType(b.getLLVMRuntimeType("_defer"), 0), b.uintptrType, b.i8ptrType}
 			for _, arg := range callback.Args {
 				valueTypes = append(valueTypes, b.getLLVMType(arg.Type()))
 			}
@@ -264,6 +265,9 @@ func (b *builder) createRunDefers() {
 				forwardParams = append(forwardParams, forwardParam)
 			}
 
+			// Isolate the typecode.
+			typecode, forwardParams := forwardParams[0], forwardParams[1:]
+
 			// Add the context parameter. An interface call cannot also be a
 			// closure but we have to supply the parameter anyway for platforms
 			// with a strict calling convention.
@@ -272,7 +276,7 @@ func (b *builder) createRunDefers() {
 			// Parent coroutine handle.
 			forwardParams = append(forwardParams, llvm.Undef(b.i8ptrType))
 
-			fnPtr, _ := b.getInvokeCall(callback)
+			fnPtr := b.getInvokePtr(callback, typecode)
 			b.createCall(fnPtr, forwardParams, "")
 
 		case *ir.Function:
diff --git a/compiler/interface.go b/compiler/interface.go
index 600bb08c..fd48f261 100644
--- a/compiler/interface.go
+++ b/compiler/interface.go
@@ -396,22 +396,26 @@ func (b *builder) createTypeAssert(expr *ssa.TypeAssert) llvm.Value {
 	}
 }
 
-// getInvokeCall creates and returns the function pointer and parameters of an
-// interface call. It can be used in a call or defer instruction.
-func (b *builder) getInvokeCall(instr *ssa.CallCommon) (llvm.Value, []llvm.Value) {
-	// Call an interface method with dynamic dispatch.
-	itf := b.getValue(instr.Value) // interface
-
+// getInvokePtr creates an interface function pointer lookup for the specified invoke instruction, using a specified typecode.
+func (b *builder) getInvokePtr(instr *ssa.CallCommon, typecode llvm.Value) llvm.Value {
 	llvmFnType := b.getRawFuncType(instr.Method.Type().(*types.Signature))
-
-	typecode := b.CreateExtractValue(itf, 0, "invoke.typecode")
 	values := []llvm.Value{
 		typecode,
 		b.getInterfaceMethodSet(instr.Value.Type()),
 		b.getMethodSignature(instr.Method),
 	}
 	fn := b.createRuntimeCall("interfaceMethod", values, "invoke.func")
-	fnCast := b.CreateIntToPtr(fn, llvmFnType, "invoke.func.cast")
+	return b.CreateIntToPtr(fn, llvmFnType, "invoke.func.cast")
+}
+
+// getInvokeCall creates and returns the function pointer and parameters of an
+// interface call.
+func (b *builder) getInvokeCall(instr *ssa.CallCommon) (llvm.Value, []llvm.Value) {
+	// Call an interface method with dynamic dispatch.
+	itf := b.getValue(instr.Value) // interface
+
+	typecode := b.CreateExtractValue(itf, 0, "invoke.typecode")
+	fnCast := b.getInvokePtr(instr, typecode)
 	receiverValue := b.CreateExtractValue(itf, 1, "invoke.func.receiver")
 
 	args := []llvm.Value{receiverValue}
diff --git a/testdata/calls.go b/testdata/calls.go
index 00a09136..9818cdb7 100644
--- a/testdata/calls.go
+++ b/testdata/calls.go
@@ -61,6 +61,9 @@ func main() {
 	thingFunctionalArgs1.Print("functional args 1")
 	thingFunctionalArgs2 := NewThing(WithName("named thing"))
 	thingFunctionalArgs2.Print("functional args 2")
+
+	// regression testing
+	regression1033()
 }
 
 func runFunc(f func(int), arg int) {
@@ -108,3 +111,36 @@ func exportedDefer() {
 func testBound(f func() string) {
 	println("bound method:", f())
 }
+
+// regression1033 is a regression test for https://github.com/tinygo-org/tinygo/issues/1033.
+// In previous versions of the compiler, a deferred call to an interface would create an instruction that did not dominate its uses.
+func regression1033() {
+	foo(&Bar{})
+}
+
+type Bar struct {
+	empty bool
+}
+
+func (b *Bar) Close() error {
+	return nil
+}
+
+type Closer interface {
+	Close() error
+}
+
+func foo(bar *Bar) error {
+	var a int
+	if !bar.empty {
+		a = 10
+		if a != 5 {
+			return nil
+		}
+	}
+
+	var c Closer = bar
+	defer c.Close()
+
+	return nil
+}