go/analysis/passes/copylock: split out of vet
Change-Id: Ib7c735e714d8b5436d055bf99ca90e840c963c7b
Reviewed-on: https://go-review.googlesource.com/c/140740
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index d2340ca..6518f38 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -135,9 +135,11 @@
if err != nil {
return nil, err
}
- if packages.PrintErrors(pkgs) > 0 {
- return nil, fmt.Errorf("loading %s failed", pkgpath)
- }
+
+ // Print errors but do not stop:
+ // some Analyzers may be disposed to RunDespiteErrors
+ packages.PrintErrors(pkgs)
+
if len(pkgs) != 1 {
return nil, fmt.Errorf("pattern %q expanded to %d packages, want 1",
pkgpath, len(pkgs))
diff --git a/go/analysis/passes/copylock/copylock.go b/go/analysis/passes/copylock/copylock.go
new file mode 100644
index 0000000..e684cd3
--- /dev/null
+++ b/go/analysis/passes/copylock/copylock.go
@@ -0,0 +1,283 @@
+// Copyright 2013 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 copylock
+
+import (
+ "bytes"
+ "fmt"
+ "go/ast"
+ "go/token"
+ "go/types"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/inspector"
+)
+
+var Analyzer = &analysis.Analyzer{
+ Name: "copylocks",
+ Doc: "check for locks erroneously passed by value",
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ RunDespiteErrors: true,
+ Run: run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+ nodeFilter := []ast.Node{
+ (*ast.AssignStmt)(nil),
+ (*ast.CallExpr)(nil),
+ (*ast.CompositeLit)(nil),
+ (*ast.FuncDecl)(nil),
+ (*ast.FuncLit)(nil),
+ (*ast.GenDecl)(nil),
+ (*ast.RangeStmt)(nil),
+ (*ast.ReturnStmt)(nil),
+ }
+ inspect.Preorder(nodeFilter, func(node ast.Node) {
+ switch node := node.(type) {
+ case *ast.RangeStmt:
+ checkCopyLocksRange(pass, node)
+ case *ast.FuncDecl:
+ checkCopyLocksFunc(pass, node.Name.Name, node.Recv, node.Type)
+ case *ast.FuncLit:
+ checkCopyLocksFunc(pass, "func", nil, node.Type)
+ case *ast.CallExpr:
+ checkCopyLocksCallExpr(pass, node)
+ case *ast.AssignStmt:
+ checkCopyLocksAssign(pass, node)
+ case *ast.GenDecl:
+ checkCopyLocksGenDecl(pass, node)
+ case *ast.CompositeLit:
+ checkCopyLocksCompositeLit(pass, node)
+ case *ast.ReturnStmt:
+ checkCopyLocksReturnStmt(pass, node)
+ }
+ })
+ return nil, nil
+}
+
+// checkCopyLocksAssign checks whether an assignment
+// copies a lock.
+func checkCopyLocksAssign(pass *analysis.Pass, as *ast.AssignStmt) {
+ for i, x := range as.Rhs {
+ if path := lockPathRhs(pass, x); path != nil {
+ pass.Reportf(x.Pos(), "assignment copies lock value to %v: %v", analysisutil.Format(pass.Fset, as.Lhs[i]), path)
+ }
+ }
+}
+
+// checkCopyLocksGenDecl checks whether lock is copied
+// in variable declaration.
+func checkCopyLocksGenDecl(pass *analysis.Pass, gd *ast.GenDecl) {
+ if gd.Tok != token.VAR {
+ return
+ }
+ for _, spec := range gd.Specs {
+ valueSpec := spec.(*ast.ValueSpec)
+ for i, x := range valueSpec.Values {
+ if path := lockPathRhs(pass, x); path != nil {
+ pass.Reportf(x.Pos(), "variable declaration copies lock value to %v: %v", valueSpec.Names[i].Name, path)
+ }
+ }
+ }
+}
+
+// checkCopyLocksCompositeLit detects lock copy inside a composite literal
+func checkCopyLocksCompositeLit(pass *analysis.Pass, cl *ast.CompositeLit) {
+ for _, x := range cl.Elts {
+ if node, ok := x.(*ast.KeyValueExpr); ok {
+ x = node.Value
+ }
+ if path := lockPathRhs(pass, x); path != nil {
+ pass.Reportf(x.Pos(), "literal copies lock value from %v: %v", analysisutil.Format(pass.Fset, x), path)
+ }
+ }
+}
+
+// checkCopyLocksReturnStmt detects lock copy in return statement
+func checkCopyLocksReturnStmt(pass *analysis.Pass, rs *ast.ReturnStmt) {
+ for _, x := range rs.Results {
+ if path := lockPathRhs(pass, x); path != nil {
+ pass.Reportf(x.Pos(), "return copies lock value: %v", path)
+ }
+ }
+}
+
+// checkCopyLocksCallExpr detects lock copy in the arguments to a function call
+func checkCopyLocksCallExpr(pass *analysis.Pass, ce *ast.CallExpr) {
+ var id *ast.Ident
+ switch fun := ce.Fun.(type) {
+ case *ast.Ident:
+ id = fun
+ case *ast.SelectorExpr:
+ id = fun.Sel
+ }
+ if fun, ok := pass.TypesInfo.Uses[id].(*types.Builtin); ok {
+ switch fun.Name() {
+ case "new", "len", "cap", "Sizeof":
+ return
+ }
+ }
+ for _, x := range ce.Args {
+ if path := lockPathRhs(pass, x); path != nil {
+ pass.Reportf(x.Pos(), "call of %s copies lock value: %v", analysisutil.Format(pass.Fset, ce.Fun), path)
+ }
+ }
+}
+
+// checkCopyLocksFunc checks whether a function might
+// inadvertently copy a lock, by checking whether
+// its receiver, parameters, or return values
+// are locks.
+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 {
+ pass.Reportf(expr.Pos(), "%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 {
+ pass.Reportf(expr.Pos(), "%s passes lock by value: %v", name, path)
+ }
+ }
+ }
+
+ // Don't check typ.Results. If T has a Lock field it's OK to write
+ // return T{}
+ // because that is returning the zero value. Leave result checking
+ // to the return statement.
+}
+
+// checkCopyLocksRange checks whether a range statement
+// might inadvertently copy a lock by checking whether
+// any of the range variables are locks.
+func checkCopyLocksRange(pass *analysis.Pass, r *ast.RangeStmt) {
+ checkCopyLocksRangeVar(pass, r.Tok, r.Key)
+ checkCopyLocksRangeVar(pass, r.Tok, r.Value)
+}
+
+func checkCopyLocksRangeVar(pass *analysis.Pass, rtok token.Token, e ast.Expr) {
+ if e == nil {
+ return
+ }
+ id, isId := e.(*ast.Ident)
+ if isId && id.Name == "_" {
+ return
+ }
+
+ var typ types.Type
+ if rtok == token.DEFINE {
+ if !isId {
+ return
+ }
+ obj := pass.TypesInfo.Defs[id]
+ if obj == nil {
+ return
+ }
+ typ = obj.Type()
+ } else {
+ typ = pass.TypesInfo.Types[e].Type
+ }
+
+ if typ == nil {
+ return
+ }
+ if path := lockPath(pass.Pkg, typ); path != nil {
+ pass.Reportf(e.Pos(), "range var %s copies lock: %v", analysisutil.Format(pass.Fset, e), path)
+ }
+}
+
+type typePath []types.Type
+
+// String pretty-prints a typePath.
+func (path typePath) String() string {
+ n := len(path)
+ var buf bytes.Buffer
+ for i := range path {
+ if i > 0 {
+ fmt.Fprint(&buf, " contains ")
+ }
+ // The human-readable path is in reverse order, outermost to innermost.
+ fmt.Fprint(&buf, path[n-i-1].String())
+ }
+ return buf.String()
+}
+
+func lockPathRhs(pass *analysis.Pass, x ast.Expr) typePath {
+ if _, ok := x.(*ast.CompositeLit); ok {
+ return nil
+ }
+ if _, ok := x.(*ast.CallExpr); ok {
+ // A call may return a zero value.
+ return nil
+ }
+ if star, ok := x.(*ast.StarExpr); ok {
+ if _, ok := star.X.(*ast.CallExpr); ok {
+ // A call may return a pointer to a zero value.
+ return nil
+ }
+ }
+ return lockPath(pass.Pkg, pass.TypesInfo.Types[x].Type)
+}
+
+// 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 {
+ if typ == nil {
+ return nil
+ }
+
+ for {
+ atyp, ok := typ.Underlying().(*types.Array)
+ if !ok {
+ break
+ }
+ typ = atyp.Elem()
+ }
+
+ // We're only interested in the case in which the underlying
+ // type is a struct. (Interfaces and pointers are safe to copy.)
+ styp, ok := typ.Underlying().(*types.Struct)
+ if !ok {
+ return nil
+ }
+
+ // We're looking for cases in which a pointer to this type
+ // is a sync.Locker, but a value is not. This differentiates
+ // embedded interfaces from embedded values.
+ if types.Implements(types.NewPointer(typ), lockerType) && !types.Implements(typ, lockerType) {
+ return []types.Type{typ}
+ }
+
+ nfields := styp.NumFields()
+ for i := 0; i < nfields; i++ {
+ ftyp := styp.Field(i).Type()
+ subpath := lockPath(tpkg, ftyp)
+ if subpath != nil {
+ return append(subpath, typ)
+ }
+ }
+
+ return nil
+}
+
+var lockerType *types.Interface
+
+// Construct a sync.Locker interface type.
+func init() {
+ nullary := types.NewSignature(nil, nil, nil, false) // func()
+ methods := []*types.Func{
+ types.NewFunc(token.NoPos, nil, "Lock", nullary),
+ types.NewFunc(token.NoPos, nil, "Unlock", nullary),
+ }
+ lockerType = types.NewInterface(methods, nil).Complete()
+}
diff --git a/go/analysis/passes/copylock/copylock_test.go b/go/analysis/passes/copylock/copylock_test.go
new file mode 100644
index 0000000..3e4ca12
--- /dev/null
+++ b/go/analysis/passes/copylock/copylock_test.go
@@ -0,0 +1,13 @@
+package copylock_test
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/copylock"
+)
+
+func TestFromFileSystem(t *testing.T) {
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, copylock.Analyzer, "a")
+}
diff --git a/go/analysis/passes/copylock/testdata/src/a/copylock.go b/go/analysis/passes/copylock/testdata/src/a/copylock.go
new file mode 100644
index 0000000..57d4076
--- /dev/null
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock.go
@@ -0,0 +1,188 @@
+package a
+
+import (
+ "sync"
+ "sync/atomic"
+ "unsafe"
+ . "unsafe"
+ unsafe1 "unsafe"
+)
+
+func OkFunc() {
+ var x *sync.Mutex
+ p := x
+ var y sync.Mutex
+ p = &y
+
+ var z = sync.Mutex{}
+ w := sync.Mutex{}
+
+ w = sync.Mutex{}
+ q := struct{ L sync.Mutex }{
+ L: sync.Mutex{},
+ }
+
+ yy := []Tlock{
+ Tlock{},
+ Tlock{
+ once: sync.Once{},
+ },
+ }
+
+ nl := new(sync.Mutex)
+ mx := make([]sync.Mutex, 10)
+ xx := struct{ L *sync.Mutex }{
+ L: new(sync.Mutex),
+ }
+}
+
+type Tlock struct {
+ once sync.Once
+}
+
+func BadFunc() {
+ var x *sync.Mutex
+ p := x
+ var y sync.Mutex
+ p = &y
+ *p = *x // want `assignment copies lock value to \*p: sync.Mutex`
+
+ var t Tlock
+ var tp *Tlock
+ tp = &t
+ *tp = t // want `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ t = *tp // want "assignment copies lock value to t: a.Tlock contains sync.Once contains sync.Mutex"
+
+ y := *x // want "assignment copies lock value to y: sync.Mutex"
+ var z = t // want "variable declaration copies lock value to z: a.Tlock contains sync.Once contains sync.Mutex"
+
+ w := struct{ L sync.Mutex }{
+ L: *x, // want `literal copies lock value from \*x: sync.Mutex`
+ }
+ var q = map[int]Tlock{
+ 1: t, // want "literal copies lock value from t: a.Tlock contains sync.Once contains sync.Mutex"
+ 2: *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ }
+ yy := []Tlock{
+ t, // want "literal copies lock value from t: a.Tlock contains sync.Once contains sync.Mutex"
+ *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ }
+
+ // override 'new' keyword
+ new := func(interface{}) {}
+ new(t) // want "call of new copies lock value: a.Tlock contains sync.Once contains sync.Mutex"
+
+ // copy of array of locks
+ var muA [5]sync.Mutex
+ muB := muA // want "assignment copies lock value to muB: sync.Mutex"
+ muA = muB // want "assignment copies lock value to muA: sync.Mutex"
+ muSlice := muA[:] // OK
+
+ // multidimensional array
+ var mmuA [5][5]sync.Mutex
+ mmuB := mmuA // want "assignment copies lock value to mmuB: sync.Mutex"
+ mmuA = mmuB // want "assignment copies lock value to mmuA: sync.Mutex"
+ mmuSlice := mmuA[:] // OK
+
+ // slice copy is ok
+ var fmuA [5][][5]sync.Mutex
+ fmuB := fmuA // OK
+ fmuA = fmuB // OK
+ fmuSlice := fmuA[:] // OK
+}
+
+func LenAndCapOnLockArrays() {
+ var a [5]sync.Mutex
+ aLen := len(a) // OK
+ aCap := cap(a) // OK
+
+ // override 'len' and 'cap' keywords
+
+ len := func(interface{}) {}
+ len(a) // want "call of len copies lock value: sync.Mutex"
+
+ cap := func(interface{}) {}
+ cap(a) // want "call of cap copies lock value: sync.Mutex"
+}
+
+func SizeofMutex() {
+ var mu sync.Mutex
+ unsafe.Sizeof(mu) // OK
+ unsafe1.Sizeof(mu) // OK
+ Sizeof(mu) // OK
+ unsafe := struct{ Sizeof func(interface{}) }{}
+ unsafe.Sizeof(mu) // want "call of unsafe.Sizeof copies lock value: sync.Mutex"
+ Sizeof := func(interface{}) {}
+ Sizeof(mu) // want "call of Sizeof copies lock value: sync.Mutex"
+}
+
+// SyncTypesCheck checks copying of sync.* types except sync.Mutex
+func SyncTypesCheck() {
+ // sync.RWMutex copying
+ var rwmuX sync.RWMutex
+ var rwmuXX = sync.RWMutex{}
+ rwmuX1 := new(sync.RWMutex)
+ rwmuY := rwmuX // want "assignment copies lock value to rwmuY: sync.RWMutex"
+ rwmuY = rwmuX // want "assignment copies lock value to rwmuY: sync.RWMutex"
+ var rwmuYY = rwmuX // want "variable declaration copies lock value to rwmuYY: sync.RWMutex"
+ rwmuP := &rwmuX
+ rwmuZ := &sync.RWMutex{}
+
+ // sync.Cond copying
+ var condX sync.Cond
+ var condXX = sync.Cond{}
+ condX1 := new(sync.Cond)
+ condY := condX // want "assignment copies lock value to condY: sync.Cond contains sync.noCopy"
+ condY = condX // want "assignment copies lock value to condY: sync.Cond contains sync.noCopy"
+ var condYY = condX // want "variable declaration copies lock value to condYY: sync.Cond contains sync.noCopy"
+ condP := &condX
+ condZ := &sync.Cond{
+ L: &sync.Mutex{},
+ }
+ condZ = sync.NewCond(&sync.Mutex{})
+
+ // sync.WaitGroup copying
+ var wgX sync.WaitGroup
+ var wgXX = sync.WaitGroup{}
+ wgX1 := new(sync.WaitGroup)
+ wgY := wgX // want "assignment copies lock value to wgY: sync.WaitGroup contains sync.noCopy"
+ wgY = wgX // want "assignment copies lock value to wgY: sync.WaitGroup contains sync.noCopy"
+ var wgYY = wgX // want "variable declaration copies lock value to wgYY: sync.WaitGroup contains sync.noCopy"
+ wgP := &wgX
+ wgZ := &sync.WaitGroup{}
+
+ // sync.Pool copying
+ var poolX sync.Pool
+ var poolXX = sync.Pool{}
+ poolX1 := new(sync.Pool)
+ poolY := poolX // want "assignment copies lock value to poolY: sync.Pool contains sync.noCopy"
+ poolY = poolX // want "assignment copies lock value to poolY: sync.Pool contains sync.noCopy"
+ var poolYY = poolX // want "variable declaration copies lock value to poolYY: sync.Pool contains sync.noCopy"
+ poolP := &poolX
+ poolZ := &sync.Pool{}
+
+ // sync.Once copying
+ var onceX sync.Once
+ var onceXX = sync.Once{}
+ onceX1 := new(sync.Once)
+ onceY := onceX // want "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
+ onceY = onceX // want "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
+ var onceYY = onceX // want "variable declaration copies lock value to onceYY: sync.Once contains sync.Mutex"
+ onceP := &onceX
+ onceZ := &sync.Once{}
+}
+
+// AtomicTypesCheck checks copying of sync/atomic types
+func AtomicTypesCheck() {
+ // atomic.Value copying
+ var vX atomic.Value
+ var vXX = atomic.Value{}
+ vX1 := new(atomic.Value)
+ // These are OK because the value has not been used yet.
+ // (And vet can't tell whether it has been used, so they're always OK.)
+ vY := vX
+ vY = vX
+ var vYY = vX
+ vP := &vX
+ vZ := &atomic.Value{}
+}
diff --git a/go/analysis/passes/vet/testdata/copylock_func.go b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
similarity index 62%
rename from go/analysis/passes/vet/testdata/copylock_func.go
rename to go/analysis/passes/copylock/testdata/src/a/copylock_func.go
index 280747a..801bc6f 100644
--- a/go/analysis/passes/vet/testdata/copylock_func.go
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
@@ -5,20 +5,20 @@
// This file contains tests for the copylock checker's
// function declaration analysis.
-package testdata
+package a
import "sync"
func OkFunc(*sync.Mutex) {}
-func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes lock by value: sync.Mutex"
-func BadFunc2(sync.Map) {} // ERROR "BadFunc2 passes lock by value: sync.Map contains sync.Mutex"
+func BadFunc(sync.Mutex) {} // want "BadFunc passes lock by value: sync.Mutex"
+func BadFunc2(sync.Map) {} // want "BadFunc2 passes lock by value: sync.Map contains sync.Mutex"
func OkRet() *sync.Mutex {}
func BadRet() sync.Mutex {} // Don't warn about results
var (
OkClosure = func(*sync.Mutex) {}
- BadClosure = func(sync.Mutex) {} // ERROR "func passes lock by value: sync.Mutex"
- BadClosure2 = func(sync.Map) {} // ERROR "func passes lock by value: sync.Map contains sync.Mutex"
+ BadClosure = func(sync.Mutex) {} // want "func passes lock by value: sync.Mutex"
+ BadClosure2 = func(sync.Map) {} // want "func passes lock by value: sync.Map contains sync.Mutex"
)
type EmbeddedRWMutex struct {
@@ -26,9 +26,9 @@
}
func (*EmbeddedRWMutex) OkMeth() {}
-func (EmbeddedRWMutex) BadMeth() {} // ERROR "BadMeth passes lock by value: testdata.EmbeddedRWMutex"
+func (EmbeddedRWMutex) BadMeth() {} // want "BadMeth passes lock by value: a.EmbeddedRWMutex"
func OkFunc(e *EmbeddedRWMutex) {}
-func BadFunc(EmbeddedRWMutex) {} // ERROR "BadFunc passes lock by value: testdata.EmbeddedRWMutex"
+func BadFunc(EmbeddedRWMutex) {} // want "BadFunc passes lock by value: a.EmbeddedRWMutex"
func OkRet() *EmbeddedRWMutex {}
func BadRet() EmbeddedRWMutex {} // Don't warn about results
@@ -37,9 +37,9 @@
}
func (*FieldMutex) OkMeth() {}
-func (FieldMutex) BadMeth() {} // ERROR "BadMeth passes lock by value: testdata.FieldMutex contains sync.Mutex"
+func (FieldMutex) BadMeth() {} // want "BadMeth passes lock by value: a.FieldMutex contains sync.Mutex"
func OkFunc(*FieldMutex) {}
-func BadFunc(FieldMutex, int) {} // ERROR "BadFunc passes lock by value: testdata.FieldMutex contains sync.Mutex"
+func BadFunc(FieldMutex, int) {} // want "BadFunc passes lock by value: a.FieldMutex contains sync.Mutex"
type L0 struct {
L1
@@ -54,7 +54,7 @@
}
func (*L0) Ok() {}
-func (L0) Bad() {} // ERROR "Bad passes lock by value: testdata.L0 contains testdata.L1 contains testdata.L2"
+func (L0) Bad() {} // want "Bad passes lock by value: a.L0 contains a.L1 contains a.L2"
type EmbeddedMutexPointer struct {
s *sync.Mutex // safe to copy this pointer
@@ -78,7 +78,7 @@
func (*CustomLock) Unlock() {}
func Ok(*CustomLock) {}
-func Bad(CustomLock) {} // ERROR "Bad passes lock by value: testdata.CustomLock"
+func Bad(CustomLock) {} // want "Bad passes lock by value: a.CustomLock"
// Passing lock values into interface function arguments
func FuncCallInterfaceArg(f func(a int, b interface{})) {
@@ -88,10 +88,10 @@
f(1, "foo")
f(2, &t)
f(3, &sync.Mutex{})
- f(4, m) // ERROR "call of f copies lock value: sync.Mutex"
- f(5, t) // ERROR "call of f copies lock value: struct.lock sync.Mutex. contains sync.Mutex"
+ f(4, m) // want "call of f copies lock value: sync.Mutex"
+ f(5, t) // want "call of f copies lock value: struct.lock sync.Mutex. contains sync.Mutex"
var fntab []func(t)
- fntab[0](t) // ERROR "call of fntab.0. copies lock value: struct.lock sync.Mutex. contains sync.Mutex"
+ fntab[0](t) // want "call of fntab.0. copies lock value: struct.lock sync.Mutex. contains sync.Mutex"
}
// Returning lock via interface value
@@ -105,9 +105,9 @@
case 1:
return 1, &sync.Mutex{}
case 2:
- return 2, m // ERROR "return copies lock value: sync.Mutex"
+ return 2, m // want "return copies lock value: sync.Mutex"
default:
- return 3, t // ERROR "return copies lock value: struct.lock sync.Mutex. contains sync.Mutex"
+ return 3, t // want "return copies lock value: struct.lock sync.Mutex. contains sync.Mutex"
}
}
@@ -126,7 +126,7 @@
// sync.Mutex gets called out, but without any reference to the sync.Once.
type LocalOnce sync.Once
-func (LocalOnce) Bad() {} // ERROR "Bad passes lock by value: testdata.LocalOnce contains sync.Mutex"
+func (LocalOnce) Bad() {} // want "Bad passes lock by value: a.LocalOnce contains sync.Mutex"
// False negative:
// LocalMutex doesn't have a Lock method.
diff --git a/go/analysis/passes/copylock/testdata/src/a/copylock_range.go b/go/analysis/passes/copylock/testdata/src/a/copylock_range.go
new file mode 100644
index 0000000..cd95a52
--- /dev/null
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock_range.go
@@ -0,0 +1,67 @@
+// Copyright 2014 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.
+
+// This file contains tests for the copylock checker's
+// range statement analysis.
+
+package a
+
+import "sync"
+
+func rangeMutex() {
+ var mu sync.Mutex
+ var i int
+
+ var s []sync.Mutex
+ for range s {
+ }
+ for i = range s {
+ }
+ for i := range s {
+ }
+ for i, _ = range s {
+ }
+ for i, _ := range s {
+ }
+ for _, mu = range s { // want "range var mu copies lock: sync.Mutex"
+ }
+ for _, m := range s { // want "range var m copies lock: sync.Mutex"
+ }
+ for i, mu = range s { // want "range var mu copies lock: sync.Mutex"
+ }
+ for i, m := range s { // want "range var m copies lock: sync.Mutex"
+ }
+
+ var a [3]sync.Mutex
+ for _, m := range a { // want "range var m copies lock: sync.Mutex"
+ }
+
+ var m map[sync.Mutex]sync.Mutex
+ for k := range m { // want "range var k copies lock: sync.Mutex"
+ }
+ for mu, _ = range m { // want "range var mu copies lock: sync.Mutex"
+ }
+ for k, _ := range m { // want "range var k copies lock: sync.Mutex"
+ }
+ for _, mu = range m { // want "range var mu copies lock: sync.Mutex"
+ }
+ for _, v := range m { // want "range var v copies lock: sync.Mutex"
+ }
+
+ var c chan sync.Mutex
+ for range c {
+ }
+ for mu = range c { // want "range var mu copies lock: sync.Mutex"
+ }
+ for v := range c { // want "range var v copies lock: sync.Mutex"
+ }
+
+ // Test non-idents in range variables
+ var t struct {
+ i int
+ mu sync.Mutex
+ }
+ for t.i, t.mu = range s { // want "range var t.mu copies lock: sync.Mutex"
+ }
+}
diff --git a/go/analysis/passes/vet/copylock.go b/go/analysis/passes/vet/copylock.go
deleted file mode 100644
index f449045..0000000
--- a/go/analysis/passes/vet/copylock.go
+++ /dev/null
@@ -1,268 +0,0 @@
-// +build ignore
-
-// Copyright 2013 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.
-
-// This file contains the code to check that locks are not passed by value.
-
-package main
-
-import (
- "bytes"
- "fmt"
- "go/ast"
- "go/token"
- "go/types"
-)
-
-func init() {
- register("copylocks",
- "check that locks are not passed by value",
- checkCopyLocks,
- funcDecl, rangeStmt, funcLit, callExpr, assignStmt, genDecl, compositeLit, returnStmt)
-}
-
-// checkCopyLocks checks whether node might
-// inadvertently copy a lock.
-func checkCopyLocks(f *File, node ast.Node) {
- switch node := node.(type) {
- case *ast.RangeStmt:
- checkCopyLocksRange(f, node)
- case *ast.FuncDecl:
- checkCopyLocksFunc(f, node.Name.Name, node.Recv, node.Type)
- case *ast.FuncLit:
- checkCopyLocksFunc(f, "func", nil, node.Type)
- case *ast.CallExpr:
- checkCopyLocksCallExpr(f, node)
- case *ast.AssignStmt:
- checkCopyLocksAssign(f, node)
- case *ast.GenDecl:
- checkCopyLocksGenDecl(f, node)
- case *ast.CompositeLit:
- checkCopyLocksCompositeLit(f, node)
- case *ast.ReturnStmt:
- checkCopyLocksReturnStmt(f, node)
- }
-}
-
-// checkCopyLocksAssign checks whether an assignment
-// copies a lock.
-func checkCopyLocksAssign(f *File, as *ast.AssignStmt) {
- for i, x := range as.Rhs {
- if path := lockPathRhs(f, x); path != nil {
- f.Badf(x.Pos(), "assignment copies lock value to %v: %v", f.gofmt(as.Lhs[i]), path)
- }
- }
-}
-
-// checkCopyLocksGenDecl checks whether lock is copied
-// in variable declaration.
-func checkCopyLocksGenDecl(f *File, gd *ast.GenDecl) {
- if gd.Tok != token.VAR {
- return
- }
- for _, spec := range gd.Specs {
- valueSpec := spec.(*ast.ValueSpec)
- for i, x := range valueSpec.Values {
- if path := lockPathRhs(f, x); path != nil {
- f.Badf(x.Pos(), "variable declaration copies lock value to %v: %v", valueSpec.Names[i].Name, path)
- }
- }
- }
-}
-
-// checkCopyLocksCompositeLit detects lock copy inside a composite literal
-func checkCopyLocksCompositeLit(f *File, cl *ast.CompositeLit) {
- for _, x := range cl.Elts {
- if node, ok := x.(*ast.KeyValueExpr); ok {
- x = node.Value
- }
- if path := lockPathRhs(f, x); path != nil {
- f.Badf(x.Pos(), "literal copies lock value from %v: %v", f.gofmt(x), path)
- }
- }
-}
-
-// checkCopyLocksReturnStmt detects lock copy in return statement
-func checkCopyLocksReturnStmt(f *File, rs *ast.ReturnStmt) {
- for _, x := range rs.Results {
- if path := lockPathRhs(f, x); path != nil {
- f.Badf(x.Pos(), "return copies lock value: %v", path)
- }
- }
-}
-
-// checkCopyLocksCallExpr detects lock copy in the arguments to a function call
-func checkCopyLocksCallExpr(f *File, ce *ast.CallExpr) {
- var id *ast.Ident
- switch fun := ce.Fun.(type) {
- case *ast.Ident:
- id = fun
- case *ast.SelectorExpr:
- id = fun.Sel
- }
- if fun, ok := f.pkg.uses[id].(*types.Builtin); ok {
- switch fun.Name() {
- case "new", "len", "cap", "Sizeof":
- return
- }
- }
- for _, x := range ce.Args {
- if path := lockPathRhs(f, x); path != nil {
- f.Badf(x.Pos(), "call of %s copies lock value: %v", f.gofmt(ce.Fun), path)
- }
- }
-}
-
-// checkCopyLocksFunc checks whether a function might
-// inadvertently copy a lock, by checking whether
-// its receiver, parameters, or return values
-// are locks.
-func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.FuncType) {
- if recv != nil && len(recv.List) > 0 {
- expr := recv.List[0].Type
- if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
- f.Badf(expr.Pos(), "%s passes lock by value: %v", name, path)
- }
- }
-
- if typ.Params != nil {
- for _, field := range typ.Params.List {
- expr := field.Type
- if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
- f.Badf(expr.Pos(), "%s passes lock by value: %v", name, path)
- }
- }
- }
-
- // Don't check typ.Results. If T has a Lock field it's OK to write
- // return T{}
- // because that is returning the zero value. Leave result checking
- // to the return statement.
-}
-
-// checkCopyLocksRange checks whether a range statement
-// might inadvertently copy a lock by checking whether
-// any of the range variables are locks.
-func checkCopyLocksRange(f *File, r *ast.RangeStmt) {
- checkCopyLocksRangeVar(f, r.Tok, r.Key)
- checkCopyLocksRangeVar(f, r.Tok, r.Value)
-}
-
-func checkCopyLocksRangeVar(f *File, rtok token.Token, e ast.Expr) {
- if e == nil {
- return
- }
- id, isId := e.(*ast.Ident)
- if isId && id.Name == "_" {
- return
- }
-
- var typ types.Type
- if rtok == token.DEFINE {
- if !isId {
- return
- }
- obj := f.pkg.defs[id]
- if obj == nil {
- return
- }
- typ = obj.Type()
- } else {
- typ = f.pkg.types[e].Type
- }
-
- if typ == nil {
- return
- }
- if path := lockPath(f.pkg.typesPkg, typ); path != nil {
- f.Badf(e.Pos(), "range var %s copies lock: %v", f.gofmt(e), path)
- }
-}
-
-type typePath []types.Type
-
-// String pretty-prints a typePath.
-func (path typePath) String() string {
- n := len(path)
- var buf bytes.Buffer
- for i := range path {
- if i > 0 {
- fmt.Fprint(&buf, " contains ")
- }
- // The human-readable path is in reverse order, outermost to innermost.
- fmt.Fprint(&buf, path[n-i-1].String())
- }
- return buf.String()
-}
-
-func lockPathRhs(f *File, x ast.Expr) typePath {
- if _, ok := x.(*ast.CompositeLit); ok {
- return nil
- }
- if _, ok := x.(*ast.CallExpr); ok {
- // A call may return a zero value.
- return nil
- }
- if star, ok := x.(*ast.StarExpr); ok {
- if _, ok := star.X.(*ast.CallExpr); ok {
- // A call may return a pointer to a zero value.
- return nil
- }
- }
- return lockPath(f.pkg.typesPkg, f.pkg.types[x].Type)
-}
-
-// 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 {
- if typ == nil {
- return nil
- }
-
- for {
- atyp, ok := typ.Underlying().(*types.Array)
- if !ok {
- break
- }
- typ = atyp.Elem()
- }
-
- // We're only interested in the case in which the underlying
- // type is a struct. (Interfaces and pointers are safe to copy.)
- styp, ok := typ.Underlying().(*types.Struct)
- if !ok {
- return nil
- }
-
- // We're looking for cases in which a pointer to this type
- // is a sync.Locker, but a value is not. This differentiates
- // embedded interfaces from embedded values.
- if types.Implements(types.NewPointer(typ), lockerType) && !types.Implements(typ, lockerType) {
- return []types.Type{typ}
- }
-
- nfields := styp.NumFields()
- for i := 0; i < nfields; i++ {
- ftyp := styp.Field(i).Type()
- subpath := lockPath(tpkg, ftyp)
- if subpath != nil {
- return append(subpath, typ)
- }
- }
-
- return nil
-}
-
-var lockerType *types.Interface
-
-// Construct a sync.Locker interface type.
-func init() {
- nullary := types.NewSignature(nil, nil, nil, false) // func()
- methods := []*types.Func{
- types.NewFunc(token.NoPos, nil, "Lock", nullary),
- types.NewFunc(token.NoPos, nil, "Unlock", nullary),
- }
- lockerType = types.NewInterface(methods, nil).Complete()
-}
diff --git a/go/analysis/passes/vet/testdata/copylock.go b/go/analysis/passes/vet/testdata/copylock.go
deleted file mode 100644
index e9902a2..0000000
--- a/go/analysis/passes/vet/testdata/copylock.go
+++ /dev/null
@@ -1,188 +0,0 @@
-package testdata
-
-import (
- "sync"
- "sync/atomic"
- "unsafe"
- . "unsafe"
- unsafe1 "unsafe"
-)
-
-func OkFunc() {
- var x *sync.Mutex
- p := x
- var y sync.Mutex
- p = &y
-
- var z = sync.Mutex{}
- w := sync.Mutex{}
-
- w = sync.Mutex{}
- q := struct{ L sync.Mutex }{
- L: sync.Mutex{},
- }
-
- yy := []Tlock{
- Tlock{},
- Tlock{
- once: sync.Once{},
- },
- }
-
- nl := new(sync.Mutex)
- mx := make([]sync.Mutex, 10)
- xx := struct{ L *sync.Mutex }{
- L: new(sync.Mutex),
- }
-}
-
-type Tlock struct {
- once sync.Once
-}
-
-func BadFunc() {
- var x *sync.Mutex
- p := x
- var y sync.Mutex
- p = &y
- *p = *x // ERROR "assignment copies lock value to \*p: sync.Mutex"
-
- var t Tlock
- var tp *Tlock
- tp = &t
- *tp = t // ERROR "assignment copies lock value to \*tp: testdata.Tlock contains sync.Once contains sync.Mutex"
- t = *tp // ERROR "assignment copies lock value to t: testdata.Tlock contains sync.Once contains sync.Mutex"
-
- y := *x // ERROR "assignment copies lock value to y: sync.Mutex"
- var z = t // ERROR "variable declaration copies lock value to z: testdata.Tlock contains sync.Once contains sync.Mutex"
-
- w := struct{ L sync.Mutex }{
- L: *x, // ERROR "literal copies lock value from \*x: sync.Mutex"
- }
- var q = map[int]Tlock{
- 1: t, // ERROR "literal copies lock value from t: testdata.Tlock contains sync.Once contains sync.Mutex"
- 2: *tp, // ERROR "literal copies lock value from \*tp: testdata.Tlock contains sync.Once contains sync.Mutex"
- }
- yy := []Tlock{
- t, // ERROR "literal copies lock value from t: testdata.Tlock contains sync.Once contains sync.Mutex"
- *tp, // ERROR "literal copies lock value from \*tp: testdata.Tlock contains sync.Once contains sync.Mutex"
- }
-
- // override 'new' keyword
- new := func(interface{}) {}
- new(t) // ERROR "call of new copies lock value: testdata.Tlock contains sync.Once contains sync.Mutex"
-
- // copy of array of locks
- var muA [5]sync.Mutex
- muB := muA // ERROR "assignment copies lock value to muB: sync.Mutex"
- muA = muB // ERROR "assignment copies lock value to muA: sync.Mutex"
- muSlice := muA[:] // OK
-
- // multidimensional array
- var mmuA [5][5]sync.Mutex
- mmuB := mmuA // ERROR "assignment copies lock value to mmuB: sync.Mutex"
- mmuA = mmuB // ERROR "assignment copies lock value to mmuA: sync.Mutex"
- mmuSlice := mmuA[:] // OK
-
- // slice copy is ok
- var fmuA [5][][5]sync.Mutex
- fmuB := fmuA // OK
- fmuA = fmuB // OK
- fmuSlice := fmuA[:] // OK
-}
-
-func LenAndCapOnLockArrays() {
- var a [5]sync.Mutex
- aLen := len(a) // OK
- aCap := cap(a) // OK
-
- // override 'len' and 'cap' keywords
-
- len := func(interface{}) {}
- len(a) // ERROR "call of len copies lock value: sync.Mutex"
-
- cap := func(interface{}) {}
- cap(a) // ERROR "call of cap copies lock value: sync.Mutex"
-}
-
-func SizeofMutex() {
- var mu sync.Mutex
- unsafe.Sizeof(mu) // OK
- unsafe1.Sizeof(mu) // OK
- Sizeof(mu) // OK
- unsafe := struct{ Sizeof func(interface{}) }{}
- unsafe.Sizeof(mu) // ERROR "call of unsafe.Sizeof copies lock value: sync.Mutex"
- Sizeof := func(interface{}) {}
- Sizeof(mu) // ERROR "call of Sizeof copies lock value: sync.Mutex"
-}
-
-// SyncTypesCheck checks copying of sync.* types except sync.Mutex
-func SyncTypesCheck() {
- // sync.RWMutex copying
- var rwmuX sync.RWMutex
- var rwmuXX = sync.RWMutex{}
- rwmuX1 := new(sync.RWMutex)
- rwmuY := rwmuX // ERROR "assignment copies lock value to rwmuY: sync.RWMutex"
- rwmuY = rwmuX // ERROR "assignment copies lock value to rwmuY: sync.RWMutex"
- var rwmuYY = rwmuX // ERROR "variable declaration copies lock value to rwmuYY: sync.RWMutex"
- rwmuP := &rwmuX
- rwmuZ := &sync.RWMutex{}
-
- // sync.Cond copying
- var condX sync.Cond
- var condXX = sync.Cond{}
- condX1 := new(sync.Cond)
- condY := condX // ERROR "assignment copies lock value to condY: sync.Cond contains sync.noCopy"
- condY = condX // ERROR "assignment copies lock value to condY: sync.Cond contains sync.noCopy"
- var condYY = condX // ERROR "variable declaration copies lock value to condYY: sync.Cond contains sync.noCopy"
- condP := &condX
- condZ := &sync.Cond{
- L: &sync.Mutex{},
- }
- condZ = sync.NewCond(&sync.Mutex{})
-
- // sync.WaitGroup copying
- var wgX sync.WaitGroup
- var wgXX = sync.WaitGroup{}
- wgX1 := new(sync.WaitGroup)
- wgY := wgX // ERROR "assignment copies lock value to wgY: sync.WaitGroup contains sync.noCopy"
- wgY = wgX // ERROR "assignment copies lock value to wgY: sync.WaitGroup contains sync.noCopy"
- var wgYY = wgX // ERROR "variable declaration copies lock value to wgYY: sync.WaitGroup contains sync.noCopy"
- wgP := &wgX
- wgZ := &sync.WaitGroup{}
-
- // sync.Pool copying
- var poolX sync.Pool
- var poolXX = sync.Pool{}
- poolX1 := new(sync.Pool)
- poolY := poolX // ERROR "assignment copies lock value to poolY: sync.Pool contains sync.noCopy"
- poolY = poolX // ERROR "assignment copies lock value to poolY: sync.Pool contains sync.noCopy"
- var poolYY = poolX // ERROR "variable declaration copies lock value to poolYY: sync.Pool contains sync.noCopy"
- poolP := &poolX
- poolZ := &sync.Pool{}
-
- // sync.Once copying
- var onceX sync.Once
- var onceXX = sync.Once{}
- onceX1 := new(sync.Once)
- onceY := onceX // ERROR "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
- onceY = onceX // ERROR "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
- var onceYY = onceX // ERROR "variable declaration copies lock value to onceYY: sync.Once contains sync.Mutex"
- onceP := &onceX
- onceZ := &sync.Once{}
-}
-
-// AtomicTypesCheck checks copying of sync/atomic types
-func AtomicTypesCheck() {
- // atomic.Value copying
- var vX atomic.Value
- var vXX = atomic.Value{}
- vX1 := new(atomic.Value)
- // These are OK because the value has not been used yet.
- // (And vet can't tell whether it has been used, so they're always OK.)
- vY := vX
- vY = vX
- var vYY = vX
- vP := &vX
- vZ := &atomic.Value{}
-}
diff --git a/go/analysis/passes/vet/testdata/copylock_range.go b/go/analysis/passes/vet/testdata/copylock_range.go
deleted file mode 100644
index f127381..0000000
--- a/go/analysis/passes/vet/testdata/copylock_range.go
+++ /dev/null
@@ -1,67 +0,0 @@
-// Copyright 2014 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.
-
-// This file contains tests for the copylock checker's
-// range statement analysis.
-
-package testdata
-
-import "sync"
-
-func rangeMutex() {
- var mu sync.Mutex
- var i int
-
- var s []sync.Mutex
- for range s {
- }
- for i = range s {
- }
- for i := range s {
- }
- for i, _ = range s {
- }
- for i, _ := range s {
- }
- for _, mu = range s { // ERROR "range var mu copies lock: sync.Mutex"
- }
- for _, m := range s { // ERROR "range var m copies lock: sync.Mutex"
- }
- for i, mu = range s { // ERROR "range var mu copies lock: sync.Mutex"
- }
- for i, m := range s { // ERROR "range var m copies lock: sync.Mutex"
- }
-
- var a [3]sync.Mutex
- for _, m := range a { // ERROR "range var m copies lock: sync.Mutex"
- }
-
- var m map[sync.Mutex]sync.Mutex
- for k := range m { // ERROR "range var k copies lock: sync.Mutex"
- }
- for mu, _ = range m { // ERROR "range var mu copies lock: sync.Mutex"
- }
- for k, _ := range m { // ERROR "range var k copies lock: sync.Mutex"
- }
- for _, mu = range m { // ERROR "range var mu copies lock: sync.Mutex"
- }
- for _, v := range m { // ERROR "range var v copies lock: sync.Mutex"
- }
-
- var c chan sync.Mutex
- for range c {
- }
- for mu = range c { // ERROR "range var mu copies lock: sync.Mutex"
- }
- for v := range c { // ERROR "range var v copies lock: sync.Mutex"
- }
-
- // Test non-idents in range variables
- var t struct {
- i int
- mu sync.Mutex
- }
- for t.i, t.mu = range s { // ERROR "range var t.mu copies lock: sync.Mutex"
- }
-}
