Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2025-12-08 06:10:04 +00:00
Code Issues Projects Releases Packages Wiki Activity

go/types, types2: guard Named.underlying with Named.mu

Browse source 
It appears that CL 695977 introduced a data race on Named.underlying.
This fixes that race by specifying a new namedState called underlying,
which, perhaps unsurprisingly, signals that Named.underlying is populated.

Unfortunately, the underlying namedState is independent of the complete
namedState (unsurprising since methods and the underlying type are not related).

Hence, they cannot be ordered and thus do not fit the current integer-based
state representation. To account for combinations of states, we introduce a
bit set representation for namedState instead. The namedState field is also
renamed to stateMask to reflect this new representation.

Methods that operate on the stateMask are adjusted and exposition is added
throughout.

Fixes #75963

Change-Id: Icfa188ea2fa7916804c06f80668e99176bf4e978
Reviewed-on: https://go-review.googlesource.com/c/go/+/712720
Auto-Submit: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This commit is contained in:
Mark Freeman 2025-10-17 13:16:34 -04:00 • committed by Gopher Robot
parent 4a0115c886
commit 39fd61ddb0
2 changed files with 184 additions and 90 deletions
Download patch file Download diff file Expand all files Collapse all files

137
src/cmd/compile/internal/types2/named.go
View file

