Skip to content

Commit 44a6f81

Browse files
committed
cmd/compile: fix write barrier coalescing
We can't coalesce a non-WB store with a subsequent Move, as the result of the store might be the source of the move. There's a simple codegen test. Not sure how we might do a real test, as all the repro's I've come up with are very expensive and unreliable. Fixes #71228 Change-Id: If18bf181a266b9b90964e2591cd2e61a7168371c Reviewed-on: https://go-review.googlesource.com/c/go/+/642197 Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: David Chase <[email protected]>
1 parent 19e9231 commit 44a6f81

File tree

2 files changed

+40
-0
lines changed

2 files changed

+40
-0
lines changed

src/cmd/compile/internal/ssa/writebarrier.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ func writebarrier(f *Func) {
252252
var start, end int
253253
var nonPtrStores int
254254
values := b.Values
255+
hasMove := false
255256
FindSeq:
256257
for i := len(values) - 1; i >= 0; i-- {
257258
w := values[i]
@@ -263,6 +264,9 @@ func writebarrier(f *Func) {
263264
end = i + 1
264265
}
265266
nonPtrStores = 0
267+
if w.Op == OpMoveWB {
268+
hasMove = true
269+
}
266270
case OpVarDef, OpVarLive:
267271
continue
268272
case OpStore:
@@ -273,6 +277,17 @@ func writebarrier(f *Func) {
273277
if nonPtrStores > 2 {
274278
break FindSeq
275279
}
280+
if hasMove {
281+
// We need to ensure that this store happens
282+
// before we issue a wbMove, as the wbMove might
283+
// use the result of this store as its source.
284+
// Even though this store is not write-barrier
285+
// eligible, it might nevertheless be the store
286+
// of a pointer to the stack, which is then the
287+
// source of the move.
288+
// See issue 71228.
289+
break FindSeq
290+
}
276291
default:
277292
if last == nil {
278293
continue

test/codegen/writebarrier.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,28 @@ func trickyWriteNil(p *int, q **int) {
6363
*q = p
6464
}
6565
}
66+
67+
type S struct {
68+
a, b string
69+
c *int
70+
}
71+
72+
var g1, g2 *int
73+
74+
func issue71228(dst *S, ptr *int) {
75+
// Make sure that the non-write-barrier write.
76+
// "sp.c = ptr" happens before the large write
77+
// barrier "*dst = *sp". We approximate testing
78+
// that by ensuring that two global variable write
79+
// barriers aren't combined.
80+
_ = *dst
81+
var s S
82+
sp := &s
83+
//amd64:`.*runtime[.]gcWriteBarrier1`
84+
g1 = nil
85+
sp.c = ptr // outside of any write barrier
86+
//amd64:`.*runtime[.]gcWriteBarrier1`
87+
g2 = nil
88+
//amd64:`.*runtime[.]wbMove`
89+
*dst = *sp
90+
}

0 commit comments

Comments
 (0)