mirror of
https://github.com/golang/go
synced 2024-10-14 20:05:36 +00:00
cmd/compile: copy volatile values before emitting write barrier call
It is possible that a "volatile" value (one that can be clobbered by preparing args of a call) to be used in multiple write barrier calls. We used to copy the volatile value right before each call. But this doesn't work if the value is used the second time, after the first call where it is already clobbered. Copy it before emitting any call. Fixes #30977. Change-Id: Iedcc91ad848d5ded547bf37a8359c125d32e994c Reviewed-on: https://go-review.googlesource.com/c/go/+/168677 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
This commit is contained in:
parent
804a4024ec
commit
f23c601bf9
|
@ -216,6 +216,43 @@ func writebarrier(f *Func) {
|
|||
// and simple store version to bElse
|
||||
memThen := mem
|
||||
memElse := mem
|
||||
|
||||
// If the source of a MoveWB is volatile (will be clobbered by a
|
||||
// function call), we need to copy it to a temporary location, as
|
||||
// marshaling the args of typedmemmove might clobber the value we're
|
||||
// trying to move.
|
||||
// Look for volatile source, copy it to temporary before we emit any
|
||||
// call.
|
||||
// It is unlikely to have more than one of them. Just do a linear
|
||||
// search instead of using a map.
|
||||
type volatileCopy struct {
|
||||
src *Value // address of original volatile value
|
||||
tmp *Value // address of temporary we've copied the volatile value into
|
||||
}
|
||||
var volatiles []volatileCopy
|
||||
copyLoop:
|
||||
for _, w := range stores {
|
||||
if w.Op == OpMoveWB {
|
||||
val := w.Args[1]
|
||||
if isVolatile(val) {
|
||||
for _, c := range volatiles {
|
||||
if val == c.src {
|
||||
continue copyLoop // already copied
|
||||
}
|
||||
}
|
||||
|
||||
t := val.Type.Elem()
|
||||
tmp := f.fe.Auto(w.Pos, t)
|
||||
memThen = bThen.NewValue1A(w.Pos, OpVarDef, types.TypeMem, tmp, memThen)
|
||||
tmpaddr := bThen.NewValue2A(w.Pos, OpLocalAddr, t.PtrTo(), tmp, sp, memThen)
|
||||
siz := t.Size()
|
||||
memThen = bThen.NewValue3I(w.Pos, OpMove, types.TypeMem, siz, tmpaddr, val, memThen)
|
||||
memThen.Aux = t
|
||||
volatiles = append(volatiles, volatileCopy{val, tmpaddr})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for _, w := range stores {
|
||||
ptr := w.Args[0]
|
||||
pos := w.Pos
|
||||
|
@ -242,11 +279,19 @@ func writebarrier(f *Func) {
|
|||
// then block: emit write barrier call
|
||||
switch w.Op {
|
||||
case OpStoreWB, OpMoveWB, OpZeroWB:
|
||||
volatile := w.Op == OpMoveWB && isVolatile(val)
|
||||
if w.Op == OpStoreWB {
|
||||
memThen = bThen.NewValue3A(pos, OpWB, types.TypeMem, gcWriteBarrier, ptr, val, memThen)
|
||||
} else {
|
||||
memThen = wbcall(pos, bThen, fn, typ, ptr, val, memThen, sp, sb, volatile)
|
||||
srcval := val
|
||||
if w.Op == OpMoveWB && isVolatile(srcval) {
|
||||
for _, c := range volatiles {
|
||||
if srcval == c.src {
|
||||
srcval = c.tmp
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
memThen = wbcall(pos, bThen, fn, typ, ptr, srcval, memThen, sp, sb)
|
||||
}
|
||||
// Note that we set up a writebarrier function call.
|
||||
f.fe.SetWBPos(pos)
|
||||
|
@ -269,6 +314,12 @@ func writebarrier(f *Func) {
|
|||
}
|
||||
}
|
||||
|
||||
// mark volatile temps dead
|
||||
for _, c := range volatiles {
|
||||
tmpNode := c.tmp.Aux
|
||||
memThen = bThen.NewValue1A(memThen.Pos, OpVarKill, types.TypeMem, tmpNode, memThen)
|
||||
}
|
||||
|
||||
// merge memory
|
||||
// Splice memory Phi into the last memory of the original sequence,
|
||||
// which may be used in subsequent blocks. Other memories in the
|
||||
|
@ -403,25 +454,9 @@ func (f *Func) computeZeroMap() map[ID]ZeroRegion {
|
|||
}
|
||||
|
||||
// wbcall emits write barrier runtime call in b, returns memory.
|
||||
// if valIsVolatile, it moves val into temp space before making the call.
|
||||
func wbcall(pos src.XPos, b *Block, fn, typ *obj.LSym, ptr, val, mem, sp, sb *Value, valIsVolatile bool) *Value {
|
||||
func wbcall(pos src.XPos, b *Block, fn, typ *obj.LSym, ptr, val, mem, sp, sb *Value) *Value {
|
||||
config := b.Func.Config
|
||||
|
||||
var tmp GCNode
|
||||
if valIsVolatile {
|
||||
// Copy to temp location if the source is volatile (will be clobbered by
|
||||
// a function call). Marshaling the args to typedmemmove might clobber the
|
||||
// value we're trying to move.
|
||||
t := val.Type.Elem()
|
||||
tmp = b.Func.fe.Auto(val.Pos, t)
|
||||
mem = b.NewValue1A(pos, OpVarDef, types.TypeMem, tmp, mem)
|
||||
tmpaddr := b.NewValue2A(pos, OpLocalAddr, t.PtrTo(), tmp, sp, mem)
|
||||
siz := t.Size()
|
||||
mem = b.NewValue3I(pos, OpMove, types.TypeMem, siz, tmpaddr, val, mem)
|
||||
mem.Aux = t
|
||||
val = tmpaddr
|
||||
}
|
||||
|
||||
// put arguments on stack
|
||||
off := config.ctxt.FixedFrameSize()
|
||||
|
||||
|
@ -449,11 +484,6 @@ func wbcall(pos src.XPos, b *Block, fn, typ *obj.LSym, ptr, val, mem, sp, sb *Va
|
|||
// issue call
|
||||
mem = b.NewValue1A(pos, OpStaticCall, types.TypeMem, fn, mem)
|
||||
mem.AuxInt = off - config.ctxt.FixedFrameSize()
|
||||
|
||||
if valIsVolatile {
|
||||
mem = b.NewValue1A(pos, OpVarKill, types.TypeMem, tmp, mem) // mark temp dead
|
||||
}
|
||||
|
||||
return mem
|
||||
}
|
||||
|
||||
|
|
52
test/fixedbugs/issue30977.go
Normal file
52
test/fixedbugs/issue30977.go
Normal file
|
@ -0,0 +1,52 @@
|
|||
// run
|
||||
|
||||
// Copyright 2019 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 30977: write barrier call clobbers volatile
|
||||
// value when there are multiple uses of the value.
|
||||
|
||||
package main
|
||||
|
||||
import "runtime"
|
||||
|
||||
type T struct {
|
||||
a, b, c, d, e string
|
||||
}
|
||||
|
||||
//go:noinline
|
||||
func g() T {
|
||||
return T{"a", "b", "c", "d", "e"}
|
||||
}
|
||||
|
||||
//go:noinline
|
||||
func f() {
|
||||
// The compiler optimizes this to direct copying
|
||||
// the call result to both globals, with write
|
||||
// barriers. The first write barrier call clobbers
|
||||
// the result of g on stack.
|
||||
X = g()
|
||||
Y = X
|
||||
}
|
||||
|
||||
var X, Y T
|
||||
|
||||
const N = 1000
|
||||
|
||||
func main() {
|
||||
// Keep GC running so the write barrier is on.
|
||||
go func() {
|
||||
for {
|
||||
runtime.GC()
|
||||
}
|
||||
}()
|
||||
|
||||
for i := 0; i < N; i++ {
|
||||
runtime.Gosched()
|
||||
f()
|
||||
if X != Y {
|
||||
panic("FAIL")
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue