aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKeith Randall <khr@golang.org>2025-01-10 17:33:26 -0800
committerMichael Knyszek <mknyszek@google.com>2025-01-17 07:41:09 -0800
commit9cbbf5e0f4fe5231a5ad425638800618839de0a5 (patch)
tree5ebe8ebd0173d28ae26f4a4bf5e92d063a0902fa
parentd04e3cbc9240e25de449dcae2ec33d03062f347b (diff)
downloadgo-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.go15
-rw-r--r--test/codegen/writebarrier.go25
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
+}