-
Notifications
You must be signed in to change notification settings - Fork 17.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/tools/go/ssa: ssautil.Packages panics with the standard library #53604
Comments
cc @dominikh in case you've seen these before, as I seem to understand staticcheck's support for generics is fairly complete. It does not panic on these packages, interestingly enough, even though I seem to recall it uses (a fork of?) go/ssa. |
Sorry, I won't be of help here. go/ssa and go/ir have deviated a lot, and Staticcheck's package loading only relies on go/packages for getting dependency information, not parsing or type checking. |
I'm not successfully reproducing this.
Am I missing something in the reproducer steps? |
I just tried everything again on a newer Go tip, c847a2c, and I still get the same failures. So there's something different between your environment and mine that's key to reproducing the bug. At first I thought it could be my use of For the sake of reducing differences between our environments, here's a reproducer with Docker:
Does that help reproduce the crash? |
Able to reproduce now. Thanks. |
Great! I believe you can adapt the same script to reproduce the other panic, the nil pointer dereference on |
A mitigation is to switch to packages.LoadAllSyntax (or equivalent). Still looking into the root cause. |
Some context:
So a lambda from a global variable. The crashing With Looks like we need to not expand lambdas called by I wonder if this ever worked or is a new problem. AFAIK |
Globals should only be initialized when there are []*ast.Files with syntax to work from, code. Otherwise we need to handle cases like |
Change https://go.dev/cl/422639 mentions this issue: |
More details: the bug seems to require the This explains why this only triggered on patterns containing |
It's entirely possible that a version of this bug has existed for some time, though that would be slightly surprising, as I've never run into it before and unparam has used go/packages in this way for a few years now. Thanks for the detailed investigation! |
I am reproducing this with a development version of https://github.com/mvdan/unparam, at mvdan/unparam#64. All it really does is:
where
args
is what I feed via the command line, andtests
defaults tofalse
. When Icd tip/src
below, I enter the GOROOT for the Go version I'm currently running.I think I'm doing everything right - https://pkg.go.dev/golang.org/x/tools/go/ssa/ssautil@master#Packages says:
and I am using
LoadSyntax
. Note that this chunk of code usinggo/packages
andgo/ssa/ssautil
has worked for years until generics support was introduced.cc @timothy-king @findleyr per https://dev.golang.org/owners
The text was updated successfully, but these errors were encountered: