From: Mark Freeman Date: Wed, 22 Oct 2025 18:04:09 +0000 (-0400) Subject: go/types, types2: verify stateMask transitions in debug mode X-Git-Tag: go1.26rc1~469 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b2af92270f5e19c759b94912470a32b5e44a5b2e;p=gostls13.git go/types, types2: verify stateMask transitions in debug mode Recently, we've changed the representation of Named type state from an integer to a bit mask, which is a bit more complicated. To make sure we uphold state invariants, we are adding a verification step on each state transition. This uncovered a few places where we do not uphold the transition invariants; those are patched in this CL. Change-Id: I76569e4326b2d362d7a1f078641029ffb3dca531 Reviewed-on: https://go-review.googlesource.com/c/go/+/714241 Auto-Submit: Mark Freeman Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI --- diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index 9f99c568d7..11d7ba1734 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -215,7 +215,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { // All others: // Effectively, nothing happens. func (n *Named) unpack() *Named { - if n.stateHas(unpacked | lazyLoaded) { // avoid locking below + if n.stateHas(lazyLoaded | unpacked) { // avoid locking below return n } @@ -225,7 +225,7 @@ func (n *Named) unpack() *Named { defer n.mu.Unlock() // only atomic for consistency; we are holding the mutex - if n.stateHas(unpacked | lazyLoaded) { + if n.stateHas(lazyLoaded | unpacked) { return n } @@ -243,10 +243,10 @@ func (n *Named) unpack() *Named { n.tparams = orig.tparams if len(orig.methods) == 0 { - n.setState(unpacked | hasMethods) // nothing further to do + n.setState(lazyLoaded | unpacked | hasMethods) // nothing further to do n.inst.ctxt = nil } else { - n.setState(unpacked) + n.setState(lazyLoaded | unpacked) } return n } @@ -275,19 +275,36 @@ func (n *Named) unpack() *Named { } } - n.setState(unpacked | hasMethods) + n.setState(lazyLoaded | unpacked | hasMethods) return n } // stateHas atomically determines whether the current state includes any active bit in sm. -func (n *Named) stateHas(sm stateMask) bool { - return atomic.LoadUint32(&n.state_)&uint32(sm) != 0 +func (n *Named) stateHas(m stateMask) bool { + return stateMask(atomic.LoadUint32(&n.state_))&m != 0 } // setState atomically sets the current state to include each active bit in sm. // Must only be called while holding n.mu. -func (n *Named) setState(sm stateMask) { - atomic.OrUint32(&n.state_, uint32(sm)) +func (n *Named) setState(m stateMask) { + atomic.OrUint32(&n.state_, uint32(m)) + // verify state transitions + if debug { + m := stateMask(atomic.LoadUint32(&n.state_)) + u := m&unpacked != 0 + // unpacked => lazyLoaded + if u { + assert(m&lazyLoaded != 0) + } + // hasMethods => unpacked + if m&hasMethods != 0 { + assert(u) + } + // hasUnder => unpacked + if m&hasUnder != 0 { + assert(u) + } + } } // newNamed is like NewNamed but with a *Checker receiver. @@ -503,7 +520,7 @@ func (t *Named) SetUnderlying(u Type) { t.fromRHS = u t.allowNilRHS = false - t.setState(unpacked | hasMethods) // TODO(markfreeman): Why hasMethods? + t.setState(lazyLoaded | unpacked | hasMethods) // TODO(markfreeman): Why hasMethods? t.underlying = u t.allowNilUnderlying = false diff --git a/src/go/types/named.go b/src/go/types/named.go index 7b9856155f..564c1be3e0 100644 --- a/src/go/types/named.go +++ b/src/go/types/named.go @@ -218,7 +218,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { // All others: // Effectively, nothing happens. func (n *Named) unpack() *Named { - if n.stateHas(unpacked | lazyLoaded) { // avoid locking below + if n.stateHas(lazyLoaded | unpacked) { // avoid locking below return n } @@ -228,7 +228,7 @@ func (n *Named) unpack() *Named { defer n.mu.Unlock() // only atomic for consistency; we are holding the mutex - if n.stateHas(unpacked | lazyLoaded) { + if n.stateHas(lazyLoaded | unpacked) { return n } @@ -246,10 +246,10 @@ func (n *Named) unpack() *Named { n.tparams = orig.tparams if len(orig.methods) == 0 { - n.setState(unpacked | hasMethods) // nothing further to do + n.setState(lazyLoaded | unpacked | hasMethods) // nothing further to do n.inst.ctxt = nil } else { - n.setState(unpacked) + n.setState(lazyLoaded | unpacked) } return n } @@ -278,19 +278,36 @@ func (n *Named) unpack() *Named { } } - n.setState(unpacked | hasMethods) + n.setState(lazyLoaded | unpacked | hasMethods) return n } // stateHas atomically determines whether the current state includes any active bit in sm. -func (n *Named) stateHas(sm stateMask) bool { - return atomic.LoadUint32(&n.state_)&uint32(sm) != 0 +func (n *Named) stateHas(m stateMask) bool { + return stateMask(atomic.LoadUint32(&n.state_))&m != 0 } // setState atomically sets the current state to include each active bit in sm. // Must only be called while holding n.mu. -func (n *Named) setState(sm stateMask) { - atomic.OrUint32(&n.state_, uint32(sm)) +func (n *Named) setState(m stateMask) { + atomic.OrUint32(&n.state_, uint32(m)) + // verify state transitions + if debug { + m := stateMask(atomic.LoadUint32(&n.state_)) + u := m&unpacked != 0 + // unpacked => lazyLoaded + if u { + assert(m&lazyLoaded != 0) + } + // hasMethods => unpacked + if m&hasMethods != 0 { + assert(u) + } + // hasUnder => unpacked + if m&hasUnder != 0 { + assert(u) + } + } } // newNamed is like NewNamed but with a *Checker receiver. @@ -506,7 +523,7 @@ func (t *Named) SetUnderlying(u Type) { t.fromRHS = u t.allowNilRHS = false - t.setState(unpacked | hasMethods) // TODO(markfreeman): Why hasMethods? + t.setState(lazyLoaded | unpacked | hasMethods) // TODO(markfreeman): Why hasMethods? t.underlying = u t.allowNilUnderlying = false