cmd/vet: added some missing copylock checks

Fixes #14664

Change-Id: I8bda2435857772f590859808904c48d768b87d46
Reviewed-on: https://go-review.googlesource.com/20254
Run-TryBot: Rob Pike <r@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/cmd/vet/copylock.go b/src/cmd/vet/copylock.go
index 8d8399a851b37192d0ea2dac789161d0583251f0..78632944ca82442577cfe5f94960974a1ca89da8 100644 (file)
--- a/src/cmd/vet/copylock.go
+++ b/src/cmd/vet/copylock.go
@@ -18,7 +18,7 @@ func init() {
        register("copylocks",
                "check that locks are not passed by value",
                checkCopyLocks,
-               funcDecl, rangeStmt, funcLit, assignStmt)
+               funcDecl, rangeStmt, funcLit, assignStmt, genDecl, compositeLit)
 }
 
 // checkCopyLocks checks whether node might
@@ -33,15 +33,47 @@ func checkCopyLocks(f *File, node ast.Node) {
                checkCopyLocksFunc(f, "func", nil, node.Type)
        case *ast.AssignStmt:
                checkCopyLocksAssign(f, node)
+       case *ast.GenDecl:
+               checkCopyLocksGenDecl(f, node)
+       case *ast.CompositeLit:
+               checkCopyCompositeLit(f, node)
        }
 }
 
 // checkCopyLocksAssign checks whether an assignment
 // copies a lock.
 func checkCopyLocksAssign(f *File, as *ast.AssignStmt) {
-       for _, x := range as.Lhs {
-               if path := lockPath(f.pkg.typesPkg, f.pkg.types[x].Type); path != nil {
-                       f.Badf(x.Pos(), "assignment copies lock value to %v: %v", f.gofmt(x), path)
+       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)
+                       }
+               }
+       }
+}
+
+// checkCopyCompositeLit detects lock copy inside a composite literal
+func checkCopyCompositeLit(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)
                }
        }
 }
@@ -132,6 +164,13 @@ func (path typePath) String() string {
        return buf.String()
 }
 
+func lockPathRhs(f *File, x ast.Expr) typePath {
+       if _, ok := x.(*ast.CompositeLit); ok {
+               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 {

diff --git a/src/cmd/vet/testdata/copylock.go b/src/cmd/vet/testdata/copylock.go
index 03d0c33f3648957cd619b42569f35ce0713f11b1..2b8cec142029b10f807faf39fbf03c4c216230dc 100644 (file)
--- a/src/cmd/vet/testdata/copylock.go
+++ b/src/cmd/vet/testdata/copylock.go
@@ -7,6 +7,21 @@ func OkFunc() {
        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{
+               sync.Tlock{},
+               sync.Tlock{
+                       once: sync.Once{},
+               },
+       }
 }
 
 type Tlock struct {
@@ -25,4 +40,19 @@ func BadFunc() {
        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"
+       }
 }