Skip to content

Commit fee86e4

Browse files
valyalarobpike
authored andcommitted
cmd/vet: added some missing copylock checks
Fixes #14664 Change-Id: I8bda2435857772f590859808904c48d768b87d46 Reviewed-on: https://go-review.googlesource.com/20254 Run-TryBot: Rob Pike <[email protected]> Reviewed-by: Rob Pike <[email protected]>
1 parent 55567d3 commit fee86e4

File tree

2 files changed

+73
-4
lines changed

2 files changed

+73
-4
lines changed

src/cmd/vet/copylock.go

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func init() {
1818
register("copylocks",
1919
"check that locks are not passed by value",
2020
checkCopyLocks,
21-
funcDecl, rangeStmt, funcLit, assignStmt)
21+
funcDecl, rangeStmt, funcLit, assignStmt, genDecl, compositeLit)
2222
}
2323

2424
// checkCopyLocks checks whether node might
@@ -33,15 +33,47 @@ func checkCopyLocks(f *File, node ast.Node) {
3333
checkCopyLocksFunc(f, "func", nil, node.Type)
3434
case *ast.AssignStmt:
3535
checkCopyLocksAssign(f, node)
36+
case *ast.GenDecl:
37+
checkCopyLocksGenDecl(f, node)
38+
case *ast.CompositeLit:
39+
checkCopyCompositeLit(f, node)
3640
}
3741
}
3842

3943
// checkCopyLocksAssign checks whether an assignment
4044
// copies a lock.
4145
func checkCopyLocksAssign(f *File, as *ast.AssignStmt) {
42-
for _, x := range as.Lhs {
43-
if path := lockPath(f.pkg.typesPkg, f.pkg.types[x].Type); path != nil {
44-
f.Badf(x.Pos(), "assignment copies lock value to %v: %v", f.gofmt(x), path)
46+
for i, x := range as.Rhs {
47+
if path := lockPathRhs(f, x); path != nil {
48+
f.Badf(x.Pos(), "assignment copies lock value to %v: %v", f.gofmt(as.Lhs[i]), path)
49+
}
50+
}
51+
}
52+
53+
// checkCopyLocksGenDecl checks whether lock is copied
54+
// in variable declaration.
55+
func checkCopyLocksGenDecl(f *File, gd *ast.GenDecl) {
56+
if gd.Tok != token.VAR {
57+
return
58+
}
59+
for _, spec := range gd.Specs {
60+
valueSpec := spec.(*ast.ValueSpec)
61+
for i, x := range valueSpec.Values {
62+
if path := lockPathRhs(f, x); path != nil {
63+
f.Badf(x.Pos(), "variable declaration copies lock value to %v: %v", valueSpec.Names[i].Name, path)
64+
}
65+
}
66+
}
67+
}
68+
69+
// checkCopyCompositeLit detects lock copy inside a composite literal
70+
func checkCopyCompositeLit(f *File, cl *ast.CompositeLit) {
71+
for _, x := range cl.Elts {
72+
if node, ok := x.(*ast.KeyValueExpr); ok {
73+
x = node.Value
74+
}
75+
if path := lockPathRhs(f, x); path != nil {
76+
f.Badf(x.Pos(), "literal copies lock value from %v: %v", f.gofmt(x), path)
4577
}
4678
}
4779
}
@@ -132,6 +164,13 @@ func (path typePath) String() string {
132164
return buf.String()
133165
}
134166

167+
func lockPathRhs(f *File, x ast.Expr) typePath {
168+
if _, ok := x.(*ast.CompositeLit); ok {
169+
return nil
170+
}
171+
return lockPath(f.pkg.typesPkg, f.pkg.types[x].Type)
172+
}
173+
135174
// lockPath returns a typePath describing the location of a lock value
136175
// contained in typ. If there is no contained lock, it returns nil.
137176
func lockPath(tpkg *types.Package, typ types.Type) typePath {

src/cmd/vet/testdata/copylock.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,21 @@ func OkFunc() {
77
p := x
88
var y sync.Mutex
99
p = &y
10+
11+
var z = sync.Mutex{}
12+
w := sync.Mutex{}
13+
14+
w = sync.Mutex{}
15+
q := struct{ L sync.Mutex }{
16+
L: sync.Mutex{},
17+
}
18+
19+
yy := []Tlock{
20+
sync.Tlock{},
21+
sync.Tlock{
22+
once: sync.Once{},
23+
},
24+
}
1025
}
1126

1227
type Tlock struct {
@@ -25,4 +40,19 @@ func BadFunc() {
2540
tp = &t
2641
*tp = t // ERROR "assignment copies lock value to \*tp: testdata.Tlock contains sync.Once contains sync.Mutex"
2742
t = *tp // ERROR "assignment copies lock value to t: testdata.Tlock contains sync.Once contains sync.Mutex"
43+
44+
y := *x // ERROR "assignment copies lock value to y: sync.Mutex"
45+
var z = t // ERROR "variable declaration copies lock value to z: testdata.Tlock contains sync.Once contains sync.Mutex"
46+
47+
w := struct{ L sync.Mutex }{
48+
L: *x, // ERROR "literal copies lock value from \*x: sync.Mutex"
49+
}
50+
var q = map[int]Tlock{
51+
1: t, // ERROR "literal copies lock value from t: testdata.Tlock contains sync.Once contains sync.Mutex"
52+
2: *tp, // ERROR "literal copies lock value from \*tp: testdata.Tlock contains sync.Once contains sync.Mutex"
53+
}
54+
yy := []Tlock{
55+
t, // ERROR "literal copies lock value from t: testdata.Tlock contains sync.Once contains sync.Mutex"
56+
*tp, // ERROR "literal copies lock value from \*tp: testdata.Tlock contains sync.Once contains sync.Mutex"
57+
}
2858
}

0 commit comments

Comments
 (0)