From cfac62a1cc021cae55c9c5e373ebe9c408b75a5d Mon Sep 17 00:00:00 2001
From: Than McIntosh <thanm@google.com>
Date: Thu, 22 Apr 2021 18:34:57 -0400
Subject: [PATCH] cmd/compile: fix bug in defer wrapping

The defer wrapping feature added to the compiler's "order" phase
creates temporaries into which it copies defer arguments. If one of
these temps is large enough that we place it into the defer closure by
address (as opposed to by value), then the temp in question can't be
reused later on in the order phase, nor do we want a VARKILL
annotation for it at the end of the current block scope.

Test written by Cherry.

Updates #40724.

Change-Id: Iec7efd87ec5a3e3d7de41cdcc7f39c093ed1e815
Reviewed-on: https://go-review.googlesource.com/c/go/+/312869
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
---
 src/cmd/compile/internal/walk/order.go | 20 +++++++++++---
 test/abi/wrapdefer_largetmp.go         | 37 ++++++++++++++++++++++++++
 test/abi/wrapdefer_largetmp.out        |  1 +
 3 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 test/abi/wrapdefer_largetmp.go
 create mode 100644 test/abi/wrapdefer_largetmp.out

diff --git a/src/cmd/compile/internal/walk/order.go b/src/cmd/compile/internal/walk/order.go
index dcb8f654f5..7037b8ea60 100644
--- a/src/cmd/compile/internal/walk/order.go
+++ b/src/cmd/compile/internal/walk/order.go
@@ -1592,12 +1592,26 @@ func (o *orderState) wrapGoDefer(n *ir.GoDeferStmt) {
 		return n.Esc() != ir.EscNever
 	}()
 
-	// A helper for making a copy of an argument.
+	// A helper for making a copy of an argument. Note that it is
+	// not safe to use o.copyExpr(arg) if we're putting a
+	// reference to the temp into the closure (as opposed to
+	// copying it in by value), since in the by-reference case we
+	// need a temporary whose lifetime extends to the end of the
+	// function (as opposed to being local to the current block or
+	// statement being ordered).
 	mkArgCopy := func(arg ir.Node) *ir.Name {
-		argCopy := o.copyExpr(arg)
+		t := arg.Type()
+		byval := t.Size() <= 128 || cloEscapes
+		var argCopy *ir.Name
+		if byval {
+			argCopy = o.copyExpr(arg)
+		} else {
+			argCopy = typecheck.Temp(t)
+			o.append(ir.NewAssignStmt(base.Pos, argCopy, arg))
+		}
 		// The value of 128 below is meant to be consistent with code
 		// in escape analysis that picks byval/byaddr based on size.
-		argCopy.SetByval(argCopy.Type().Size() <= 128 || cloEscapes)
+		argCopy.SetByval(byval)
 		return argCopy
 	}
 
diff --git a/test/abi/wrapdefer_largetmp.go b/test/abi/wrapdefer_largetmp.go
new file mode 100644
index 0000000000..fb6eebaaca
--- /dev/null
+++ b/test/abi/wrapdefer_largetmp.go
@@ -0,0 +1,37 @@
+// run
+
+//go:build !wasm
+// +build !wasm
+
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+//go:noinline
+func F() {
+	b := g()
+	defer g2(b)
+	n := g()[20]
+	println(n)
+}
+
+type T [45]int
+
+var x = 0
+
+//go:noinline
+func g() T {
+	x++
+	return T{20: x}
+}
+
+//go:noinline
+func g2(t T) {
+	if t[20] != 1 {
+		println("FAIL", t[20])
+	}
+}
+
+func main() { F() }
diff --git a/test/abi/wrapdefer_largetmp.out b/test/abi/wrapdefer_largetmp.out
new file mode 100644
index 0000000000..0cfbf08886
--- /dev/null
+++ b/test/abi/wrapdefer_largetmp.out
@@ -0,0 +1 @@
+2