@ -47,10 +47,8 @@ import (
// soon.
//
// We achieve this by tracking state with an atomic state variable, and
// guarding potentially concurrent calculations with a mutex. At any point in
// time this state variable determines which data on N may be accessed. As
// state monotonically progresses, any data available at state M may be
// accessed without acquiring the mutex at state N, provided N >= M.
// guarding potentially concurrent calculations with a mutex. See [stateMask]
// for details.
//
// GLOSSARY: Here are a few terms used in this file to describe Named types:
// - We say that a Named type is "instantiated" if it has been constructed by
@ -111,13 +109,13 @@ 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]
underlying Type // underlying type, or nil
inst *instance // information for instantiated types; nil otherwise
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
fromRHS Type // the declaration RHS this type is derived from
tparams *TypeParamList // type parameters, or nil
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
// methods declared for this type (not the method set of this type)
// Signatures are type-checked lazily.
@ -139,15 +137,43 @@ type instance struct {
ctxt *Context // local Context; set to nil after full expansion
}
// namedState represents the possible states that a named type may assume.
type namedState uint32
// stateMask represents each state in the lifecycle of a named type.
//
// Each named type begins in the unresolved state. A named type may transition to a new state
// according to the below diagram:
//
// unresolved
// loaded
// resolved
// └── complete
// └── underlying
//
// That is, descent down the tree is mostly linear (unresolved through resolved), except upon
// reaching the leaves (complete and underlying). 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.
//
// The above constraints significantly narrow the possible bit sets for a named type. With bits
// set left-to-right, they are:
//
// 0000 | unresolved
// 1000 | loaded
// 1100 | resolved, which implies loaded
// 1110 | completed, which implies resolved (which in turn implies loaded)
// 1101 | underlying, which implies resolved ...
// 1111 | both completed and underlying which implies resolved ...
//
// To read the state of a named type, use [Named.stateHas]; to write, use [Named.setState].
type stateMask uint32
// Note: the order of states is relevant
const (
unresolved namedState = iota // type parameters, RHS, underlying, and methods might be unavailable
resolved // resolve has run; methods might be unexpanded (for instances)
loaded // loader has run; constraints might be unexpanded (for generic types)
complete // all data is known
// before resolved, type parameters, RHS, underlying, and methods might be unavailable
resolved stateMask = 1 << iota // methods might be unexpanded (for instances)
complete // methods are all expanded (for instances)
loaded // methods are available, but constraints might be unexpanded (for generic types)
underlying // underlying type is available
)
// NewNamed returns a new named type for the given type name, underlying type, and associated methods.
@ -188,7 +214,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
// All others:
// Effectively, nothing happens.
func (n *Named) resolve() *Named {
if n.state() >= resolved { // avoid locking below
if n.stateHas(resolved | loaded) { // avoid locking below
return n
}
@ -197,10 +223,14 @@ func (n *Named) resolve() *Named {
n.mu.Lock()
defer n.mu.Unlock()
if n.state() >= resolved {
// only atomic for consistency; we are holding the mutex
if n.stateHas(resolved | loaded) {
return n
}
// underlying comes after resolving, do not set it
defer (func() { assert(!n.stateHas(underlying)) })()
if n.inst != nil {
assert(n.fromRHS == nil) // instantiated types are not declared types
assert(n.loader == nil) // cannot import an instantiation
@ -212,7 +242,7 @@ func (n *Named) resolve() *Named {
n.tparams = orig.tparams
if len(orig.methods) == 0 {
n.setState(complete) // nothing further to do
n.setState(resolved | complete) // nothing further to do
n.inst.ctxt = nil
} else {
n.setState(resolved)
@ -239,27 +269,24 @@ func (n *Named) resolve() *Named {
n.methods = methods
n.setState(loaded) // avoid deadlock calling delayed functions
for _, f := range delayed {
f()
}
}
assert(n.fromRHS != nil || n.allowNilRHS)
assert(n.underlying == nil) // underlying comes after resolving
n.setState(complete)
n.setState(resolved | complete)
return n
}
// state atomically accesses the current state of the receiver.
func (n *Named) state() namedState {
return namedState(atomic.LoadUint32(&n.state_))
// 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
}
// setState atomically stores the given state for n.
// 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(state namedState) {
atomic.StoreUint32(&n.state_, uint32(state))
func (n *Named) setState(sm stateMask) {
atomic.OrUint32(&n.state_, uint32(sm))
}
// newNamed is like NewNamed but with a *Checker receiver.
@ -369,7 +396,7 @@ func (t *Named) NumMethods() int {
func (t *Named) Method(i int) *Func {
t.resolve()
if t.state() >= complete {
if t.stateHas(complete) {
return t.methods[i]
}
@ -461,20 +488,25 @@ func (t *Named) expandMethod(i int) *Func {
// SetUnderlying sets the underlying type and marks t as complete.
// t must not have type arguments.
func (t *Named) SetUnderlying(underlying Type) {
func (t *Named) SetUnderlying(u Type) {
assert(t.inst == nil)
if underlying == nil {
if u == nil {
panic("underlying type must not be nil")
}
if asNamed(underlying) != nil {
if asNamed(u) != nil {
panic("underlying type must not be *Named")
}
// Invariant: Presence of underlying type implies it was resolved.
t.fromRHS = underlying
// be careful to uphold the state invariants
t.mu.Lock()
defer t.mu.Unlock()
t.fromRHS = u
t.allowNilRHS = false
t.resolve()
t.underlying = underlying
t.setState(resolved | complete) // TODO(markfreeman): Why complete?
t.underlying = u
t.allowNilUnderlying = false
t.setState(underlying)
}
// AddMethod adds method m unless it is already in the method list.
@ -553,9 +585,10 @@ func (t *Named) String() string { return TypeString(t, nil) }
// cycle is found, the result is Typ[Invalid]; if n.check != nil, the
// cycle is also reported.
func (n *Named) under() Type {
assert(n.state() >= resolved)
assert(n.stateHas(resolved))
if n.underlying != nil {
// optimization for likely case
if n.stateHas(underlying) {
return n.underlying
}
@ -569,19 +602,34 @@ func (n *Named) under() Type {
switch t := rhs.(type) {
case nil:
u = Typ[Invalid]
case *Alias:
rhs = unalias(t)
case *Named:
if i, ok := seen[t]; ok {
n.check.cycleError(path[i:], firstInSrc(path[i:]))
u = Typ[Invalid]
break
}
// acquire the lock without checking; performance is probably fine
t.resolve()
t.mu.Lock()
defer t.mu.Unlock()
// t.underlying might have been set while we were locking
if t.stateHas(underlying) {
u = t.underlying
break
}
seen[t] = len(seen)
path = append(path, t.obj)
t.resolve()
assert(t.fromRHS != nil || t.allowNilRHS)
rhs = t.fromRHS
default:
u = rhs // any type literal works
}
@ -590,6 +638,7 @@ func (n *Named) under() Type {
// go back up the chain
for t := range seen {
t.underlying = u
t.setState(underlying)
}
return u
@ -659,7 +708,8 @@ func (n *Named) expandRHS() (rhs Type) {
}()
}
assert(n.state() == unresolved)
assert(!n.stateHas(resolved))
assert(n.inst.orig.stateHas(resolved | loaded))
if n.inst.ctxt == nil {
n.inst.ctxt = NewContext()
@ -668,9 +718,6 @@ func (n *Named) expandRHS() (rhs Type) {
ctxt := n.inst.ctxt
orig := n.inst.orig
assert(orig.state() >= resolved)
assert(orig.fromRHS != nil)
targs := n.inst.targs
tpars := orig.tparams

137
src/go/types/named.go
View file

@ -50,10 +50,8 @@ import (
// soon.
//
// We achieve this by tracking state with an atomic state variable, and
// guarding potentially concurrent calculations with a mutex. At any point in
// time this state variable determines which data on N may be accessed. As
// state monotonically progresses, any data available at state M may be
// accessed without acquiring the mutex at state N, provided N >= M.
// guarding potentially concurrent calculations with a mutex. See [stateMask]
// for details.
//
// GLOSSARY: Here are a few terms used in this file to describe Named types:
// - We say that a Named type is "instantiated" if it has been constructed by
@ -114,13 +112,13 @@ 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]
underlying Type // underlying type, or nil
inst *instance // information for instantiated types; nil otherwise
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
fromRHS Type // the declaration RHS this type is derived from
tparams *TypeParamList // type parameters, or nil
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
// methods declared for this type (not the method set of this type)
// Signatures are type-checked lazily.
@ -142,15 +140,43 @@ type instance struct {
ctxt *Context // local Context; set to nil after full expansion
}
// namedState represents the possible states that a named type may assume.
type namedState uint32
// stateMask represents each state in the lifecycle of a named type.
//
// Each named type begins in the unresolved state. A named type may transition to a new state
// according to the below diagram:
//
// unresolved
// loaded
// resolved
// └── complete
// └── underlying
//
// That is, descent down the tree is mostly linear (unresolved through resolved), except upon
// reaching the leaves (complete and underlying). 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.
//
// The above constraints significantly narrow the possible bit sets for a named type. With bits
// set left-to-right, they are:
//
// 0000 | unresolved
// 1000 | loaded
// 1100 | resolved, which implies loaded
// 1110 | completed, which implies resolved (which in turn implies loaded)
// 1101 | underlying, which implies resolved ...
// 1111 | both completed and underlying which implies resolved ...
//
// To read the state of a named type, use [Named.stateHas]; to write, use [Named.setState].
type stateMask uint32
// Note: the order of states is relevant
const (
unresolved namedState = iota // type parameters, RHS, underlying, and methods might be unavailable
resolved // resolve has run; methods might be unexpanded (for instances)
loaded // loader has run; constraints might be unexpanded (for generic types)
complete // all data is known
// before resolved, type parameters, RHS, underlying, and methods might be unavailable
resolved stateMask = 1 << iota // methods might be unexpanded (for instances)
complete // methods are all expanded (for instances)
loaded // methods are available, but constraints might be unexpanded (for generic types)
underlying // underlying type is available
)
// NewNamed returns a new named type for the given type name, underlying type, and associated methods.
@ -191,7 +217,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
// All others:
// Effectively, nothing happens.
func (n *Named) resolve() *Named {
if n.state() >= resolved { // avoid locking below
if n.stateHas(resolved | loaded) { // avoid locking below
return n
}
@ -200,10 +226,14 @@ func (n *Named) resolve() *Named {
n.mu.Lock()
defer n.mu.Unlock()
if n.state() >= resolved {
// only atomic for consistency; we are holding the mutex
if n.stateHas(resolved | loaded) {
return n
}
// underlying comes after resolving, do not set it
defer (func() { assert(!n.stateHas(underlying)) })()
if n.inst != nil {
assert(n.fromRHS == nil) // instantiated types are not declared types
assert(n.loader == nil) // cannot import an instantiation
@ -215,7 +245,7 @@ func (n *Named) resolve() *Named {
n.tparams = orig.tparams
if len(orig.methods) == 0 {
n.setState(complete) // nothing further to do
n.setState(resolved | complete) // nothing further to do
n.inst.ctxt = nil
} else {
n.setState(resolved)
@ -242,27 +272,24 @@ func (n *Named) resolve() *Named {
n.methods = methods
n.setState(loaded) // avoid deadlock calling delayed functions
for _, f := range delayed {
f()
}
}
assert(n.fromRHS != nil || n.allowNilRHS)
assert(n.underlying == nil) // underlying comes after resolving
n.setState(complete)
n.setState(resolved | complete)
return n
}
// state atomically accesses the current state of the receiver.
func (n *Named) state() namedState {
return namedState(atomic.LoadUint32(&n.state_))
// 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
}
// setState atomically stores the given state for n.
// 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(state namedState) {
atomic.StoreUint32(&n.state_, uint32(state))
func (n *Named) setState(sm stateMask) {
atomic.OrUint32(&n.state_, uint32(sm))
}
// newNamed is like NewNamed but with a *Checker receiver.
@ -372,7 +399,7 @@ func (t *Named) NumMethods() int {
func (t *Named) Method(i int) *Func {
t.resolve()
if t.state() >= complete {
if t.stateHas(complete) {
return t.methods[i]
}
@ -464,20 +491,25 @@ func (t *Named) expandMethod(i int) *Func {
// SetUnderlying sets the underlying type and marks t as complete.
// t must not have type arguments.
func (t *Named) SetUnderlying(underlying Type) {
func (t *Named) SetUnderlying(u Type) {
assert(t.inst == nil)
if underlying == nil {
if u == nil {
panic("underlying type must not be nil")
}
if asNamed(underlying) != nil {
if asNamed(u) != nil {
panic("underlying type must not be *Named")
}
// Invariant: Presence of underlying type implies it was resolved.
t.fromRHS = underlying
// be careful to uphold the state invariants
t.mu.Lock()
defer t.mu.Unlock()
t.fromRHS = u
t.allowNilRHS = false
t.resolve()
t.underlying = underlying
t.setState(resolved | complete) // TODO(markfreeman): Why complete?
t.underlying = u
t.allowNilUnderlying = false
t.setState(underlying)
}
// AddMethod adds method m unless it is already in the method list.
@ -556,9 +588,10 @@ func (t *Named) String() string { return TypeString(t, nil) }
// cycle is found, the result is Typ[Invalid]; if n.check != nil, the
// cycle is also reported.
func (n *Named) under() Type {
assert(n.state() >= resolved)
assert(n.stateHas(resolved))
if n.underlying != nil {
// optimization for likely case
if n.stateHas(underlying) {
return n.underlying
}
@ -572,19 +605,34 @@ func (n *Named) under() Type {
switch t := rhs.(type) {
case nil:
u = Typ[Invalid]
case *Alias:
rhs = unalias(t)
case *Named:
if i, ok := seen[t]; ok {
n.check.cycleError(path[i:], firstInSrc(path[i:]))
u = Typ[Invalid]
break
}
// acquire the lock without checking; performance is probably fine
t.resolve()
t.mu.Lock()
defer t.mu.Unlock()
// t.underlying might have been set while we were locking
if t.stateHas(underlying) {
u = t.underlying
break
}
seen[t] = len(seen)
path = append(path, t.obj)
t.resolve()
assert(t.fromRHS != nil || t.allowNilRHS)
rhs = t.fromRHS
default:
u = rhs // any type literal works
}
@ -593,6 +641,7 @@ func (n *Named) under() Type {
// go back up the chain
for t := range seen {
t.underlying = u
t.setState(underlying)
}
return u
@ -662,7 +711,8 @@ func (n *Named) expandRHS() (rhs Type) {
}()
}
assert(n.state() == unresolved)
assert(!n.stateHas(resolved))
assert(n.inst.orig.stateHas(resolved | loaded))
if n.inst.ctxt == nil {
n.inst.ctxt = NewContext()
@ -671,9 +721,6 @@ func (n *Named) expandRHS() (rhs Type) {
ctxt := n.inst.ctxt
orig := n.inst.orig
assert(orig.state() >= resolved)
assert(orig.fromRHS != nil)
targs := n.inst.targs
tpars := orig.tparams