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>

diff --git a/src/cmd/compile/internal/gc/order.go b/src/cmd/compile/internal/gc/order.go
index 297db503333a26f98df5f72fa743c0605a9404de..3265805000769f2b3255240341df719c86f795af 100644 (file)
--- 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 (file)
index 0000000..9b4c03f
--- /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)
+       }
+}