Skip to content
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.

Sign up for GitHub

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

Jump to bottom

x/tools/go/ssa: ssautil.Packages panics with the standard library #53604

Closed
mvdan opened this issue Jun 29, 2022 · 12 comments
Closed

x/tools/go/ssa: ssautil.Packages panics with the standard library #53604

mvdan opened this issue Jun 29, 2022 · 12 comments
Assignees
@timothy-king
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@mvdan
Copy link
Member

mvdan commented Jun 29, 2022

I am reproducing this with a development version of https://github.com/mvdan/unparam, at mvdan/unparam#64. All it really does is:

	cfg := &packages.Config{
		Mode:  packages.LoadSyntax,
		Tests: c.tests,
	}
	pkgs, err := packages.Load(cfg, args...)
	if err != nil {
		return nil, err
	}
	if packages.PrintErrors(pkgs) > 0 {
		return nil, fmt.Errorf("encountered errors")
	}

	prog, _ := ssautil.Packages(pkgs, 0)
	prog.Build()

where args is what I feed via the command line, and tests defaults to false. When I cd tip/src below, I enter the GOROOT for the Go version I'm currently running.

$ go version
go version devel go1.19-160414ca6a Tue Jun 28 21:01:39 2022 +0000 linux/amd64
$ go install mvdan.cc/unparam@cc9b2a7c1ddf60b6ea80199f4e5d0f3039ec01ae
go: downloading mvdan.cc/unparam v0.0.0-20220629152518-cc9b2a7c1ddf
$ go version -m $(which unparam)
/home/mvdan/go/bin/unparam: devel go1.19-160414ca6a Tue Jun 28 21:01:39 2022 +0000
	path	mvdan.cc/unparam
	mod	mvdan.cc/unparam	v0.0.0-20220629152518-cc9b2a7c1ddf	h1:eit9NI4NnFOViaBdtWF7EVZte9EoK79/B8K5T1LGEb4=
	dep	golang.org/x/mod	v0.6.0-dev.0.20220419223038-86c51ed26bb4	h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
	dep	golang.org/x/sys	v0.0.0-20220627191245-f75cf1eec38b	h1:2n253B2r0pYSmEV+UNCQoPfU/FiaizQEK5Gu4Bq4JE8=
	dep	golang.org/x/tools	v0.1.12-0.20220628192153-7743d1d949f1	h1:NHLFZ56qCjD+0hYY3kE5Wl40Z7q4Gn9Ln/7YU0lsGko=
	build	-compiler=gc
	build	CGO_ENABLED=1
	build	CGO_CFLAGS=
	build	CGO_CPPFLAGS=
	build	CGO_CXXFLAGS=
	build	CGO_LDFLAGS=
	build	GOARCH=amd64
	build	GOOS=linux
	build	GOAMD64=v3
$ cd tip/src
$ unparam ./path/...
panic: no types.Object for ast.Ident buf @ /home/mvdan/tip/src/os/dir_unix.go:32:3

goroutine 173 [running]:
golang.org/x/tools/go/ssa.(*Function).objectOf(0xc00047a000, 0xc000380360)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/func.go:28 +0x208
golang.org/x/tools/go/ssa.(*builder).addr(0x6716e0?, 0xc00047a000, {0x716228?, 0xc000380360}, 0x0)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:419 +0xbc5
golang.org/x/tools/go/ssa.(*builder).assignStmt(0x67c180?, 0xc00047a000, {0xc000069f10, 0x1, 0x663bc0?}, {0xc000069f30, 0x1, 0xc0006172c0?}, 0x1)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:1157 +0x40b
golang.org/x/tools/go/ssa.(*builder).stmt(0xc000617208?, 0xc00047a000, {0x715da8?, 0xc000891700?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2122 +0xf65
golang.org/x/tools/go/ssa.(*builder).stmtList(0xc000617218?, 0xc0003cc000?, {0xc000380460?, 0x2, 0x4b2287?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:911 +0x67
golang.org/x/tools/go/ssa.(*builder).stmt(0xc00047a000?, 0xc00047a000, {0x715ec8?, 0xc000208930?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2218 +0xefd
golang.org/x/tools/go/ssa.(*builder).buildFunctionBody(0x6ac600?, 0xc00047a000)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2327 +0x489
golang.org/x/tools/go/ssa.(*builder).expr0(0xc000617df0, 0xc0002f5a00, {0x716168?, 0xc000069f60?}, {0x7, {0x715a78, 0xc000804f00}, {0x0, 0x0}})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:640 +0x4f3
golang.org/x/tools/go/ssa.(*builder).expr(0x0?, 0xc0002f5a00, {0x716168?, 0xc000069f60?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:610 +0x18e
golang.org/x/tools/go/ssa.(*builder).assign(0xc0002f5a00?, 0x717568?, {0x716c08?, 0xc00029a480}, {0x716168?, 0xc000069f60?}, 0x0?, 0xc000617c58)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:583 +0x3e5
golang.org/x/tools/go/ssa.(*builder).compLit(0x715a28?, 0xc00047ebd0?, {0x717688, 0xc000804640}, 0xc000891740, 0x1, 0xc000617c58)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:1252 +0xb4b
golang.org/x/tools/go/ssa.(*builder).assign(0x6728e0?, 0xc000b63ef0?, {0x716c08?, 0xc00029a1e0}, {0x715fe8?, 0xc000891740?}, 0x58?, 0x0)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:566 +0x385
golang.org/x/tools/go/ssa.(*Package).build(0xc0000c6b00)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2477 +0xb71
sync.(*Once).doSlow(0xc000ba6940?, 0xc00054b620?)
	/home/mvdan/tip/src/sync/once.go:74 +0xc2
sync.(*Once).Do(...)
	/home/mvdan/tip/src/sync/once.go:65
golang.org/x/tools/go/ssa.(*Package).Build(...)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2413
golang.org/x/tools/go/ssa.(*Program).Build.func1(0x0?)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2398 +0x4c
created by golang.org/x/tools/go/ssa.(*Program).Build
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2397 +0x19c
$ unparam ./os/...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x38 pc=0x612005]

goroutine 400 [running]:
golang.org/x/tools/go/ssa.(*Program).packageLevelMember(...)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/source.go:184
golang.org/x/tools/go/ssa.(*builder).expr0(0xc0000c5df0, 0xc000f92000, {0x716228?, 0xc000ccaae0?}, {0x0, {0x0, 0x0}, {0x0, 0x0}})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:774 +0x2245
golang.org/x/tools/go/ssa.(*builder).expr(0x203000?, 0xc000f92000, {0x716228?, 0xc000ccaae0?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:610 +0x18e
golang.org/x/tools/go/ssa.(*builder).setCallFunc(0x7f303287c5b8?, 0x80?, 0xc000056800?, 0xc000f8c540)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:1006 +0x32f
golang.org/x/tools/go/ssa.(*builder).setCall(0x6716e0?, 0xc000b19d70?, 0xc000873440, 0xc000f8c540)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:1085 +0x34
golang.org/x/tools/go/ssa.(*builder).expr0(0xc0000c5df0, 0xc000f92000, {0x715f28?, 0xc000873440?}, {0x0, {0x0, 0x0}, {0x0, 0x0}})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:686 +0x2785
golang.org/x/tools/go/ssa.(*builder).expr(0x0?, 0xc000f92000, {0x715f28?, 0xc000873440?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:610 +0x18e
golang.org/x/tools/go/ssa.(*builder).stmt(0xc0000c5208?, 0xc000f92000, {0x716468?, 0xc000ccab20?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2161 +0x1872
golang.org/x/tools/go/ssa.(*builder).stmtList(0xc000c97ac0?, 0xc00018f160?, {0xc00035aaf0?, 0x1, 0x4b2287?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:911 +0x67
golang.org/x/tools/go/ssa.(*builder).stmt(0xc000f92000?, 0xc000f92000, {0x715ec8?, 0xc00067d470?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2218 +0xefd
golang.org/x/tools/go/ssa.(*builder).buildFunctionBody(0x6ac600?, 0xc000f92000)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2327 +0x489
golang.org/x/tools/go/ssa.(*builder).expr0(0xc0000c5df0, 0xc000a631e0, {0x716168?, 0xc00035ab00?}, {0x7, {0x715a78, 0xc000cc7880}, {0x0, 0x0}})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:640 +0x4f3
golang.org/x/tools/go/ssa.(*builder).expr(0x0?, 0xc000a631e0, {0x716168?, 0xc00035ab00?})
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:610 +0x18e
golang.org/x/tools/go/ssa.(*builder).assign(0xc000a631e0?, 0x717568?, {0x716c08?, 0xc0004a8540}, {0x716168?, 0xc00035ab00?}, 0xe8?, 0xc0011adc58)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:583 +0x3e5
golang.org/x/tools/go/ssa.(*builder).compLit(0x715a28?, 0xc0006bfce0?, {0x717688, 0xc000b294c0}, 0xc000873480, 0x1, 0xc0011adc58)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:1252 +0xb4b
golang.org/x/tools/go/ssa.(*builder).assign(0x6728e0?, 0xc000681590?, {0x716c08?, 0xc0004a8510}, {0x715fe8?, 0xc000873480?}, 0x98?, 0x0)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:566 +0x385
golang.org/x/tools/go/ssa.(*Package).build(0xc000530a00)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2477 +0xb71
sync.(*Once).doSlow(0xc0000ab910?, 0xc000687b60?)
	/home/mvdan/tip/src/sync/once.go:74 +0xc2
sync.(*Once).Do(...)
	/home/mvdan/tip/src/sync/once.go:65
golang.org/x/tools/go/ssa.(*Package).Build(...)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2413
golang.org/x/tools/go/ssa.(*Program).Build.func1(0x0?)
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2398 +0x4c
created by golang.org/x/tools/go/ssa.(*Program).Build
	/home/mvdan/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/builder.go:2397 +0x19c

I think I'm doing everything right - https://pkg.go.dev/golang.org/x/tools/go/ssa/ssautil@master#Packages says:

The packages must have been loaded from source syntax using the golang.org/x/tools/go/packages.Load function in LoadSyntax or LoadAllSyntax mode.

and I am using LoadSyntax. Note that this chunk of code using go/packages and go/ssa/ssautil has worked for years until generics support was introduced.

cc @timothy-king @findleyr per https://dev.golang.org/owners
@mvdan mvdan mentioned this issue Jun 29, 2022
Merged
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 29, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jun 29, 2022
@mvdan
Copy link
Member Author

mvdan commented Jun 29, 2022

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.

@dominikh
Copy link
Member

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.

@timothy-king
Copy link
Contributor

I'm not successfully reproducing this.

$ gotip version
go version devel go1.19-e5017a93fc Wed Jun 29 20:22:10 2022 +0000 linux/amd64
$ gotip install mvdan.cc/unparam@cc9b2a7c1ddf60b6ea80199f4e5d0f3039ec01ae
$ gotip version -m $(which unparam )
/usr/local/google/home/taking/go/bin/unparam: devel go1.19-e5017a93fc Wed Jun 29 20:22:10 2022 +0000
	path	mvdan.cc/unparam
	mod	mvdan.cc/unparam	v0.0.0-20220629152518-cc9b2a7c1ddf	h1:eit9NI4NnFOViaBdtWF7EVZte9EoK79/B8K5T1LGEb4=
	dep	golang.org/x/mod	v0.6.0-dev.0.20220419223038-86c51ed26bb4	h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
	dep	golang.org/x/sys	v0.0.0-20220627191245-f75cf1eec38b	h1:2n253B2r0pYSmEV+UNCQoPfU/FiaizQEK5Gu4Bq4JE8=
	dep	golang.org/x/tools	v0.1.12-0.20220628192153-7743d1d949f1	h1:NHLFZ56qCjD+0hYY3kE5Wl40Z7q4Gn9Ln/7YU0lsGko=
	build	-compiler=gc
	build	CGO_ENABLED=1
	build	CGO_CFLAGS=
	build	CGO_CPPFLAGS=
	build	CGO_CXXFLAGS=
	build	CGO_LDFLAGS=
	build	GOARCH=amd64
	build	GOOS=linux
	build	GOAMD64=v1
$ cd goroot/src/
$ git log -1 --oneline
e5017a93fc (HEAD -> master, origin/master, origin/HEAD) net/http: don't strip whitespace from Transfer-Encoding headers
$ unparam ./path/...
$

Am I missing something in the reproducer steps?

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 30, 2022
@mvdan
Copy link
Member Author

mvdan commented Jul 1, 2022

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 GOAMD64=v3 everywhere - I use it for make.bash, so it also propagates as a default for go build and go list. But re-doing everything with v1 still results in the same.

For the sake of reducing differences between our environments, here's a reproducer with Docker:

$ uname -a
Linux p14s 5.18.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 25 Jun 2022 20:22:01 +0000 x86_64 GNU/Linux
$ docker --version
Docker version 20.10.17, build 100c70180f
$ cat repro-tip.sh 
#!/bin/bash

docker run -i golang:1.18.2 <<-'SCRIPT'

	set -ex

	cd "$HOME"
	git clone --depth=1 https://go.googlesource.com/go gotip
	cd gotip/src
	./make.bash

	export PATH="$HOME/go/bin:$HOME/gotip/bin:$PATH"

	go version
	go install mvdan.cc/unparam@cc9b2a7c1ddf60b6ea80199f4e5d0f3039ec01ae

	unparam ./path/...

SCRIPT
$ ./repro-tip.sh 
[...]
+ go version
go version devel go1.19-c847a2c Fri Jul 1 01:36:45 2022 +0000 linux/amd64
+ go install mvdan.cc/unparam@cc9b2a7c1ddf60b6ea80199f4e5d0f3039ec01ae
go: downloading mvdan.cc/unparam v0.0.0-20220629152518-cc9b2a7c1ddf
go: downloading golang.org/x/tools v0.1.12-0.20220628192153-7743d1d949f1
go: downloading golang.org/x/sys v0.0.0-20220627191245-f75cf1eec38b
go: downloading golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4
+ unparam ./path/...
panic: no types.Object for ast.Ident buf @ /root/gotip/src/os/dir_unix.go:32:3

goroutine 242 [running]:
golang.org/x/tools/go/ssa.(*Function).objectOf(0xc0008c5d40, 0xc0002bd360)
	/go/pkg/mod/golang.org/x/tools@v0.1.12-0.20220628192153-7743d1d949f1/go/ssa/func.go:28 +0x208
[...]

Does that help reproduce the crash?

@timothy-king
Copy link
Contributor

Able to reproduce now. Thanks.

@mvdan
Copy link
Member Author

mvdan commented Jul 6, 2022

Great! I believe you can adapt the same script to reproduce the other panic, the nil pointer dereference on ./os/....

@timothy-king timothy-king self-assigned this Jul 21, 2022
@timothy-king timothy-king added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Jul 21, 2022
@timothy-king
Copy link
Contributor

A mitigation is to switch to packages.LoadAllSyntax (or equivalent). Still looking into the root cause.

@timothy-king
Copy link
Contributor

timothy-king commented Aug 9, 2022 •
edited

Some context:

var dirBufPool = sync.Pool{
	New: func() any {
		// The buffer must be at least a block long.
		buf := make([]byte, blockSize) // <--- This is what is being complained about.
		return &buf
	},
}

So a lambda from a global variable. The crashing *ssa.Function is named init$1 so consistent.

With LoadSyntax, other functions in the package like "readInt" have a nil syntax field, i.e. external.

Looks like we need to not expand lambdas called by init() if we do not have sufficient syntax for the Package.

I wonder if this ever worked or is a new problem. AFAIK LoadSyntax is a path less tested.

@timothy-king
Copy link
Contributor

Globals should only be initialized when there are []*ast.Files with syntax to work from, code.

Otherwise we need to handle cases like var g T = func() T { <body> }() where we do not have types.Info for <body>.

@gopherbot
Copy link

Change https://go.dev/cl/422639 mentions this issue: go/ssa: initialize globals when there is syntax

@timothy-king
Copy link
Contributor

More details: the bug seems to require the *ssa.Package to be created when []*ast.Files is empty but types.Info is non-nil. This requires at least 3 packages, x, y, and z s.t. x transitively imports y and y transitively imports z, and x and z are root packages loaded by packages.Packages. This is will construct a y that needtypes and needsrc while deps (by using ssautil.Packages) and isinitial can be false.

This explains why this only triggered on patterns containing ....

@mvdan
Copy link
Member Author

mvdan commented Aug 11, 2022

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!

@golang golang locked and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@timothy-king timothy-king

Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@dominikh @timothy-king @dr2chase @mvdan @gopherbot