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/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
Labels
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

@perillo
Copy link
Contributor

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
Contributor Author

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
Contributor

/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 This label describes issues relating to any tools in the x/tools repository. label Feb 13, 2020
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 13, 2020
@perillo
Copy link
Contributor Author

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 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 as completed Feb 13, 2020
@josharian
Copy link
Contributor

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 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.

@golang golang locked and limited conversation to collaborators Feb 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
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
@josharian @dmitshur @perillo @gopherbot @matloob