From deaf29a8a8ab76613bf0d5d97c4e31bfbdc4c4e9 Mon Sep 17 00:00:00 2001
From: Matthew Dempsky <mdempsky@google.com>
Date: Thu, 21 Jan 2021 19:27:12 -0800
Subject: [PATCH] cmd/compile: fix order-of-assignment issue w/ defers

CL 261677 fixed a logic issue in walk's alias detection, where it was
checking the RHS expression instead of the LHS expression when trying
to determine the kind of assignment. However, correcting this exposed
a latent issue with assigning to result parameters in functions with
defers, where an assignment could become visible earlier than intended
if a later expression could panic.

Fixes #43835.

Change-Id: I061ced125e3896e26d65f45b28c99db2c8a74a8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/285633
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
---
 src/cmd/compile/internal/gc/walk.go | 12 +++++++++--
 test/fixedbugs/issue43835.go        | 33 +++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 test/fixedbugs/issue43835.go

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index a7b6e7fcb3c765..2133a160b2b6cf 100644
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -267,7 +267,7 @@ func walkstmt(n *Node) *Node {
 		if n.List.Len() == 0 {
 			break
 		}
-		if (Curfn.Type.FuncType().Outnamed && n.List.Len() > 1) || paramoutheap(Curfn) {
+		if (Curfn.Type.FuncType().Outnamed && n.List.Len() > 1) || paramoutheap(Curfn) || Curfn.Func.HasDefer() {
 			// assign to the function out parameters,
 			// so that reorder3 can fix up conflicts
 			var rl []*Node
@@ -2233,7 +2233,15 @@ func aliased(r *Node, all []*Node) bool {
 			memwrite = true
 			continue
 
-		case PAUTO, PPARAM, PPARAMOUT:
+		case PPARAMOUT:
+			// Assignments to a result parameter in a function with defers
+			// becomes visible early if evaluation of any later expression
+			// panics (#43835).
+			if Curfn.Func.HasDefer() {
+				return true
+			}
+			fallthrough
+		case PAUTO, PPARAM:
 			if l.Name.Addrtaken() {
 				memwrite = true
 				continue
diff --git a/test/fixedbugs/issue43835.go b/test/fixedbugs/issue43835.go
new file mode 100644
index 00000000000000..449eb72ee109b6
--- /dev/null
+++ b/test/fixedbugs/issue43835.go
@@ -0,0 +1,33 @@
+// run
+
+// 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
+
+func main() {
+	if f() {
+		panic("FAIL")
+	}
+	if bad, _ := g(); bad {
+		panic("FAIL")
+	}
+}
+
+func f() (bad bool) {
+	defer func() {
+		recover()
+	}()
+	var p *int
+	bad, _ = true, *p
+	return
+}
+
+func g() (bool, int) {
+	defer func() {
+		recover()
+	}()
+	var p *int
+	return true, *p
+}