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.

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/packages, go/ast: NewPackage causes SIGSEGV #37202

Closed
perillo opened this issue Feb 12, 2020 · 6 comments
Closed

x/tools/go/packages, go/ast: NewPackage causes SIGSEGV #37202

perillo opened this issue Feb 12, 2020 · 6 comments

Comments

@perillo
Copy link

@perillo perillo commented Feb 12, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14rc1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/manlio/sdk/go1.14rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/manlio/sdk/go1.14rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build928658787=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14rc1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14rc1
uname -sr: Linux 5.5.2-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.31.
gdb --version: GNU gdb (GDB) 8.3.1

What did you do?

With main.go from https://play.golang.org/p/d0Nm2xOm74F, and in a directory with a go.mod file:

go run main.go

What did you expect to see?

The program to exit normally.

What did you see instead?

The following stacktrace:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4d789b]

goroutine 1 [running]:
panic(0x648e20, 0x867330)
	/home/manlio/sdk/go1.14rc1/src/runtime/panic.go:1060 +0x420 fp=0xc00017d950 sp=0xc00017d8a8 pc=0x431df0
runtime.panicmem(...)
	/home/manlio/sdk/go1.14rc1/src/runtime/panic.go:212
runtime.sigpanic()
	/home/manlio/sdk/go1.14rc1/src/runtime/signal_unix.go:687 +0x3da fp=0xc00017d980 sp=0xc00017d950 pc=0x44792a
go/token.(*FileSet).file(0x0, 0x5e, 0xc000156400)
	/home/manlio/sdk/go1.14rc1/src/go/token/position.go:452 +0x2b fp=0xc00017d9c8 sp=0xc00017d980 pc=0x4d789b
go/token.(*FileSet).PositionFor(0x0, 0x5e, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/manlio/sdk/go1.14rc1/src/go/token/position.go:492 +0x61 fp=0xc00017da18 sp=0xc00017d9c8 pc=0x4d7b21
go/token.(*FileSet).Position(...)
	/home/manlio/sdk/go1.14rc1/src/go/token/position.go:503
go/ast.(*pkgBuilder).error(0xc00017dc48, 0x5e, 0xc000156400, 0x14)
	/home/manlio/sdk/go1.14rc1/src/go/ast/resolve.go:22 +0x6e fp=0xc00017dac0 sp=0xc00017da18 pc=0x4edaee
go/ast.(*pkgBuilder).errorf(0xc00017dc48, 0x5e, 0x68f5af, 0x13, 0xc00017dc38, 0x1, 0x1)
	/home/manlio/sdk/go1.14rc1/src/go/ast/resolve.go:26 +0x7f fp=0xc00017db08 sp=0xc00017dac0 pc=0x4edd1f
go/ast.NewPackage(0x0, 0xc00010d3b0, 0x0, 0x0, 0xc000208228, 0x1, 0x1)
	/home/manlio/sdk/go1.14rc1/src/go/ast/resolve.go:162 +0xa51 fp=0xc00017de58 sp=0xc00017db08 pc=0x4eec41
main.main()
	/home/manlio/src/go/src/bugs/ast-newpackage/main.go:37 +0x2d2 fp=0xc00017df88 sp=0xc00017de58 pc=0x612982
runtime.main()
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:203 +0x212 fp=0xc00017dfe0 sp=0xc00017df88 pc=0x434822
runtime.goexit()
	/home/manlio/sdk/go1.14rc1/src/runtime/asm_amd64.s:1375 +0x1 fp=0xc00017dfe8 sp=0xc00017dfe0 pc=0x460c51

goroutine 2 [force gc (idle)]:
runtime.gopark(0x69aeb8, 0x8705b0, 0x1411, 0x1)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:304 +0xe0 fp=0xc000030fb0 sp=0xc000030f90 pc=0x434c00
runtime.goparkunlock(...)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:310
runtime.forcegchelper()
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:253 +0xb7 fp=0xc000030fe0 sp=0xc000030fb0 pc=0x434ab7
runtime.goexit()
	/home/manlio/sdk/go1.14rc1/src/runtime/asm_amd64.s:1375 +0x1 fp=0xc000030fe8 sp=0xc000030fe0 pc=0x460c51
created by runtime.init.6
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:242 +0x35

goroutine 3 [GC sweep wait]:
runtime.gopark(0x69aeb8, 0x870740, 0x140c, 0x1)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:304 +0xe0 fp=0xc0000317a8 sp=0xc000031788 pc=0x434c00
runtime.goparkunlock(...)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:310
runtime.bgsweep(0xc00004c000)
	/home/manlio/sdk/go1.14rc1/src/runtime/mgcsweep.go:70 +0x9c fp=0xc0000317d8 sp=0xc0000317a8 pc=0x421d1c
runtime.goexit()
	/home/manlio/sdk/go1.14rc1/src/runtime/asm_amd64.s:1375 +0x1 fp=0xc0000317e0 sp=0xc0000317d8 pc=0x460c51
created by runtime.gcenable
	/home/manlio/sdk/go1.14rc1/src/runtime/mgc.go:214 +0x5c

goroutine 4 [GC scavenge wait]:
runtime.gopark(0x69aeb8, 0x870700, 0x140d, 0x1)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:304 +0xe0 fp=0xc000031f78 sp=0xc000031f58 pc=0x434c00
runtime.goparkunlock(...)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:310
runtime.bgscavenge(0xc00004c000)
	/home/manlio/sdk/go1.14rc1/src/runtime/mgcscavenge.go:237 +0xd0 fp=0xc000031fd8 sp=0xc000031f78 pc=0x420320
runtime.goexit()
	/home/manlio/sdk/go1.14rc1/src/runtime/asm_amd64.s:1375 +0x1 fp=0xc000031fe0 sp=0xc000031fd8 pc=0x460c51
created by runtime.gcenable
	/home/manlio/sdk/go1.14rc1/src/runtime/mgc.go:215 +0x7e

goroutine 17 [finalizer wait]:
runtime.gopark(0x69aeb8, 0x89aad8, 0x1410, 0x1)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:304 +0xe0 fp=0xc00002c758 sp=0xc00002c738 pc=0x434c00
runtime.goparkunlock(...)
	/home/manlio/sdk/go1.14rc1/src/runtime/proc.go:310
runtime.runfinq()
	/home/manlio/sdk/go1.14rc1/src/runtime/mfinal.go:175 +0xa3 fp=0xc00002c7e0 sp=0xc00002c758 pc=0x4179d3
runtime.goexit()
	/home/manlio/sdk/go1.14rc1/src/runtime/asm_amd64.s:1375 +0x1 fp=0xc00002c7e8 sp=0xc00002c7e0 pc=0x460c51
created by runtime.createfing
	/home/manlio/sdk/go1.14rc1/src/runtime/mfinal.go:156 +0x61
exit status 2

With go1.13.7 the stack trace of goroutine 1 is a bit different:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4d53fb]

