diff options
| author | Keith Randall <khr@golang.org> | 2025-01-10 17:33:26 -0800 |
|---|---|---|
| committer | Michael Knyszek <mknyszek@google.com> | 2025-01-17 07:41:09 -0800 |
| commit | 9cbbf5e0f4fe5231a5ad425638800618839de0a5 (patch) | |
| tree | 5ebe8ebd0173d28ae26f4a4bf5e92d063a0902fa | |
| parent | d04e3cbc9240e25de449dcae2ec33d03062f347b (diff) | |
| download | go-9cbbf5e0f4fe5231a5ad425638800618839de0a5.tar.xz | |
[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 <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/642498
| -rw-r--r-- | src/cmd/compile/internal/ssa/writebarrier.go | 15 | ||||
| -rw-r--r-- | test/codegen/writebarrier.go | 25 |
2 files changed, 40 insertions, 0 deletions
diff --git a/src/cmd/compile/internal/ssa/writebarrier.go b/src/cmd/compile/internal/ssa/writebarrier.go index 1caccb7c18..71acefbf8a 100644 --- a/src/cmd/compile/internal/ssa/writebarrier.go +++ b/src/cmd/compile/internal/ssa/writebarrier.go @@ -252,6 +252,7 @@ func writebarrier(f *Func) { var start, end int var nonPtrStores int values := b.Values + hasMove := false FindSeq: for i := len(values) - 1; i >= 0; i-- { w := values[i] @@ -263,6 +264,9 @@ func writebarrier(f *Func) { end = i + 1 } nonPtrStores = 0 + if w.Op == OpMoveWB { + hasMove = true + } case OpVarDef, OpVarLive: continue case OpStore: @@ -273,6 +277,17 @@ func writebarrier(f *Func) { if nonPtrStores > 2 { break FindSeq } + if hasMove { + // We need to ensure that this store happens + // before we issue a wbMove, as the wbMove might + // use the result of this store as its source. + // Even though this store is not write-barrier + // eligible, it might nevertheless be the store + // of a pointer to the stack, which is then the + // source of the move. + // See issue 71228. + break FindSeq + } default: if last == nil { continue diff --git a/test/codegen/writebarrier.go b/test/codegen/writebarrier.go index cfcfe15a40..4e0da14433 100644 --- a/test/codegen/writebarrier.go +++ b/test/codegen/writebarrier.go @@ -53,3 +53,28 @@ func combine4slice(p *[4][]byte, a, b, c, d []byte) { // arm64:-`.*runtime[.]gcWriteBarrier` p[3] = d } + +type S struct { + a, b string + c *int +} + +var g1, g2 *int + +func issue71228(dst *S, ptr *int) { + // Make sure that the non-write-barrier write. + // "sp.c = ptr" happens before the large write + // barrier "*dst = *sp". We approximate testing + // that by ensuring that two global variable write + // barriers aren't combined. + _ = *dst + var s S + sp := &s + //amd64:`.*runtime[.]gcWriteBarrier1` + g1 = nil + sp.c = ptr // outside of any write barrier + //amd64:`.*runtime[.]gcWriteBarrier1` + g2 = nil + //amd64:`.*runtime[.]wbMove` + *dst = *sp +} |
