x/tools/gopls: call to IsGenerated ocassionally has nil Snapshot causing panic

[kortschak]

What did you do?

This is sporadic, and I'm not entirely sure how to reliably repoduce it; my current approach to reproduction is to to open new files and make modifications. About 1/20 times, this will cause this crash.

I am using gopls with sublimelsp/LSP as recommended. ST3.

What did you expect to see?

No crash.

What did you see instead?

A crash.

gopls: panic: runtime error: invalid memory address or nil pointer dereference
gopls: [signal SIGSEGV: segmentation violation code=0x1 addr=0x50 pc=0x7e4dba]
gopls: 
gopls: goroutine 14460 [running]:
gopls: golang.org/x/tools/internal/lsp/source.IsGenerated(0xef2aa0, 0xc001af7140, 0x0, 0x0, 0xc013a48100, 0x32, 0x0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/source/util.go:90 +0x3a
gopls: golang.org/x/tools/internal/lsp.(*Server).didModifyFiles(0xc00010cbb0, 0xef2aa0, 0xc001af7140, 0xc020b65ef0, 0x1, 0x1, 0x1, 0x0, 0x0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/text_synchronization.go:244 +0xecf
gopls: golang.org/x/tools/internal/lsp.(*Server).didChange(0xc00010cbb0, 0xef2aa0, 0xc001af7140, 0xc017c50300, 0x0, 0x0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/text_synchronization.go:117 +0x266
gopls: golang.org/x/tools/internal/lsp.(*Server).DidChange(0xc00010cbb0, 0xef2aa0, 0xc001af7140, 0xc017c50300, 0xc017c50300, 0x0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/server_gen.go:36 +0x49
gopls: golang.org/x/tools/internal/lsp/protocol.serverDispatch(0xef2aa0, 0xc001af7140, 0xf065e0, 0xc00010cbb0, 0xc017c502d0, 0x7fc32cee4040, 0xc001af66f0, 0x0, 0xedff00, 0xc02e7d2dc0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/protocol/tsserver.go:120 +0x1a7e
gopls: golang.org/x/tools/internal/lsp/protocol.ServerHandler.func1(0xef2aa0, 0xc001af7140, 0xc017c502d0, 0x7fc32cee4040, 0xc001af66f0, 0x20af386554, 0x134ade0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/protocol/protocol.go:63 +0xc5
gopls: golang.org/x/tools/internal/lsp/lsprpc.handshaker.func1(0xef2aa0, 0xc001af7140, 0xc017c502d0, 0x7fc32cee4040, 0xc001af66f0, 0x0, 0x0)
gopls: /home/user/src/golang.org/x/tools/internal/lsp/lsprpc/lsprpc.go:557 +0x452
gopls: golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1(0xef2aa0, 0xc001af7140, 0xc0225f27e0, 0x7fc32cee4040, 0xc001af66f0, 0xc00a2cc4b0, 0xe17fc0)
gopls: /home/user/src/golang.org/x/tools/internal/jsonrpc2/handler.go:35 +0xcf
gopls: golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2(0xc019d9a0c0, 0xc017c85260, 0xc0006ba210, 0xef2aa0, 0xc001af7140, 0xc0225f27e0, 0x7fc32cee4040, 0xc001af66f0)
gopls: /home/user/src/golang.org/x/tools/internal/jsonrpc2/handler.go:103 +0x86
gopls: created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
gopls: /home/user/src/golang.org/x/tools/internal/jsonrpc2/handler.go:100 +0x173

This is a consequence of filling snapshotByURI with nil Snaphots here and then if a snapshot value is nil here leaving it there. Finally, the check for valid snaphots here uses map presence rather than Snapshot nil-ness as the check for validity.

Either the validity check in the generated code check loop should be for non-nil Snapshots

		snapshot := snapshotByURI[mod.URI]
		if snapshot == nil {
			continue
		}

or probably better, remove the snapshotByURI elements that are nil in the check above.

		// If the file isn't in any known views (for example, if it's in a dependency),
		// we may not have a snapshot to map it to. As a result, we won't try to
		// diagnose it. TODO(rstambler): Figure out how to handle this better.
		if snapshot == nil {
			delete(snapshotByURI, uri)
			continue
		}

Maybe both.

Build info

golang/tools@9647ced

golang.org/x/tools/gopls v0.5.1
    golang.org/x/tools/gopls@(devel)
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/google/go-cmp@v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
    golang.org/x/sync@v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
    golang.org/x/tools@v0.0.0-20200930165741-f1523d29dbb9 => ../
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=
    mvdan.cc/gofumpt@v0.0.0-20200802201014-ab5a8192947d h1:t8TAw9WgTLghti7RYkpPmqk4JtQ3+wcP5GgZqgWeWLQ=
    mvdan.cc/xurls/v2@v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=

[stamblerre]

Thank you for this report! I think this bug has been around for a while, but it was actually just reported by someone else, so I will close this as a duplicate of #41777.

[stamblerre]

Duplicate of #41777

[cespare]

Well that's quite the coincidence.