uber-go / nilaway

Static analysis tool to detect potential nil panics in Go code
Apache License 2.0
3.19k stars 66 forks source link

panic with go1.23.0 #269

Closed jayalane closed 3 months ago

jayalane commented 3 months ago

I just added nilaway and got my build clean (thanks, very nice code; I'm adding more devs to my project, so need to be less trusting of my callers :)

Went to upgrade to 1.23.0 next but the nilaway binary is failing on my repo:


rlcs(1:595)$ go install go.uber.org/nilaway/cmd/nilaway@latest
Tue Aug 13 17:55:37 PDT 2024a
/Users/chlane/go/src/github.paypal.com/chlane/rlproxyserv/rlcs
Tue Aug 13 17:55:37 PDT 2024
rlcs(1:596)$ nilaway ./...
panic: Cannot range over: func(yield func(E) bool)

goroutine 9900 [running]:
golang.org/x/tools/go/ssa.(*builder).rangeStmt(0x14039cece40, 0x14039cf8680, 0x14003b64420, 0x0)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2281 +0x698
golang.org/x/tools/go/ssa.(*builder).stmt(0x14039cece40, 0x14039cf8680, {0x10538dad0?, 0x14003b64420?})
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2505 +0x1ac
golang.org/x/tools/go/ssa.(*builder).stmtList(...)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:910
golang.org/x/tools/go/ssa.(*builder).stmt(0x14039cece40, 0x14039cf8680, {0x10538d950?, 0x140041f68d0?})
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2467 +0xbf0
golang.org/x/tools/go/ssa.(*builder).buildFromSyntax(0x14039cece40, 0x14039cf8680)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2579 +0x204
golang.org/x/tools/go/ssa.(*builder).buildFunction(0x10530e401?, 0x14039cf8680)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2533 +0x110
golang.org/x/tools/go/ssa.(*builder).iterate(0x14039cece40)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2521 +0x2c
golang.org/x/tools/go/ssa.(*Package).build(0x14039ce6c00)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2650 +0xa4
sync.(*Once).doSlow(0x14039031c70?, 0x14004a8b200?)
    /Users/chlane/sdk/go1.23.0/src/sync/once.go:76 +0xf8
sync.(*Once).Do(...)
    /Users/chlane/sdk/go1.23.0/src/sync/once.go:67
golang.org/x/tools/go/ssa.(*Package).Build(...)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/ssa/builder.go:2639
golang.org/x/tools/go/analysis/passes/buildssa.run(0x14039031ba0)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/analysis/passes/buildssa/buildssa.go:59 +0x270
golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce(0x14035d17a40)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/analysis/internal/checker/checker.go:775 +0x838
sync.(*Once).doSlow(0x0?, 0x0?)
    /Users/chlane/sdk/go1.23.0/src/sync/once.go:76 +0xf8
sync.(*Once).Do(...)
    /Users/chlane/sdk/go1.23.0/src/sync/once.go:67
golang.org/x/tools/go/analysis/internal/checker.(*action).exec(...)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/analysis/internal/checker/checker.go:691
golang.org/x/tools/go/analysis/internal/checker.execAll.func1(0x0?)
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/analysis/internal/checker/checker.go:679 +0x48
created by golang.org/x/tools/go/analysis/internal/checker.execAll in goroutine 9898
    /Users/chlane/go/pkg/mod/golang.org/x/tools@v0.18.0/go/analysis/internal/checker/checker.go:685 +0x148
Tue Aug 13 17:55:47 PDT 2024
/Users/chlane/go/src/github.paypal.com/chlane/rlproxyserv/rlcs
Tue Aug 13 17:55:47 PDT 2024
rlcs(1:597)$ 
jayalane commented 3 months ago

This seems to be fixed by this diff: (I edited x/tools to 1.23 and then did "go mod tidy" and then built it and tried again)


nilaway(1:607)$ git diff
diff --git a/go.mod b/go.mod
index 37daed9..03879f3 100644
--- a/go.mod
+++ b/go.mod
@@ -8,7 +8,7 @@ require (
    github.com/klauspost/compress v1.17.6
    github.com/stretchr/testify v1.8.4
    go.uber.org/goleak v1.3.0
-   golang.org/x/tools v0.18.0
+   golang.org/x/tools v0.23.0
 )

 require (
@@ -16,6 +16,7 @@ require (
    github.com/kr/text v0.2.0 // indirect
    github.com/pmezard/go-difflib v1.0.0 // indirect
    github.com/stretchr/objx v0.5.1 // indirect
-   golang.org/x/mod v0.15.0 // indirect
+   golang.org/x/mod v0.19.0 // indirect
+   golang.org/x/sync v0.7.0 // indirect
    gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/go.sum b/go.sum
index d2b4e9f..e7ad8e6 100644
--- a/go.sum
+++ b/go.sum
@@ -26,12 +26,12 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
-golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8=
-golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
-golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
-golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
-golang.org/x/tools v0.18.0 h1:k8NLag8AGHnn+PHbl7g43CtqZAwG60vZkLqgyZgIHgQ=
-golang.org/x/tools v0.18.0/go.mod h1:GL7B4CwcLLeo59yx/9UWWuNOW1n3VZ4f5axWfML7Lcg=
+golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8=
+golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
+golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
+golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg=
+golang.org/x/tools v0.23.0/go.mod h1:pnu6ufv6vQkll6szChhK3C3L/ruaIv5eBeztNG8wtsI=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
jayalane commented 3 months ago

If you like this: https://github.com/uber-go/nilaway/pull/270

yuxincs commented 3 months ago

Yeah this is related to range-over-func introduced in Go 1.23: https://go.dev/wiki/RangefuncExperiment. Thanks for the PR!

yuxincs commented 3 months ago

Fixed by #270