From: Mark Freeman Date: Fri, 13 Jun 2025 15:47:57 +0000 (-0400) Subject: cmd/compile/internal/types2: add loaded state between loader calls and constraint... X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=7b53d8d06ec432b1434b675b96a11526de6e6abe;p=gostls13.git cmd/compile/internal/types2: add loaded state between loader calls and constraint expansion There is a deadlock issue when calling SetConstraint from a lazy loader because the loader is called from resolve(), which is holding a lock on the loaded type. If the loaded type has a generic constraint which refers back to the loaded type (such as an argument or result), then we will loop back to the loaded type and deadlock. This change postpones calls to SetConstraint and passes them back to resolve(). At that point, the loaded type is mostly constructed, but its constraints might be unexpanded. Similar to how we handle resolved instances, we advance the state for the loaded type to a, appropriately named, loaded state. When we expand the constraint, we don't try to acquire the lock on the loaded type. Thus, no deadlock. Fixes #63285 Change-Id: Ie0204b58a5b433f6d839ce8fd8a99542246367b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/681875 Commit-Queue: Mark Freeman Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI --- diff --git a/src/cmd/compile/internal/importer/gcimporter_test.go b/src/cmd/compile/internal/importer/gcimporter_test.go index 965c5d1a84..11e4ee6b58 100644 --- a/src/cmd/compile/internal/importer/gcimporter_test.go +++ b/src/cmd/compile/internal/importer/gcimporter_test.go @@ -673,3 +673,50 @@ type S struct { } wg.Wait() } + +func TestIssue63285(t *testing.T) { + testenv.MustHaveGoBuild(t) + + // This package only handles gc export data. + if runtime.Compiler != "gc" { + t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler) + } + + tmpdir := t.TempDir() + testoutdir := filepath.Join(tmpdir, "testdata") + if err := os.Mkdir(testoutdir, 0700); err != nil { + t.Fatalf("making output dir: %v", err) + } + + compile(t, "testdata", "issue63285.go", testoutdir, nil) + + issue63285, err := Import(make(map[string]*types2.Package), "./testdata/issue63285", tmpdir, nil) + if err != nil { + t.Fatal(err) + } + + check := func(pkgname, src string, imports importMap) (*types2.Package, error) { + f, err := syntax.Parse(syntax.NewFileBase(pkgname), strings.NewReader(src), nil, nil, 0) + if err != nil { + return nil, err + } + config := &types2.Config{ + Importer: imports, + } + return config.Check(pkgname, []*syntax.File{f}, nil) + } + + const pSrc = `package p + +import "issue63285" + +var _ issue63285.A[issue63285.B[any]] +` + + importer := importMap{ + "issue63285": issue63285, + } + if _, err := check("p", pSrc, importer); err != nil { + t.Errorf("Check failed: %v", err) + } +} diff --git a/src/cmd/compile/internal/importer/testdata/issue63285.go b/src/cmd/compile/internal/importer/testdata/issue63285.go new file mode 100644 index 0000000000..54a06b7b88 --- /dev/null +++ b/src/cmd/compile/internal/importer/testdata/issue63285.go @@ -0,0 +1,11 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package issue63285 + +type A[_ B[any]] struct{} + +type B[_ any] interface { + f() A[B[any]] +} diff --git a/src/cmd/compile/internal/importer/ureader.go b/src/cmd/compile/internal/importer/ureader.go index 6012d283ac..f6df56b70e 100644 --- a/src/cmd/compile/internal/importer/ureader.go +++ b/src/cmd/compile/internal/importer/ureader.go @@ -67,7 +67,8 @@ type reader struct { p *pkgReader - dict *readerDict + dict *readerDict + delayed []func() } type readerDict struct { @@ -420,7 +421,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types2.Package, string) { pos := r.pos() var tparams []*types2.TypeParam if r.Version().Has(pkgbits.AliasTypeParamNames) { - tparams = r.typeParamNames() + tparams = r.typeParamNames(false) } typ := r.typ() return newAliasTypeName(pr.enableAlias, pos, objPkg, objName, typ, tparams) @@ -433,28 +434,28 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types2.Package, string) { case pkgbits.ObjFunc: pos := r.pos() - tparams := r.typeParamNames() + tparams := r.typeParamNames(false) sig := r.signature(nil, nil, tparams) return types2.NewFunc(pos, objPkg, objName, sig) case pkgbits.ObjType: pos := r.pos() - return types2.NewTypeNameLazy(pos, objPkg, objName, func(named *types2.Named) (tparams []*types2.TypeParam, underlying types2.Type, methods []*types2.Func) { - tparams = r.typeParamNames() + return types2.NewTypeNameLazy(pos, objPkg, objName, func(_ *types2.Named) ([]*types2.TypeParam, types2.Type, []*types2.Func, []func()) { + tparams := r.typeParamNames(true) // TODO(mdempsky): Rewrite receiver types to underlying is an // Interface? The go/types importer does this (I think because // unit tests expected that), but cmd/compile doesn't care // about it, so maybe we can avoid worrying about that here. - underlying = r.typ().Underlying() + underlying := r.typ().Underlying() - methods = make([]*types2.Func, r.Len()) + methods := make([]*types2.Func, r.Len()) for i := range methods { - methods[i] = r.method() + methods[i] = r.method(true) } - return + return tparams, underlying, methods, r.delayed }) case pkgbits.ObjVar: @@ -497,7 +498,7 @@ func (pr *pkgReader) objDictIdx(idx pkgbits.Index) *readerDict { return &dict } -func (r *reader) typeParamNames() []*types2.TypeParam { +func (r *reader) typeParamNames(isLazy bool) []*types2.TypeParam { r.Sync(pkgbits.SyncTypeParamNames) // Note: This code assumes it only processes objects without @@ -523,19 +524,38 @@ func (r *reader) typeParamNames() []*types2.TypeParam { r.dict.tparams[i] = types2.NewTypeParam(tname, nil) } - for i, bound := range r.dict.bounds { - r.dict.tparams[i].SetConstraint(r.p.typIdx(bound, r.dict)) + // Type parameters that are read by lazy loaders cannot have their + // constraints set eagerly; do them after loading (go.dev/issue/63285). + if isLazy { + // The reader dictionary will continue mutating before we have time + // to call delayed functions; must make a local copy of both the type + // parameters and their (unexpanded) constraints. + bounds := make([]types2.Type, len(r.dict.bounds)) + for i, bound := range r.dict.bounds { + bounds[i] = r.p.typIdx(bound, r.dict) + } + + tparams := r.dict.tparams + r.delayed = append(r.delayed, func() { + for i, bound := range bounds { + tparams[i].SetConstraint(bound) + } + }) + } else { + for i, bound := range r.dict.bounds { + r.dict.tparams[i].SetConstraint(r.p.typIdx(bound, r.dict)) + } } return r.dict.tparams } -func (r *reader) method() *types2.Func { +func (r *reader) method(isLazy bool) *types2.Func { r.Sync(pkgbits.SyncMethod) pos := r.pos() pkg, name := r.selector() - rtparams := r.typeParamNames() + rtparams := r.typeParamNames(isLazy) sig := r.signature(r.param(), rtparams, nil) _ = r.pos() // TODO(mdempsky): Remove; this is a hacker for linker.go. diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index a9a27c9320..dbb1fa0b3e 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -127,8 +127,8 @@ type Named struct { // accessed. methods []*Func - // loader may be provided to lazily load type parameters, underlying type, and methods. - loader func(*Named) (tparams []*TypeParam, underlying Type, methods []*Func) + // loader may be provided to lazily load type parameters, underlying type, methods, and delayed functions + loader func(*Named) ([]*TypeParam, Type, []*Func, []func()) } // instance holds information that is only necessary for instantiated named @@ -143,9 +143,11 @@ type instance struct { // namedState represents the possible states that a named type may assume. type namedState uint32 +// Note: the order of states is relevant const ( unresolved namedState = iota // tparams, underlying type and methods might be unavailable - resolved // resolve has run; methods might be incomplete (for instances) + 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 ) @@ -167,7 +169,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { // accessible; but if n is an instantiated type, its methods may still be // unexpanded. func (n *Named) resolve() *Named { - if n.state() >= resolved { // avoid locking below + if n.state() > unresolved { // avoid locking below return n } @@ -176,7 +178,7 @@ func (n *Named) resolve() *Named { n.mu.Lock() defer n.mu.Unlock() - if n.state() >= resolved { + if n.state() > unresolved { return n } @@ -212,13 +214,20 @@ func (n *Named) resolve() *Named { assert(n.underlying == nil) assert(n.TypeArgs().Len() == 0) // instances are created by instantiation, in which case n.loader is nil - tparams, underlying, methods := n.loader(n) + tparams, underlying, methods, delayed := n.loader(n) + n.loader = nil n.tparams = bindTParams(tparams) n.underlying = underlying n.fromRHS = underlying // for cycle detection n.methods = methods - n.loader = nil + + // advance state to avoid deadlock calling delayed functions + n.setState(loaded) + + for _, f := range delayed { + f() + } } n.setState(complete) diff --git a/src/cmd/compile/internal/types2/object.go b/src/cmd/compile/internal/types2/object.go index 26752c44b0..7096c55697 100644 --- a/src/cmd/compile/internal/types2/object.go +++ b/src/cmd/compile/internal/types2/object.go @@ -293,7 +293,7 @@ func NewTypeName(pos syntax.Pos, pkg *Package, name string, typ Type) *TypeName // NewTypeNameLazy returns a new defined type like NewTypeName, but it // lazily calls resolve to finish constructing the Named object. -func NewTypeNameLazy(pos syntax.Pos, pkg *Package, name string, load func(named *Named) (tparams []*TypeParam, underlying Type, methods []*Func)) *TypeName { +func NewTypeNameLazy(pos syntax.Pos, pkg *Package, name string, load func(*Named) ([]*TypeParam, Type, []*Func, []func())) *TypeName { obj := NewTypeName(pos, pkg, name, nil) NewNamed(obj, nil, nil).loader = load return obj diff --git a/src/go/types/named.go b/src/go/types/named.go index 1282abfa3f..72fd4970bb 100644 --- a/src/go/types/named.go +++ b/src/go/types/named.go @@ -130,8 +130,8 @@ type Named struct { // accessed. methods []*Func - // loader may be provided to lazily load type parameters, underlying type, and methods. - loader func(*Named) (tparams []*TypeParam, underlying Type, methods []*Func) + // loader may be provided to lazily load type parameters, underlying type, methods, and delayed functions + loader func(*Named) ([]*TypeParam, Type, []*Func, []func()) } // instance holds information that is only necessary for instantiated named @@ -146,9 +146,11 @@ type instance struct { // namedState represents the possible states that a named type may assume. type namedState uint32 +// Note: the order of states is relevant const ( unresolved namedState = iota // tparams, underlying type and methods might be unavailable - resolved // resolve has run; methods might be incomplete (for instances) + 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 ) @@ -170,7 +172,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { // accessible; but if n is an instantiated type, its methods may still be // unexpanded. func (n *Named) resolve() *Named { - if n.state() >= resolved { // avoid locking below + if n.state() > unresolved { // avoid locking below return n } @@ -179,7 +181,7 @@ func (n *Named) resolve() *Named { n.mu.Lock() defer n.mu.Unlock() - if n.state() >= resolved { + if n.state() > unresolved { return n } @@ -215,13 +217,20 @@ func (n *Named) resolve() *Named { assert(n.underlying == nil) assert(n.TypeArgs().Len() == 0) // instances are created by instantiation, in which case n.loader is nil - tparams, underlying, methods := n.loader(n) + tparams, underlying, methods, delayed := n.loader(n) + n.loader = nil n.tparams = bindTParams(tparams) n.underlying = underlying n.fromRHS = underlying // for cycle detection n.methods = methods - n.loader = nil + + // advance state to avoid deadlock calling delayed functions + n.setState(loaded) + + for _, f := range delayed { + f() + } } n.setState(complete) diff --git a/src/go/types/object.go b/src/go/types/object.go index aa7dcb835c..823c03c7fd 100644 --- a/src/go/types/object.go +++ b/src/go/types/object.go @@ -296,7 +296,7 @@ func NewTypeName(pos token.Pos, pkg *Package, name string, typ Type) *TypeName { // NewTypeNameLazy returns a new defined type like NewTypeName, but it // lazily calls resolve to finish constructing the Named object. -func _NewTypeNameLazy(pos token.Pos, pkg *Package, name string, load func(named *Named) (tparams []*TypeParam, underlying Type, methods []*Func)) *TypeName { +func _NewTypeNameLazy(pos token.Pos, pkg *Package, name string, load func(*Named) ([]*TypeParam, Type, []*Func, []func())) *TypeName { obj := NewTypeName(pos, pkg, name, nil) NewNamed(obj, nil, nil).loader = load return obj