search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.23k stars 17.7k forks source link

x/tools/gopls: crash in Hover (telemetry) #69362

Open adonovan opened 2 months ago

adonovan commented 2 months ago 
#!stacks
"runtime.sigpanic" && ("golang.hover:+170" || "golang.hover:+209")

This stack zUGLQA was reported by telemetry:

if def, ok := pkg.TypesInfo().Defs[ident]; ok && ident.Pos() == def.Pos() {

Looks like Defs[ident]=nil is an actual map entry.

crash/crash runtime.gopanic:+69 runtime.panicmem:=262 runtime.sigpanic:+19 golang.org/x/tools/gopls/internal/golang.hover:+170 golang.org/x/tools/gopls/internal/golang.Hover:+4 golang.org/x/tools/gopls/internal/server.(*server).Hover:+30 golang.org/x/tools/gopls/internal/protocol.serverDispatch:+335 golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5 golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52 golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2 golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3 runtime.goexit:+0

golang.org/x/tools/gopls@v0.16.1 go1.23.0 darwin/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks. Dups: ylB3Iw

gabyhelp commented 2 months ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

adonovan commented 2 months ago

I can't explain this one. The only three calls to recordDef(id, nil) in the type checker are for:

  1. package id
  2. switch id := expr.(type) {}
  3. the blank identifier in _ = expr.

It can't be (1) because the Hover logic handled the package clause specially at L186. It can't be (2) because this would cause referencedObject to return a non-nil third result (selectedType), causing an early return. And it can't be (3) because that would cause referencedObject not to return a non-nil Object, Ident pair, leading to an early return.

Clearly there is a mistake in my logic. But where?

findleyr commented 1 month ago

Agreed. I think the best we'll be able to do here is downgrade the panic into two or more bug reports that refine the panic.

This is not the first "can't happen" bug related to go/types...

gopherbot commented 1 week ago

Change https://go.dev/cl/627015 mentions this issue: gopls/internal/golang: refine crash report golang/go#69362

adonovan commented 2 days ago

This stack ylB3Iw was reported by telemetry: