From: Keith Randall <khr@golang.org>
Date: Sat, 11 Jan 2025 01:33:26 +0000 (-0800)
Subject: cmd/compile: fix write barrier coalescing
X-Git-Tag: go1.24rc3~2^2~51
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=44a6f817ea0fbeb3ba4aa398794c4e80dba13b1e;p=gostls13.git

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 <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
---

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 e125973e7c..e2b1399399 100644
--- a/test/codegen/writebarrier.go
+++ b/test/codegen/writebarrier.go
@@ -63,3 +63,28 @@ func trickyWriteNil(p *int, q **int) {
 		*q = p
 	}
 }
+
+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
+}