Skip to content

Commit 9cbbf5e

Browse files
randall77mknyszek
authored andcommitted
[release-branch.go1.23] 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 #71230 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]> Reviewed-on: https://go-review.googlesource.com/c/go/+/642498
1 parent d04e3cb commit 9cbbf5e

File tree

2 files changed

+40
-0
lines changed

2 files changed

+40
-0
lines changed

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

+15
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

+25
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,28 @@ func combine4slice(p *[4][]byte, a, b, c, d []byte) {
5353
// arm64:-`.*runtime[.]gcWriteBarrier`
5454
p[3] = d
5555
}
56+
57+
type S struct {
58+
a, b string
59+
c *int
60+
}
61+
62+
var g1, g2 *int
63+
64+
func issue71228(dst *S, ptr *int) {
65+
// Make sure that the non-write-barrier write.
66+
// "sp.c = ptr" happens before the large write
67+
// barrier "*dst = *sp". We approximate testing
68+
// that by ensuring that two global variable write
69+
// barriers aren't combined.
70+
_ = *dst
71+
var s S
72+
sp := &s
73+
//amd64:`.*runtime[.]gcWriteBarrier1`
74+
g1 = nil
75+
sp.c = ptr // outside of any write barrier
76+
//amd64:`.*runtime[.]gcWriteBarrier1`
77+
g2 = nil
78+
//amd64:`.*runtime[.]wbMove`
79+
*dst = *sp
80+
}

0 commit comments

Comments
 (0)