From 34db5f0c4d80b8fe3fb4b5be90efd9ee92bd1d4d Mon Sep 17 00:00:00 2001
From: Matthew Dempsky <mdempsky@google.com>
Date: Tue, 29 Aug 2017 15:40:32 -0700
Subject: [PATCH] cmd/compile: fix evaluation order for OASOP

Currently, we handle "x op= y" by rewriting as "x = x op y", while
ensuring that any calls or receive operations in 'x' are only
evaluated once. Notably, pointer indirection, indexing operations,
etc. are left alone as it's typically safe to re-evaluate those.

However, those operations were interleaved with evaluating 'y', which
could include function calls that might cause re-evaluation to yield
different memory addresses.

As a fix, simply ensure that we order side-effecting operations in 'y'
before either evaluation of 'x'.

Fixes #21687.

Change-Id: Ib14e77760fda9c828e394e8e362dc9e5319a84b2
Reviewed-on: https://go-review.googlesource.com/60091
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
---
 src/cmd/compile/internal/gc/order.go |  3 +-
 test/fixedbugs/issue21687.go         | 68 ++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 test/fixedbugs/issue21687.go

diff --git a/src/cmd/compile/internal/gc/order.go b/src/cmd/compile/internal/gc/order.go
index 297db503333..32658050007 100644
--- a/src/cmd/compile/internal/gc/order.go
+++ b/src/cmd/compile/internal/gc/order.go
@@ -533,8 +533,9 @@ func orderstmt(n *Node, order *Order) {
 		// out map read from map write when l is
 		// a map index expression.
 		t := marktemp(order)
-
 		n.Left = orderexpr(n.Left, order, nil)
+		n.Right = orderexpr(n.Right, order, nil)
+
 		n.Left = ordersafeexpr(n.Left, order)
 		tmp1 := treecopy(n.Left, src.NoXPos)
 		if tmp1.Op == OINDEXMAP {
diff --git a/test/fixedbugs/issue21687.go b/test/fixedbugs/issue21687.go
new file mode 100644
index 00000000000..9b4c03feb3d
--- /dev/null
+++ b/test/fixedbugs/issue21687.go
@@ -0,0 +1,68 @@
+// run
+
+// Copyright 2017 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.
+
+// Issue 21687: cmd/compile evaluates x twice in "x op= y", which was
+// detectable if evaluating y affects x.
+
+package main
+
+func ptrs() (int, int) {
+	one := 1
+	two := 2
+
+	x := &one
+	*x += func() int {
+		x = &two
+		return 0
+	}()
+
+	return one, two
+}
+
+func slices() (int, int) {
+	one := []int{1}
+	two := []int{2}
+
+	x := one
+	x[0] += func() int {
+		x = two
+		return 0
+	}()
+
+	return one[0], two[0]
+}
+
+func maps() (int, int) {
+	one := map[int]int{0: 1}
+	two := map[int]int{0: 2}
+
+	x := one
+	x[0] += func() int {
+		x = two
+		return 0
+	}()
+
+	return one[0], two[0]
+}
+
+var tests = [...]func() (int, int){
+	ptrs,
+	slices,
+	maps,
+}
+
+func main() {
+	bad := 0
+	for i, f := range tests {
+		if a, b := f(); a+b != 3 {
+			println(i, a, b)
+			bad++
+		}
+	}
+	if bad != 0 {
+		panic(bad)
+	}
+}