x/tools/cmd/gopls: nil dereference of fToken in cache view

[pwaller]

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

$ go version
go version devel +4b3f04c63b Thu Jan 10 18:15:48 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes, I am testing on 7f7074d (master at time of writing) with the recent patch for URI escaping applied.

git fetch https://go.googlesource.com/tools refs/changes/77/161077/2 && git cherry-pick FETCH_HEAD

What did you do?

Sorry, I don't have an exact sequence of steps to reproduce. I feel that the stack trace below may be useful to someone nonetheless.

What did you expect to see?

gopls continue to work.

What did you see instead?

gopls exited with a segfault.

[Trace - 2:12:31 PM] Sending notification 'textDocument/didOpen'.
Params: {"textDocument":{"uri":"file:///home/pwaller/.local/src/github.com/pwaller/example/testdata/a/b/b.go","languageId":"go","version":1,"text":""}}

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

goroutine 100466 [running]:
go/token.(*File).Name(...)
	/home/pwaller/.local/src/go/src/go/token/position.go:110
golang.org/x/tools/internal/lsp/cache.(*View).parse(0xc0001df710, 0xc029285c20, 0x49, 0xc02ddb4320, 0x7fcdda8ed1d0)
	/home/pwaller/.local/src/golang.org/x/tools/internal/lsp/cache/view.go:116 +0x3c9
golang.org/x/tools/internal/lsp/cache.(*File).GetPackage(0xc02ddb4320, 0x0, 0x0, 0x0)
	/home/pwaller/.local/src/golang.org/x/tools/internal/lsp/cache/file.go:71 +0x150
golang.org/x/tools/internal/lsp/source.Diagnostics(0xa77d00, 0xc0000bc010, 0xa77080, 0xc0001df710, 0xc029285c20, 0x49, 0x0, 0x0, 0x0)
	/home/pwaller/.local/src/golang.org/x/tools/internal/lsp/source/diagnostics.go:28 +0xb8
golang.org/x/tools/internal/lsp.(*server).cacheAndDiagnose.func1(0xa77d00, 0xc0000bc010, 0xc0000c4940, 0xc029285c20, 0x49)
	/home/pwaller/.local/src/golang.org/x/tools/internal/lsp/diagnostics.go:24 +0xb4
created by golang.org/x/tools/internal/lsp.(*server).cacheAndDiagnose
	/home/pwaller/.local/src/golang.org/x/tools/internal/lsp/diagnostics.go:23 +0x14f

The stack trace seems to indicate that fToken is nil:

https://github.com/golang/tools/blob/7f7074d5bcfd282eb16bc382b0bb3da762461985/internal/lsp/cache/view.go#L116

/cc @stamblerre

[stamblerre]

I'll add some additional error handling in this function, but this seems like a really strange error. Do you mind sharing what this example/testdata/a/b/b.go file looks like and what package it's in? It's very strange that go/packages would succeed but then return invalid ASTs.

/cc @ianthehat

[pwaller]

I must warn that I don't know when the crash occurred precisely, so the file could have been in an intermediate state of completeness at the time of crash for all I know.

It's not very interesting:

github.com/pwaller/example/testdata/a/b/b.go

package b

func B() {
	println("B")
}

github.com/pwaller/example/testdata/a/a.go

package a

import "github.com/pwaller/example/testdata/a/b"

func A() {
	b.B()
}

[stamblerre]

Are you using modules?

[pwaller]

@stamblerre yes, and I have GOPATH and GO111MODULE=on set. I'm working in the same location as the GOPATH.

[stamblerre]

Can you try running:

PWD=<the directory you open in VSCode> go list -e -json -compiled -test=true -export=true -deps=true -find=false -- <absolute path to package github.com/pwaller/example/testdata/a/b>

[pwaller]

If I cd to the directory, I get this:

{
	"Dir": "/home/pwaller/.local/src/github.com/pwaller/example/testdata/a/b",
	"ImportPath": "github.com/pwaller/example/testdata/a/b",
	"Name": "b",
	"Export": "/home/pwaller/.cache/go-build/17/17ee798024e64617c899c103610bb14bee6494f44fc94570d728092a1ad03848-d",
	"Module": {
		"Path": "github.com/pwaller/example",
		"Main": true,
		"Dir": "/home/pwaller/.local/src/github.com/pwaller/example",
		"GoMod": "/home/pwaller/.local/src/github.com/pwaller/example/go.mod",
		"GoVersion": "1.12"
	},
	"Match": [
		"/home/pwaller/.local/src/github.com/pwaller/example/testdata/a/b"
	],
	"Stale": true,
	"StaleReason": "not installed but available in build cache",
	"GoFiles": [
		"b.go"
	],
	"CompiledGoFiles": [
		"b.go"
	]
}

