Skip to content

Commit

Permalink
go/analysis/passes/copylock: avoid infinite recursion via type params
Browse files Browse the repository at this point in the history
We should not infinitely expand type parameters looking for a lock path.

Fixes golang/go#49384

Change-Id: I8e1c952d468908aeb16751dddd6c05bb07f3c95c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/361456
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
  • Loading branch information
findleyr committed Nov 5, 2021
1 parent 1c6f3cc commit 4ab7496
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
28 changes: 20 additions & 8 deletions go/analysis/passes/copylock/copylock.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,15 @@ func checkCopyLocksCallExpr(pass *analysis.Pass, ce *ast.CallExpr) {
func checkCopyLocksFunc(pass *analysis.Pass, name string, recv *ast.FieldList, typ *ast.FuncType) {
if recv != nil && len(recv.List) > 0 {
expr := recv.List[0].Type
if path := lockPath(pass.Pkg, pass.TypesInfo.Types[expr].Type); path != nil {
if path := lockPath(pass.Pkg, pass.TypesInfo.Types[expr].Type, nil); path != nil {
pass.ReportRangef(expr, "%s passes lock by value: %v", name, path)
}
}

if typ.Params != nil {
for _, field := range typ.Params.List {
expr := field.Type
if path := lockPath(pass.Pkg, pass.TypesInfo.Types[expr].Type); path != nil {
if path := lockPath(pass.Pkg, pass.TypesInfo.Types[expr].Type, nil); path != nil {
pass.ReportRangef(expr, "%s passes lock by value: %v", name, path)
}
}
Expand Down Expand Up @@ -200,7 +200,7 @@ func checkCopyLocksRangeVar(pass *analysis.Pass, rtok token.Token, e ast.Expr) {
if typ == nil {
return
}
if path := lockPath(pass.Pkg, typ); path != nil {
if path := lockPath(pass.Pkg, typ, nil); path != nil {
pass.Reportf(e.Pos(), "range var %s copies lock: %v", analysisutil.Format(pass.Fset, e), path)
}
}
Expand Down Expand Up @@ -235,23 +235,35 @@ func lockPathRhs(pass *analysis.Pass, x ast.Expr) typePath {
return nil
}
}
return lockPath(pass.Pkg, pass.TypesInfo.Types[x].Type)
return lockPath(pass.Pkg, pass.TypesInfo.Types[x].Type, nil)
}

// lockPath returns a typePath describing the location of a lock value
// contained in typ. If there is no contained lock, it returns nil.
func lockPath(tpkg *types.Package, typ types.Type) typePath {
//
// The seenTParams map is used to short-circuit infinite recursion via type
// parameters.
func lockPath(tpkg *types.Package, typ types.Type, seenTParams map[*typeparams.TypeParam]bool) typePath {
if typ == nil {
return nil
}

if tpar, ok := typ.(*typeparams.TypeParam); ok {
if seenTParams == nil {
// Lazily allocate seenTParams, since the common case will not involve
// any type parameters.
seenTParams = make(map[*typeparams.TypeParam]bool)
}
if seenTParams[tpar] {
return nil
}
seenTParams[tpar] = true
terms, err := typeparams.StructuralTerms(tpar)
if err != nil {
return nil // invalid type
}
for _, term := range terms {
subpath := lockPath(tpkg, term.Type())
subpath := lockPath(tpkg, term.Type(), seenTParams)
if len(subpath) > 0 {
if term.Tilde() {
// Prepend a tilde to our lock path entry to clarify the resulting
Expand Down Expand Up @@ -285,7 +297,7 @@ func lockPath(tpkg *types.Package, typ types.Type) typePath {
ttyp, ok := typ.Underlying().(*types.Tuple)
if ok {
for i := 0; i < ttyp.Len(); i++ {
subpath := lockPath(tpkg, ttyp.At(i).Type())
subpath := lockPath(tpkg, ttyp.At(i).Type(), seenTParams)
if subpath != nil {
return append(subpath, typ.String())
}
Expand Down Expand Up @@ -319,7 +331,7 @@ func lockPath(tpkg *types.Package, typ types.Type) typePath {
nfields := styp.NumFields()
for i := 0; i < nfields; i++ {
ftyp := styp.Field(i).Type()
subpath := lockPath(tpkg, ftyp)
subpath := lockPath(tpkg, ftyp, seenTParams)
if subpath != nil {
return append(subpath, typ.String())
}
Expand Down
11 changes: 11 additions & 0 deletions go/analysis/passes/copylock/testdata/src/typeparams/typeparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ package typeparams

import "sync"

// The copylock analyzer runs despite errors. The following invalid type should
// not cause an infinite recursion.
type R struct{ r R }

func TestNoRecursion(r R) {}

// The following recursive type parameter definitions should not cause an
// infinite recursion.
func TestNoTypeParamRecursion[T1 ~[]T2, T2 ~[]T1 | string, T3 ~struct{ F T3 }](t1 T1, t2 T2, t3 T3) {
}

func OkFunc1[Struct ~*struct{ mu sync.Mutex }](s Struct) {
}

Expand Down

0 comments on commit 4ab7496

Please sign in to comment.