cmd/compile: fix static initializer

staticcopy of a struct or array should recursively call itself, not
staticassign.

This fixes an issue where a struct with a slice in it is copied during
static initialization. In this case, the backing array for the slice
is duplicated, and each copy of the slice refers to a different
backing array, which is incorrect.  That issue has existed since at
least Go 1.2.

I'm not sure why this was never noticed. It seems like a pretty
obvious bug if anyone modifies the resulting slice.

In any case, we started to notice when the optimization in CL 140301
landed.  Here is basically what happens in issue29013b.go:
1) The error above happens, so we get two backing stores for what
   should be the same slice.
2) The code for initializing those backing stores is reused.
   But not duplicated: they are the same Node structure.
3) The order pass allocates temporaries for the map operations.
   For the first instance, things work fine and two temporaries are
   allocated and stored in the OKEY nodes. For the second instance,
   the order pass decides new temporaries aren't needed, because
   the OKEY nodes already have temporaries in them.
   But the order pass also puts a VARKILL of the temporaries between
   the two instance initializations.
4) In this state, the code is technically incorrect. But before
   CL 140301 it happens to work because the temporaries are still
   correctly initialized when they are used for the second time. But then...
5) The new CL 140301 sees the VARKILLs and decides to reuse the
   temporary for instance 1 map 2 to initialize the instance 2 map 1
   map. Because the keys aren't re-initialized, instance 2 map 1
   gets the wrong key inserted into it.

Fixes #29013

Change-Id: I840ce1b297d119caa706acd90e1517a5e47e9848
Reviewed-on: https://go-review.googlesource.com/c/152081
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go
index 56c63065b222ae4d2374864bf6351ed0be0fc8e4..de0298b746fe46709f17b028a7709367e9c8e51b 100644 (file)
--- a/src/cmd/compile/internal/gc/sinit.go
+++ b/src/cmd/compile/internal/gc/sinit.go
@@ -350,7 +350,7 @@ func staticcopy(l *Node, r *Node, out *[]*Node) bool {
                                continue
                        }
                        ll := n.sepcopy()
-                       if staticassign(ll, e.Expr, out) {
+                       if staticcopy(ll, e.Expr, out) {
                                continue
                        }
                        // Requires computation, but we're

diff --git a/test/fixedbugs/issue29013a.go b/test/fixedbugs/issue29013a.go
new file mode 100644 (file)
index 0000000..efc50df
--- /dev/null
+++ b/test/fixedbugs/issue29013a.go
@@ -0,0 +1,24 @@
+// run
+
+// Copyright 2018 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
+
+type TestSuite struct {
+       Tests []int
+}
+
+var Suites = []TestSuite{
+       Dicts,
+}
+var Dicts = TestSuite{
+       Tests: []int{0},
+}
+
+func main() {
+       if &Dicts.Tests[0] != &Suites[0].Tests[0] {
+               panic("bad")
+       }
+}

diff --git a/test/fixedbugs/issue29013b.go b/test/fixedbugs/issue29013b.go
new file mode 100644 (file)
index 0000000..b8502da
--- /dev/null
+++ b/test/fixedbugs/issue29013b.go
@@ -0,0 +1,43 @@
+// run
+
+// Copyright 2018 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
+
+type TestSuite struct {
+       Tests []Test
+}
+type Test struct {
+       Want interface{}
+}
+type Int struct {
+       i int
+}
+
+func NewInt(v int) Int {
+       return Int{i: v}
+}
+
+var Suites = []TestSuite{
+       Dicts,
+}
+var Dicts = TestSuite{
+       Tests: []Test{
+               {
+                       Want: map[Int]bool{NewInt(1): true},
+               },
+               {
+                       Want: map[Int]string{
+                               NewInt(3): "3",
+                       },
+               },
+       },
+}
+
+func main() {
+       if Suites[0].Tests[0].Want.(map[Int]bool)[NewInt(3)] {
+               panic("bad")
+       }
+}

diff --git a/test/sinit.go b/test/sinit.go
index c4d0edf87136ac9fa2b044ed9c3ab9de87d3af0b..df4d50d36769ce0f9649f7f3855b156f39eeaaf5 100644 (file)
--- a/test/sinit.go
+++ b/test/sinit.go
@@ -43,15 +43,12 @@ var c = []int{1201, 1202, 1203}
 
 var aa = [3][3]int{[3]int{2001, 2002, 2003}, [3]int{2004, 2005, 2006}, [3]int{2007, 2008, 2009}}
 var as = [3]S{S{2101, 2102, 2103}, S{2104, 2105, 2106}, S{2107, 2108, 2109}}
-var ac = [3][]int{[]int{2201, 2202, 2203}, []int{2204, 2205, 2206}, []int{2207, 2208, 2209}}
 
 var sa = SA{[3]int{3001, 3002, 3003}, [3]int{3004, 3005, 3006}, [3]int{3007, 3008, 3009}}
 var ss = SS{S{3101, 3102, 3103}, S{3104, 3105, 3106}, S{3107, 3108, 3109}}
-var sc = SC{[]int{3201, 3202, 3203}, []int{3204, 3205, 3206}, []int{3207, 3208, 3209}}
 
 var ca = [][3]int{[3]int{4001, 4002, 4003}, [3]int{4004, 4005, 4006}, [3]int{4007, 4008, 4009}}
 var cs = []S{S{4101, 4102, 4103}, S{4104, 4105, 4106}, S{4107, 4108, 4109}}
-var cc = [][]int{[]int{4201, 4202, 4203}, []int{4204, 4205, 4206}, []int{4207, 4208, 4209}}
 
 var answers = [...]int{
        // s
@@ -135,15 +132,12 @@ var copy_c = c
 
 var copy_aa = aa
 var copy_as = as
-var copy_ac = ac
 
 var copy_sa = sa
 var copy_ss = ss
-var copy_sc = sc
 
 var copy_ca = ca
 var copy_cs = cs
-var copy_cc = cc
 
 var copy_answers = answers