]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: put Named.finite behind Named.mu
authorMark Freeman <mark@golang.org>
Wed, 10 Dec 2025 18:29:01 +0000 (13:29 -0500)
committerGopher Robot <gobot@golang.org>
Thu, 11 Dec 2025 16:18:30 +0000 (08:18 -0800)
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 <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>

src/cmd/compile/internal/types2/cycles.go
src/cmd/compile/internal/types2/named.go
src/go/types/cycles.go
src/go/types/named.go

index f6721fa593f2efc273b5b1e5b617907c3716513a..09229617f0442098e45198e7a14e25f6430064ac 100644 (file)
@@ -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
index 6d5ff976c35a157615cbc5e4cddf283147d08689..6b31d6d5df981bd5d94a105bf313fb060b83f600 100644 (file)
@@ -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)
+               }
        }
 }
 
index ca4d5f7744ce577c97083f300f55bb42c4f50ed7..68868fcc76277e3352a5190f81852ba2df0500e1 100644 (file)
@@ -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
index ba0f92015d4715aa7fc46f4af51f0165849b55c9..5925a40c00afa09ad797d00022b4ebdc2389b1e7 100644 (file)
@@ -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)
+               }
        }
 }