If I run the command from my home directory pass PWD in through the environment as you suggested, then first I am prompted to enter the password for my SSH key, and if I abort, I get this:

PWD=$HOME/.local/src/github.com/pwaller/example go list -e -json -compiled -test=true -export=true -deps=true -find=false -- $HOME/.local/src/github.com/pwaller/example/testdata/a/a.go
go build github.com/pwaller/example/testdata/a/b: no Go files in 
{
	"ImportPath": "github.com/pwaller/example/testdata/a/b",
	"DepOnly": true,
	"Incomplete": true,
	"Stale": true,
	"StaleReason": "build ID mismatch",
	"Error": {
		"ImportStack": [
			"main",
			"github.com/pwaller/example/testdata/a/b"
		],
		"Pos": ".local/src/github.com/pwaller/example/testdata/a/a.go:3:8",
		"Err": "unknown import path \"github.com/pwaller/example/testdata/a/b\": cannot find module providing package github.com/pwaller/example/testdata/a/b"
	}
}
{
	"Dir": "/home/pwaller/.local/src/github.com/pwaller/example/testdata/a",
	"ImportPath": "command-line-arguments",
	"Name": "a",
	"Module": {
		"Path": "command-line-arguments",
		"Main": true
	},
	"Match": [
		"/home/pwaller/.local/src/github.com/pwaller/example/testdata/a/a.go"
	],
	"Incomplete": true,
	"GoFiles": [
		"a.go"
	],
	"Imports": [
		"github.com/pwaller/example/testdata/a/b"
	],
	"Deps": [
		"github.com/pwaller/example/testdata/a/b"
	],
	"DepsErrors": [
		{
			"ImportStack": [
				"main",
				"github.com/pwaller/example/testdata/a/b"
			],
			"Pos": ".local/src/github.com/pwaller/example/testdata/a/a.go:3:8",
			"Err": "unknown import path \"github.com/pwaller/example/testdata/a/b\": cannot find module providing package github.com/pwaller/example/testdata/a/b"
		}
	]
}

[pwaller]

I note that if I do not have my ssh keyring unlocked, I am frequently prompted to unlock it while using vscode-go.

I have the following git configuration, as suggested in various places such as #6968. It has the effect of allowing go get to access private repositories using the SSH key.

[url "ssh://[email protected]/"]
	insteadOf = https://github.com/
	insteadOf = git://github.com/

[matloob]

@bcmills (who understands modules much much better than I)

If you enter the password for your ssh key, everything works fine right? I think the problem is that gopls uses go/packages to get information about the modules you're in, which uses go list, and go list needs to download modules from github. Also see #29452. I think that's pretty much unavoidable. If git can't get your dependencies without user interaction go/packages and gopls won't be able to function correctly (of course they shouldn't panic!).

I'm not sure what we do here. Maybe we'll have to have go/packages tell go list to run in a special read-only mode?

[pwaller]

Yeah, I really don't like the idea of my editor doing network activity to fulfill completion lists, etc. Sometimes I use a mobile network and the latency can measure in the tens of seconds. I really want this to work with a mobile network or offline.

[matloob]

We thought about this some more, I think we have to close this as "Working as Intended/Unfortunate".

Ideally if you've already run "go mod tidy" in the module you're working in, go list won't need to make any network access, so gopls should work fine on a mobile network/offline. If it doesn't, that's probably something we can fix in go list.
If dependencies are missing, gopls just can't work because it can't resolve module dependencies without the network. That's inherent in the design of go modules.

Another way around this is to run a local module proxy on your workstation serving the set of dependency modules you need. That will let you avoid making network access. Of course, even with this solution, you still need to have all the dependencies you need on your local machine.

[pwaller]

This issue was hit in a case where all the dependencies were present, as far as I know. That is, I have run go install before, and that doesn't result in network access.

At the very least, it's coming as a surprise to me that not all of my dependencies were present, if that was somehow the case.

I would prefer if my editor informed me if it needs to do network activity in order to do useful work. Doing it in the background is quite counter-intuitive!

Also, is the nil dereference not still an issue which should remain open? Is there not a better failure mode to give to the user?

[stamblerre]

@pwaller: I added some additional logging in https://golang.org/cl/161218, so the error shouldn't come up again. The nil pointer is strange enough to me that it indicates that the go list issue above was the problem - it looks like some malformed results came out of go/packages, which caused the nil pointer exceptions.

[pwaller]

Thanks for the update @stamblerre, and thanks too all for quickly addressing issues as I report them 🎉

Also, go mod tidy did actually add a dependency introduced by a test of a transitive dependency which I was unaware of. I hadn't realized this possibility because go install appeared to complete without network activity.

Hopefully the behaviour I'm finding troubling is simply teething pain. :)