lostcancel: do not analyze cancel variable which defined outside current function scope
See golang/go#31856.
Change-Id: I229a7f4a48e7806df62941f801302b6da8a0c12b
GitHub-Last-Rev: 33f85236bb79e325112866ee555950a4479ad7a7
GitHub-Pull-Request: golang/tools#95
Reviewed-on: https://go-review.googlesource.com/c/tools/+/175617
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/go/analysis/passes/lostcancel/lostcancel.go b/go/analysis/passes/lostcancel/lostcancel.go
index b516183..e88cf57 100644
--- a/go/analysis/passes/lostcancel/lostcancel.go
+++ b/go/analysis/passes/lostcancel/lostcancel.go
@@ -45,6 +45,8 @@
// control-flow path from the call to a return statement and that path
// does not "use" the cancel function. Any reference to the variable
// counts as a use, even within a nested function literal.
+// If the variable's scope is larger than the function
+// containing the assignment, we assume that other uses exist.
//
// checkLostCancel analyzes a single named or literal function.
func run(pass *analysis.Pass) (interface{}, error) {
@@ -66,6 +68,15 @@
}
func runFunc(pass *analysis.Pass, node ast.Node) {
+ // Find scope of function node
+ var funcScope *types.Scope
+ switch v := node.(type) {
+ case *ast.FuncLit:
+ funcScope = pass.TypesInfo.Scopes[v.Type]
+ case *ast.FuncDecl:
+ funcScope = pass.TypesInfo.Scopes[v.Type]
+ }
+
// Maps each cancel variable to its defining ValueSpec/AssignStmt.
cancelvars := make(map[*types.Var]ast.Node)
@@ -114,7 +125,11 @@
"the cancel function returned by context.%s should be called, not discarded, to avoid a context leak",
n.(*ast.SelectorExpr).Sel.Name)
} else if v, ok := pass.TypesInfo.Uses[id].(*types.Var); ok {
- cancelvars[v] = stmt
+ // If the cancel variable is defined outside function scope,
+ // do not analyze it.
+ if funcScope.Contains(v.Pos()) {
+ cancelvars[v] = stmt
+ }
} else if v, ok := pass.TypesInfo.Defs[id].(*types.Var); ok {
cancelvars[v] = stmt
}
diff --git a/go/analysis/passes/lostcancel/testdata/src/a/a.go b/go/analysis/passes/lostcancel/testdata/src/a/a.go
index f9277ba..4aa3a61 100644
--- a/go/analysis/passes/lostcancel/testdata/src/a/a.go
+++ b/go/analysis/passes/lostcancel/testdata/src/a/a.go
@@ -171,3 +171,22 @@
ctx, cancel = context.WithCancel(bg)
return
}
+
+// Test for Go issue 31856.
+func _() {
+ var cancel func()
+
+ func() {
+ _, cancel = context.WithCancel(bg)
+ }()
+
+ cancel()
+}
+
+var cancel1 func()
+
+// Same as above, but for package-level cancel variable.
+func _() {
+ // We assume that other uses of cancel1 exist.
+ _, cancel1 = context.WithCancel(bg)
+}
