Skip to content

Commit

Permalink
passes/sigchanyzer: allow valid inlined unbuffered signal channel
Browse files Browse the repository at this point in the history
Permit signal.Notify(make(chan os.Signal)) which is valid code,
given that the channel isn't read elsewhere, the fact that signals
can be lost is unimportant.

Updates golang/go#45043

Change-Id: Ie984dfeedbb4e1e33a29391c3abb1fc83299fed3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/311729
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
  • Loading branch information
AGMETEOR authored and odeke-em committed Apr 22, 2021
1 parent 4934781 commit f946a15
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
18 changes: 18 additions & 0 deletions go/analysis/passes/sigchanyzer/sigchanyzer.go
Expand Up @@ -49,6 +49,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
chanDecl = decl
}
case *ast.CallExpr:
// Only signal.Notify(make(chan os.Signal), os.Interrupt) is safe,
// conservatively treate others as not safe, see golang/go#45043
if isBuiltinMake(pass.TypesInfo, arg) {
return
}
chanDecl = arg
}
if chanDecl == nil || len(chanDecl.Args) != 1 {
Expand Down Expand Up @@ -127,3 +132,16 @@ func findDecl(arg *ast.Ident) ast.Node {
}
return nil
}

func isBuiltinMake(info *types.Info, call *ast.CallExpr) bool {
typVal := info.Types[call.Fun]
if !typVal.IsBuiltin() {
return false
}
switch fun := call.Fun.(type) {
case *ast.Ident:
return info.ObjectOf(fun).Name() == "make"
default:
return false
}
}
16 changes: 16 additions & 0 deletions go/analysis/passes/sigchanyzer/testdata/src/a/a.go
Expand Up @@ -36,3 +36,19 @@ func j() {
f := signal.Notify
f(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
}

func k() {
signal.Notify(make(chan os.Signal), os.Interrupt) // ok
}

func l() {
signal.Notify(make(chan os.Signal, 1), os.Interrupt) // ok
}

func m() {
signal.Notify(make(chan ao.Signal, 1), os.Interrupt) // ok
}

func n() {
signal.Notify(make(chan ao.Signal), os.Interrupt) // ok
}
16 changes: 16 additions & 0 deletions go/analysis/passes/sigchanyzer/testdata/src/a/a.go.golden
Expand Up @@ -36,3 +36,19 @@ func j() {
f := signal.Notify
f(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
}

func k() {
signal.Notify(make(chan os.Signal), os.Interrupt) // ok
}

func l() {
signal.Notify(make(chan os.Signal, 1), os.Interrupt) // ok
}

func m() {
signal.Notify(make(chan ao.Signal, 1), os.Interrupt) // ok
}

func n() {
signal.Notify(make(chan ao.Signal), os.Interrupt) // ok
}

0 comments on commit f946a15

Please sign in to comment.