goroutine 1 [running]:
panic(0x6546e0, 0x871280)
	/usr/lib/go/src/runtime/panic.go:722 +0x2c2 fp=0xc000119928 sp=0xc000119898 pc=0x42cca2
runtime.panicmem(...)
	/usr/lib/go/src/runtime/panic.go:199
runtime.sigpanic()
	/usr/lib/go/src/runtime/signal_unix.go:394 +0x3ec fp=0xc000119958 sp=0xc000119928 pc=0x4413cc
sync.(*RWMutex).RLock(...)
	/usr/lib/go/src/sync/rwmutex.go:48
go/token.(*FileSet).file(0x0, 0x5e, 0xc0000fa3a0)
	/usr/lib/go/src/go/token/position.go:452 +0x2b fp=0xc0001199a0 sp=0xc000119958 pc=0x4d53fb
go/token.(*FileSet).PositionFor(0x0, 0x5e, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/lib/go/src/go/token/position.go:492 +0x61 fp=0xc0001199f0 sp=0xc0001199a0 pc=0x4d5681
go/token.(*FileSet).Position(...)
	/usr/lib/go/src/go/token/position.go:503
go/ast.(*pkgBuilder).error(0xc000119c20, 0x5e, 0xc0000fa3a0, 0x14)
	/usr/lib/go/src/go/ast/resolve.go:22 +0x6e fp=0xc000119a98 sp=0xc0001199f0 pc=0x4eb91e
go/ast.(*pkgBuilder).errorf(0xc000119c20, 0x5e, 0x69fe23, 0x13, 0xc000119c10, 0x1, 0x1)
	/usr/lib/go/src/go/ast/resolve.go:26 +0x7f fp=0xc000119ae0 sp=0xc000119a98 pc=0x4ebb4f
go/ast.NewPackage(0x0, 0xc0000e2a50, 0x0, 0x0, 0xc0001030c8, 0x1, 0x1)
	/usr/lib/go/src/go/ast/resolve.go:162 +0xa51 fp=0xc000119e30 sp=0xc000119ae0 pc=0x4eca71
main.main()
	/home/manlio/src/go/src/bugs/ast-newpackage/main.go:37 +0x2d2 fp=0xc000119f60 sp=0xc000119e30 pc=0x616172
runtime.main()
	/usr/lib/go/src/runtime/proc.go:203 +0x21e fp=0xc000119fe0 sp=0xc000119f60 pc=0x42e9ee
runtime.goexit()
	/usr/lib/go/src/runtime/asm_amd64.s:1357 +0x1 fp=0xc000119fe8 sp=0xc000119fe0 pc=0x458b31

The error seems to be in

s.mutex.RLock()
@perillo
Copy link
Author

@perillo perillo commented Feb 12, 2020

I'm unable to reproduce the problem by using go/parser directly, so it may be a problem with golang.org/x/tools/go/packages.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Feb 13, 2020

/cc @matloob per owners.

@dmitshur dmitshur changed the title go/ast: NewPackage causes SIGSEGV x/tools/go/packages, go/ast: NewPackage causes SIGSEGV Feb 13, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 13, 2020
@gopherbot gopherbot added the Tools label Feb 13, 2020
@perillo
Copy link
Author

@perillo perillo commented Feb 13, 2020

The bug was with my code, sorry.
From packages documentation:

// Fset provides position information for Types, TypesInfo, and Syntax.
// It is set only when Types is set.

Originally I had the NeedTypes mode set, but removed it later.
However I think that packages should set Fset when NeedSyntax is set, too.

@matloob
Copy link
Contributor

@matloob matloob commented Feb 13, 2020

Okay, i think it's fair to want Fset set when NeedSyntax is, but I'd like to see more demand for it before I make an API change. I'll close this issue for now, if you want to open an issue about setting Fset when NeedSyntax is set, we can see if there are others who want it.

@matloob matloob closed this Feb 13, 2020
@josharian
Copy link
Contributor

@josharian josharian commented Feb 13, 2020

As a general point, I've definitely lost time debugging go/packages issues because I've failed to set NeedX when I've set NeedY and NeedY requires NeedX. I don't think I've hit this particular combination.

@matloob
Copy link
Contributor

@matloob matloob commented Feb 13, 2020

When that's happened (NeedY requires NeedX), it's been a bug, and we've fixed it. The principle of the Need fields are that if a need field says it's going to populate a field, that field should be populated (which sounds obvious, but we've had a number of bugs where that didn't happen, and I think that's what you were running into @josharian). I think the difference here is that the documentation is correct, but inconvenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.