From: Mark Freeman Date: Wed, 10 Dec 2025 18:29:01 +0000 (-0500) Subject: go/types, types2: put Named.finite behind Named.mu X-Git-Tag: go1.26rc1~1^2~31 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=72c83bcc80;p=gostls13.git go/types, types2: put Named.finite behind Named.mu This change adds another leaf state to named types which indicates whether the type's size finiteness is known. Without this, writes to Named.finite can result in a clobbered value. While benign in terms of functionality, it triggers the race detector. Fixes #76773 Change-Id: I2ac3d8d6f8be55a8120598daecb3e78aa7df5f30 Reviewed-on: https://go-review.googlesource.com/c/go/+/729021 Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI Auto-Submit: Mark Freeman --- diff --git a/src/cmd/compile/internal/types2/cycles.go b/src/cmd/compile/internal/types2/cycles.go index f6721fa593..09229617f0 100644 --- a/src/cmd/compile/internal/types2/cycles.go +++ b/src/cmd/compile/internal/types2/cycles.go @@ -106,17 +106,13 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) { // TODO(markfreeman): Can the value cached on Named be used in validType / hasVarSize? // finiteSize returns whether a type has finite size. -func (check *Checker) finiteSize(t Type) (b bool) { +func (check *Checker) finiteSize(t Type) bool { switch t := Unalias(t).(type) { case *Named: - if t.finite != nil { + if t.stateHas(hasFinite) { return *t.finite } - defer func() { - t.finite = &b - }() - if i, ok := check.objPathIdx[t.obj]; ok { cycle := check.objPath[i:] check.cycleError(cycle, firstInSrc(cycle)) @@ -125,7 +121,19 @@ func (check *Checker) finiteSize(t Type) (b bool) { check.push(t.obj) defer check.pop() - return check.finiteSize(t.fromRHS) + isFinite := check.finiteSize(t.fromRHS) + + t.mu.Lock() + defer t.mu.Unlock() + // Careful, t.finite has lock-free readers. Since we might be racing + // another call to finiteSize, we have to avoid overwriting t.finite. + // Otherwise, the race detector will be tripped. + if !t.stateHas(hasFinite) { + t.finite = &isFinite + t.setState(hasFinite) + } + + return isFinite case *Array: // The array length is already computed. If it was a valid length, it diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index 6d5ff976c3..6b31d6d5df 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -110,14 +110,14 @@ type Named struct { allowNilRHS bool // same as below, as well as briefly during checking of a type declaration allowNilUnderlying bool // may be true from creation via [NewNamed] until [Named.SetUnderlying] - inst *instance // information for instantiated types; nil otherwise - finite *bool // whether the type has finite size, or nil + inst *instance // information for instantiated types; nil otherwise mu sync.Mutex // guards all fields below state_ uint32 // the current state of this type; must only be accessed atomically or when mu is held fromRHS Type // the declaration RHS this type is derived from tparams *TypeParamList // type parameters, or nil underlying Type // underlying type, or nil + finite *bool // whether the type has finite size, or nil // methods declared for this type (not the method set of this type) // Signatures are type-checked lazily. @@ -149,10 +149,11 @@ type instance struct { // unpacked // └── hasMethods // └── hasUnder +// └── hasFinite // // That is, descent down the tree is mostly linear (initial through unpacked), except upon -// reaching the leaves (hasMethods and hasUnder). A type may occupy any combination of the -// leaf states at once (they are independent states). +// reaching the leaves (hasMethods, hasUnder, and hasFinite). A type may occupy any +// combination of the leaf states at once (they are independent states). // // To represent this independence, the set of active states is represented with a bit set. State // transitions are monotonic. Once a state bit is set, it remains set. @@ -160,12 +161,14 @@ type instance struct { // The above constraints significantly narrow the possible bit sets for a named type. With bits // set left-to-right, they are: // -// 0000 | initial -// 1000 | lazyLoaded -// 1100 | unpacked, which implies lazyLoaded -// 1110 | hasMethods, which implies unpacked (which in turn implies lazyLoaded) -// 1101 | hasUnder, which implies unpacked ... -// 1111 | both hasMethods and hasUnder which implies unpacked ... +// 00000 | initial +// 10000 | lazyLoaded +// 11000 | unpacked, which implies lazyLoaded +// 11100 | hasMethods, which implies unpacked (which in turn implies lazyLoaded) +// 11010 | hasUnder, which implies unpacked ... +// 11001 | hasFinite, which implies unpacked ... +// 11110 | both hasMethods and hasUnder which implies unpacked ... +// ... | (other combinations of leaf states) // // To read the state of a named type, use [Named.stateHas]; to write, use [Named.setState]. type stateMask uint32 @@ -176,6 +179,7 @@ const ( unpacked // methods might be unexpanded (for instances) hasMethods // methods are all expanded (for instances) hasUnder // underlying type is available + hasFinite // size finiteness is available ) // NewNamed returns a new named type for the given type name, underlying type, and associated methods. @@ -305,6 +309,10 @@ func (n *Named) setState(m stateMask) { if m&hasUnder != 0 { assert(u) } + // hasFinite => unpacked + if m&hasFinite != 0 { + assert(u) + } } } diff --git a/src/go/types/cycles.go b/src/go/types/cycles.go index ca4d5f7744..68868fcc76 100644 --- a/src/go/types/cycles.go +++ b/src/go/types/cycles.go @@ -109,17 +109,13 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) { // TODO(markfreeman): Can the value cached on Named be used in validType / hasVarSize? // finiteSize returns whether a type has finite size. -func (check *Checker) finiteSize(t Type) (b bool) { +func (check *Checker) finiteSize(t Type) bool { switch t := Unalias(t).(type) { case *Named: - if t.finite != nil { + if t.stateHas(hasFinite) { return *t.finite } - defer func() { - t.finite = &b - }() - if i, ok := check.objPathIdx[t.obj]; ok { cycle := check.objPath[i:] check.cycleError(cycle, firstInSrc(cycle)) @@ -128,7 +124,19 @@ func (check *Checker) finiteSize(t Type) (b bool) { check.push(t.obj) defer check.pop() - return check.finiteSize(t.fromRHS) + isFinite := check.finiteSize(t.fromRHS) + + t.mu.Lock() + defer t.mu.Unlock() + // Careful, t.finite has lock-free readers. Since we might be racing + // another call to finiteSize, we have to avoid overwriting t.finite. + // Otherwise, the race detector will be tripped. + if !t.stateHas(hasFinite) { + t.finite = &isFinite + t.setState(hasFinite) + } + + return isFinite case *Array: // The array length is already computed. If it was a valid length, it diff --git a/src/go/types/named.go b/src/go/types/named.go index ba0f92015d..5925a40c00 100644 --- a/src/go/types/named.go +++ b/src/go/types/named.go @@ -113,14 +113,14 @@ type Named struct { allowNilRHS bool // same as below, as well as briefly during checking of a type declaration allowNilUnderlying bool // may be true from creation via [NewNamed] until [Named.SetUnderlying] - inst *instance // information for instantiated types; nil otherwise - finite *bool // whether the type has finite size, or nil + inst *instance // information for instantiated types; nil otherwise mu sync.Mutex // guards all fields below state_ uint32 // the current state of this type; must only be accessed atomically or when mu is held fromRHS Type // the declaration RHS this type is derived from tparams *TypeParamList // type parameters, or nil underlying Type // underlying type, or nil + finite *bool // whether the type has finite size, or nil // methods declared for this type (not the method set of this type) // Signatures are type-checked lazily. @@ -152,10 +152,11 @@ type instance struct { // unpacked // └── hasMethods // └── hasUnder +// └── hasFinite // // That is, descent down the tree is mostly linear (initial through unpacked), except upon -// reaching the leaves (hasMethods and hasUnder). A type may occupy any combination of the -// leaf states at once (they are independent states). +// reaching the leaves (hasMethods, hasUnder, and hasFinite). A type may occupy any +// combination of the leaf states at once (they are independent states). // // To represent this independence, the set of active states is represented with a bit set. State // transitions are monotonic. Once a state bit is set, it remains set. @@ -163,12 +164,14 @@ type instance struct { // The above constraints significantly narrow the possible bit sets for a named type. With bits // set left-to-right, they are: // -// 0000 | initial -// 1000 | lazyLoaded -// 1100 | unpacked, which implies lazyLoaded -// 1110 | hasMethods, which implies unpacked (which in turn implies lazyLoaded) -// 1101 | hasUnder, which implies unpacked ... -// 1111 | both hasMethods and hasUnder which implies unpacked ... +// 00000 | initial +// 10000 | lazyLoaded +// 11000 | unpacked, which implies lazyLoaded +// 11100 | hasMethods, which implies unpacked (which in turn implies lazyLoaded) +// 11010 | hasUnder, which implies unpacked ... +// 11001 | hasFinite, which implies unpacked ... +// 11110 | both hasMethods and hasUnder which implies unpacked ... +// ... | (other combinations of leaf states) // // To read the state of a named type, use [Named.stateHas]; to write, use [Named.setState]. type stateMask uint32 @@ -179,6 +182,7 @@ const ( unpacked // methods might be unexpanded (for instances) hasMethods // methods are all expanded (for instances) hasUnder // underlying type is available + hasFinite // size finiteness is available ) // NewNamed returns a new named type for the given type name, underlying type, and associated methods. @@ -308,6 +312,10 @@ func (n *Named) setState(m stateMask) { if m&hasUnder != 0 { assert(u) } + // hasFinite => unpacked + if m&hasFinite != 0 { + assert(u) + } } }