From: Mark Freeman <mark@golang.org>
Date: Thu, 23 Oct 2025 16:52:19 +0000 (-0400)
Subject: go/types, types2: set t.underlying exactly once in resolveUnderlying
X-Git-Tag: go1.26rc1~518
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=cf826bfcb494a7dba5451bd7e4432f150590b19e;p=gostls13.git

go/types, types2: set t.underlying exactly once in resolveUnderlying

While the locking in Named.resolveUnderlying is mostly fine, we do not
perform an atomic read before the write to underlying.

If resolveUnderlying returns and another thread was waiting on the lock,
it can perform a second (in this case identical) write to t.underlying.
A reader thread on n.underlying can thus observe either of those writes,
tripping the race detector.

Michael was kind enough to provide a diagram:

   T1                                   T2
1. t.stateHas(underlying) // false
2.                                      t.stateHas(underlying) // false
3. t.mu.Lock() // acquired
4.                                      t.mu.Lock() // blocked
5. t.underlying = u
6. t.setState(underlying)
7. t.mu.Unlock()
8.                                      t.underlying = u // overwritten
9.                                      t.setState(underlying)
10.                                     t.mu.Unlock()

Adding a second check before setting t.underlying prevents the write on
line 8.

Change-Id: Ia43a6d3ba751caef436b9926c6ece2a71dfb9d38
Reviewed-on: https://go-review.googlesource.com/c/go/+/714300
Auto-Submit: Mark Freeman <markfreeman@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---

diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go
index 26be3104b8..a75ca75d0c 100644
--- a/src/cmd/compile/internal/types2/named.go
+++ b/src/cmd/compile/internal/types2/named.go
@@ -630,6 +630,7 @@ func (n *Named) resolveUnderlying() {
 			t.resolve()
 			t.mu.Lock()
 			defer t.mu.Unlock()
+
 			assert(t.fromRHS != nil || t.allowNilRHS)
 			rhs = t.fromRHS
 
@@ -640,6 +641,12 @@ func (n *Named) resolveUnderlying() {
 
 	// set underlying for all Named types in the chain
 	for t := range seen {
+		// Careful, t.underlying has lock-free readers. Since we might be racing
+		// another call to resolveUnderlying, we have to avoid overwriting
+		// t.underlying. Otherwise, the race detector will be tripped.
+		if t.stateHas(underlying) {
+			continue
+		}
 		t.underlying = u
 		t.setState(underlying)
 	}
diff --git a/src/go/types/named.go b/src/go/types/named.go
index 79f6fc6a49..7f0e6f4904 100644
--- a/src/go/types/named.go
+++ b/src/go/types/named.go
@@ -633,6 +633,7 @@ func (n *Named) resolveUnderlying() {
 			t.resolve()
 			t.mu.Lock()
 			defer t.mu.Unlock()
+
 			assert(t.fromRHS != nil || t.allowNilRHS)
 			rhs = t.fromRHS
 
@@ -643,6 +644,12 @@ func (n *Named) resolveUnderlying() {
 
 	// set underlying for all Named types in the chain
 	for t := range seen {
+		// Careful, t.underlying has lock-free readers. Since we might be racing
+		// another call to resolveUnderlying, we have to avoid overwriting
+		// t.underlying. Otherwise, the race detector will be tripped.
+		if t.stateHas(underlying) {
+			continue
+		}
 		t.underlying = u
 		t.setState(underlying)
 	}