cmd/vet: diagnose plain assignment in copylock detector

It went out of its way to look for implicit assignments
but never checked explicit assignments.

This detects the root bug for #12099.

Change-Id: I6a6e774cc38749ea8be7cfd58ba6421247b67000
Reviewed-on: https://go-review.googlesource.com/13646
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>

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)
+		funcDecl, rangeStmt, funcLit, assignStmt)
 }
 
 // checkCopyLocks checks whether node might
@@ -31,6 +31,18 @@ func checkCopyLocks(f *File, node ast.Node) {
 		checkCopyLocksFunc(f, node.Name.Name, node.Recv, node.Type)
 	case *ast.FuncLit:
 		checkCopyLocksFunc(f, "func", nil, node.Type)
+	case *ast.AssignStmt:
+		checkCopyLocksAssign(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)
+		}
 	}
 }
 
@@ -42,7 +54,7 @@ func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.Func
 	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)
+			f.Badf(expr.Pos(), "%s passes lock by value: %v", name, path)
 		}
 	}
 
@@ -50,7 +62,7 @@ func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.Func
 		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)
+				f.Badf(expr.Pos(), "%s passes lock by value: %v", name, path)
 			}
 		}
 	}
@@ -59,7 +71,7 @@ func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.Func
 		for _, field := range typ.Results.List {
 			expr := field.Type
 			if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
-				f.Badf(expr.Pos(), "%s returns Lock by value: %v", name, path)
+				f.Badf(expr.Pos(), "%s returns lock by value: %v", name, path)
 			}
 		}
 	}
@@ -100,7 +112,7 @@ func checkCopyLocksRangeVar(f *File, rtok token.Token, e ast.Expr) {
 		return
 	}
 	if path := lockPath(f.pkg.typesPkg, typ); path != nil {
-		f.Badf(e.Pos(), "range var %s copies Lock: %v", f.gofmt(e), path)
+		f.Badf(e.Pos(), "range var %s copies lock: %v", f.gofmt(e), path)
 	}
 }
 

src/cmd/vet/testdata/copylock.go

@@ -0,0 +1,28 @@
+package testdata
+
+import "sync"
+
+func OkFunc() {
+	var x *sync.Mutex
+	p := x
+	var y sync.Mutex
+	p = &y
+}
+
+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"
+}

src/cmd/vet/testdata/copylock_func.go

@@ -10,13 +10,13 @@ package testdata
 import "sync"
 
 func OkFunc(*sync.Mutex) {}
-func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes Lock by value: sync.Mutex"
+func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes lock by value: sync.Mutex"
 func OkRet() *sync.Mutex {}
-func BadRet() sync.Mutex {} // ERROR "BadRet returns Lock by value: sync.Mutex"
+func BadRet() sync.Mutex {} // ERROR "BadRet returns lock by value: sync.Mutex"
 
 var (
 	OkClosure  = func(*sync.Mutex) {}
-	BadClosure = func(sync.Mutex) {} // ERROR "func passes Lock by value: sync.Mutex"
+	BadClosure = func(sync.Mutex) {} // ERROR "func passes lock by value: sync.Mutex"
 )
 
 type EmbeddedRWMutex struct {
@@ -24,20 +24,20 @@ type EmbeddedRWMutex struct {
 }
 
 func (*EmbeddedRWMutex) OkMeth() {}
-func (EmbeddedRWMutex) BadMeth() {} // ERROR "BadMeth passes Lock by value: testdata.EmbeddedRWMutex"
+func (EmbeddedRWMutex) BadMeth() {} // ERROR "BadMeth passes lock by value: testdata.EmbeddedRWMutex"
 func OkFunc(e *EmbeddedRWMutex)  {}
-func BadFunc(EmbeddedRWMutex)    {} // ERROR "BadFunc passes Lock by value: testdata.EmbeddedRWMutex"
+func BadFunc(EmbeddedRWMutex)    {} // ERROR "BadFunc passes lock by value: testdata.EmbeddedRWMutex"
 func OkRet() *EmbeddedRWMutex    {}
-func BadRet() EmbeddedRWMutex    {} // ERROR "BadRet returns Lock by value: testdata.EmbeddedRWMutex"
+func BadRet() EmbeddedRWMutex    {} // ERROR "BadRet returns lock by value: testdata.EmbeddedRWMutex"
 
 type FieldMutex struct {
 	s sync.Mutex
 }
 
 func (*FieldMutex) OkMeth()   {}
-func (FieldMutex) BadMeth()   {} // ERROR "BadMeth passes Lock by value: testdata.FieldMutex contains sync.Mutex"
+func (FieldMutex) BadMeth()   {} // ERROR "BadMeth passes lock by value: testdata.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) {} // ERROR "BadFunc passes lock by value: testdata.FieldMutex contains sync.Mutex"
 
 type L0 struct {
 	L1
@@ -52,7 +52,7 @@ type L2 struct {
 }
 
 func (*L0) Ok() {}
-func (L0) Bad() {} // ERROR "Bad passes Lock by value: testdata.L0 contains testdata.L1 contains testdata.L2"
+func (L0) Bad() {} // ERROR "Bad passes lock by value: testdata.L0 contains testdata.L1 contains testdata.L2"
 
 type EmbeddedMutexPointer struct {
 	s *sync.Mutex // safe to copy this pointer
@@ -76,7 +76,7 @@ func (*CustomLock) Lock()   {}
 func (*CustomLock) Unlock() {}
 
 func Ok(*CustomLock) {}
-func Bad(CustomLock) {} // ERROR "Bad passes Lock by value: testdata.CustomLock"
+func Bad(CustomLock) {} // ERROR "Bad passes lock by value: testdata.CustomLock"
 
 // TODO: Unfortunate cases
 
@@ -85,7 +85,7 @@ func Bad(CustomLock) {} // ERROR "Bad passes Lock by value: testdata.CustomLock"
 // 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() {} // ERROR "Bad passes lock by value: testdata.LocalOnce contains sync.Mutex"
 
 // False negative:
 // LocalMutex doesn't have a Lock method.

src/cmd/vet/testdata/copylock_range.go

@@ -24,37 +24,37 @@ func rangeMutex() {
 	}
 	for i, _ := range s {
 	}
-	for _, mu = range s { // ERROR "range var mu copies Lock: sync.Mutex"
+	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 _, 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, mu = range s { // ERROR "range var mu copies lock: sync.Mutex"
 	}
-	for i, m := range s { // ERROR "range var m 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"
+	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 k := range m { // ERROR "range var k copies lock: sync.Mutex"
 	}
-	for mu, _ = range m { // ERROR "range var mu 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 k, _ := range m { // ERROR "range var k copies lock: sync.Mutex"
 	}
-	for _, mu = range m { // ERROR "range var mu 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"
+	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 mu = range c { // ERROR "range var mu copies lock: sync.Mutex"
 	}
-	for v := range c { // ERROR "range var v copies Lock: sync.Mutex"
+	for v := range c { // ERROR "range var v copies lock: sync.Mutex"
 	}
 
 	// Test non-idents in range variables
@@ -62,6 +62,6 @@ func rangeMutex() {
 		i  int
 		mu sync.Mutex
 	}
-	for t.i, t.mu = range s { // ERROR "range var t.mu copies Lock: sync.Mutex"
+	for t.i, t.mu = range s { // ERROR "range var t.mu copies lock: sync.Mutex"
 	}
 }