Commit 4a1a783d authored by Keith Randall's avatar Keith Randall Committed by Keith Randall

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: 's avatarJosh Bleecher Snyder <josharian@gmail.com>
parent 5bd7e9c5
......@@ -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
......
// 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")
}
}
// 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")
}
}
......@@ -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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment