golang
x/tools/gopls: Highlight: nil deref panic on CompositeLiteral with no type
/Users/adonovan/go/bin/gopls: devel go1.24-760b722c34 Fri Aug 9 14:54:31 2024 +0000
path golang.org/x/tools/gopls
mod golang.org/x/tools/gopls (devel)
dep github.com/BurntSushi/toml v1.2.1
dep github.com/google/go-cmp v0.6.0
dep golang.org/x/exp/typeparams v0.0.0-20221212164502-fae10dda9338
dep golang.org/x/mod v0.20.0
dep golang.org/x/sync v0.8.0
dep golang.org/x/telemetry v0.0.0-20240712210958-268b4a8ec2d7
dep golang.org/x/text v0.17.0
dep golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d
=> ../ (devel)
dep golang.org/x/vuln v1.0.4
dep honnef.co/go/tools v0.4.7
dep mvdan.cc/gofumpt v0.6.0
dep mvdan.cc/xurls/v2 v2.5.0
...
build vcs.revision=7cc3be7d11326f7aa5fc7a51166590788049d072
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x20 pc=0x100b57bb4]
goroutine 101 gp=0x14000602700 m=6 mp=0x14000500008 [running]:
panic({0x100ff4840?, 0x1017acf00?})
/Users/adonovan/w/goroot/src/runtime/panic.go:804 +0x154 fp=0x140050f8ba0 sp=0x140050f8af0 pc=0x100336bc4
runtime.panicmem(...)
/Users/adonovan/w/goroot/src/runtime/panic.go:262
runtime.sigpanic()
runtime.sigpanic()
e[16:00:36.275] <-- textDocument/hover[4] {"jsonrpc":"2.0","result":{"contents":{"kind":"plaintext","value":"func (Node) Common() *Common"},"range":{"start":{"line":144,"character":15},"end":{"line":144,"character":21}}},"id":4}
i[16:00:36.275] anxious continuation to 4 can't run, held up by (6)
/Users/adonovan/w/goroot/src/runtime/signal_unix.go:900 +0x300 fp=0x140050f8c00 sp=0x140050f8ba0 pc=0x100339060
golang.org/x/tools/gopls/internal/golang.highlightIdentifier.func3({0x10112a448?, 0x14003074480})
/Users/adonovan/w/xtools/gopls/internal/golang/highlight.go:562 +0x264 fp=0x140050f8cc0 sp=0x140050f8c10 pc=0x100b57bb4
go/ast.inspector.Visit(0x140009c8440, {0x10112a448?, 0x14003074480?})
/Users/adonovan/w/goroot/src/go/ast/walk.go:361 +0x38 fp=0x140050f8ce0 sp=0x140050f8cc0 pc=0x100648638
go/ast.Walk({0x101125e80?, 0x140009c8440?}, {0x10112a448, 0x14003074480})
/Users/adonovan/w/goroot/src/go/ast/walk.go:34 +0x44 fp=0x140050f8e50 sp=0x140050f8ce0 pc=0x100645404
go/ast.Walk({0x101125e80?, 0x140009c8440?}, {0x10112a5b0, 0x14004270740})
/Users/adonovan/w/goroot/src/go/ast/walk.go:127 +0x1db4 fp=0x140050f8fc0 sp=0x140050f8e50 pc=0x100647174
go/ast.walkList[...](...)
/Users/adonovan/w/goroot/src/go/ast/walk.go:21
go/ast.Walk({0x101125e80?, 0x140009c8440?}, {0x10112a808, 0x140030744c0})
/Users/adonovan/w/goroot/src/go/ast/walk.go:194 +0x2884 fp=0x140050f9130 sp=0x140050f8fc0 pc=0x100647c44
go/ast.walkList[...](...)
/Users/adonovan/w/goroot/src/go/ast/walk.go:21
go/ast.Walk({0x101125e80?, 0x140009c8440?}, {0x10112a420, 0x1400426fdd0})
/Users/adonovan/w/goroot/src/go/ast/walk.go:211 +0x2978 fp=0x140050f92a0 sp=0x140050f9130 pc=0x100647d38
go/ast.Walk({0x101125e80?, 0x140009c8440?}, {0x10112aab0, 0x1400426fe00})
/Users/adonovan/w/goroot/src/go/ast/walk.go:332 +0xd78 fp=0x140050f9410 sp=0x140050f92a0 pc=0x100646138
go/ast.walkList[...](...)
/Users/adonovan/w/goroot/src/go/ast/walk.go:21
go/ast.Walk({0x101125e80?, 0x140009c8440?}, {0x101129958, 0x1400410ea00})
/Users/adonovan/w/goroot/src/go/ast/walk.go:341 +0x2d88 fp=0x140050f9580 sp=0x140050f9410 pc=0x100648148
go/ast.Inspect(...)
/Users/adonovan/w/goroot/src/go/ast/walk.go:372
golang.org/x/tools/gopls/internal/golang.highlightIdentifier(0x1400426c280, 0x1400410ea00, 0x14000635da0, 0x140037206c0)
/Users/adonovan/w/xtools/gopls/internal/golang/highlight.go:540 +0x208 fp=0x140050f95d0 sp=0x140050f9580 pc=0x100b57918
golang.org/x/tools/gopls/internal/golang.highlightPath({0x140040ba300, 0xe, 0x10}, 0x1400410ea00, 0x14000635da0)
/Users/adonovan/w/xtools/gopls/internal/golang/highlight.go:106 +0x280 fp=0x140050f9650 sp=0x140050f95d0 pc=0x100b555b0
Related Issues and Documentation
- x/tools/gopls: InlayHint: nil panic due to ast.CallExpr.Fun with no type #67142 (closed)
- x/tools/gopls: crash due to panic: invalid Pos value (go/token/position.go:281) #47231 (closed)
- x/tools/gopls: panic in analysis #60551
- x/tools/gopls: automated issue report (crash) #40449 (closed)
- x/tools/gopls: panic in MethodSets: nil data for package #59138 (closed)
- x/tools/gopls: panic in internal/lsp/debug #53793 (closed)
- x/tools/gopls: automated issue report (crash) #48763 (closed)
- x/tools/gopls: crash in filepath.Walk during go.work completion #64225 (closed)
- text/template: panics encountered during function calls are hard to treat and debug #28242 (closed)
- x/tools/gopls: illegal line number panic #37226 (closed)
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
This appears to be a recent regression caused by https://go.dev/cl/597675.
Change https://go.dev/cl/606535 mentions this issue: gopls/internal/golang: make sure CompositeLit node is non-nil
I've been trying all week to come up with a good repro for this, but to no avail. What I have found is that it consistently crashes in an if-statement that has been misparsed as a CompositeLit:
if decl.Tok == token.CONST { <---- this "{" is the start of a CompositeLit with no Type
info.declare(id, &Const{ <-- the declare call is Elt[0] of the CompositeLit
symbol: symbol{
block: block,
name: id.Name,
},
expr: lazye,
})
} else { <----this "}" is the end of the lit
However, the edits I was making were about 1000 lines up the file (e.g. an extra "}" inserted by mistake) so it looks like a cascading failure of parser recovery. (Nonetheless, the CompositeLiteral should have a type.)
Yay, I have a deterministic repro of the crash using the gopls CLI. Unfortunately the file is very large and nonpublic so I will need to bisect it down a bit first.
OK, I reduced it from about 2000 lines to around 1KB by manual bisection and some lucky guesses. (The result is a total mess, but that's not the point.)
Write this file into /tmp/a.go:
package p
func f() {
constant. MakeFromLiteral(e.Valuelit string,
case *ast.ArrayType:
if e.Len != nil {
mlen, _, klen := f(info, ictx, e.Len, env)
if mlen != ValueMode || klen == nil || klen.Kind() != constant.Int {
if mlen != ValueMode || klen == nil || klen.Kind() != constant.Int {
}
} else {
}
}
}
func g(info *Info, block *Block, stmt ast.Stmt) {
if stmt, ok := stmt.(*ast.AssignStmt); ok && stmt.Tok == token.DEFINE {
for i, lhs := range stmt.Lhs {
}
}
}
and run this command:
$ gopls highlight /tmp/a.go:#462
to trigger the panic. That offset corresponds to the lhs ident in the final for loop, which, according to the types playground, has this enclosing path (along with a huge cascade of parse and type errors):
[0] *ast.Ident @ 19:10-19:13 (#460-463)
[1] *ast.CompositeLit @ 18:59-0:0 (#436-0)
[2] *ast.BinaryExpr @ 18:47-0:0 (#424-0)
[3] *ast.BinaryExpr @ 18:41-0:0 (#418-0)
[4] *ast.CallExpr @ 4:2-0:0 (#23-0)
[5] *ast.ExprStmt @ 4:2-0:0 (#23-0)
[6] *ast.BlockStmt @ 3:10-0:0 (#20-0)
[7] *ast.FuncDecl @ 3:1-0:0 (#11-0) Type.Scope={}
[8] *ast.File @ 1:1-0:0 (#0-0) Scope={}
Poking around in the types playground reveals what a fascinating mess the parser makes of this input. The malformed MakeFromLiteral call on line 6 seems to have a subtree that is a BinaryExpr (the final &&), which suggests that && stmt.Tok == token.DEFINE {...} is being parsed as an expression outside of an if-statement context where the "{" would indicate a BodyStmt; instead it looks like a CompositeLit of a bogus type.
Even smaller minification:
package p
var _ = g(if c { { lhs
From types playground:
Pretty-printed:
g(BadExpr, c{{lhs}})
Syntax:
0 *ast.CallExpr {
1 . Fun: *ast.Ident {
2 . . NamePos: play.go:3:9
3 . . Name: "g"
4 . . Obj: nil
5 . }
6 . Lparen: play.go:3:10
7 . Args: []ast.Expr (len = 2) {
8 . . 0: *ast.BadExpr {
9 . . . From: play.go:3:11
10 . . . To: play.go:3:11
11 . . }
12 . . 1: *ast.CompositeLit {
13 . . . Type: *ast.Ident {
14 . . . . NamePos: play.go:3:14
15 . . . . Name: "c"
16 . . . . Obj: nil
17 . . . }
18 . . . Lbrace: play.go:3:16
19 . . . Elts: []ast.Expr (len = 1) {
20 . . . . 0: *ast.CompositeLit {
21 . . . . . Type: nil
22 . . . . . Lbrace: play.go:3:18
23 . . . . . Elts: []ast.Expr (len = 1) {
24 . . . . . . 0: *ast.Ident {
25 . . . . . . . NamePos: play.go:3:19
26 . . . . . . . Name: "lhs"
27 . . . . . . . Obj: nil
28 . . . . . . }
29 . . . . . }
30 . . . . . Rbrace: play.go:3:23
31 . . . . . Incomplete: false
32 . . . . }
33 . . . }
34 . . . Rbrace: play.go:3:23
35 . . . Incomplete: false
36 . . }
37 . }
38 . Ellipsis: -
39 . Rparen: play.go:3:23
40 }
The issue seems to be that, although the outer CompositeLit has a type of Invalid recorded for it, the type checker is not descending into the inner literal and recording Invalid for it too.
This appears to be an even more minimal case, using valid syntax:
var _ = T{{ x }}
I've filed an issue against the type checker:
- https://github.com/golang/go/issues/69092
Independently, the other issue is that the parser swallows thousands of lines of text while recovering from a call. I've added a note to the existing issue https://github.com/golang/go/issues/58833#issuecomment-2313308593.
So, the gopls logic is actually correct; the bugs are in the parser and type-checker. The fix in https://go.dev/cl/606535 is a workaround, and an appropriate one for this particular issue, though I suspect there may be a great many places in the gopls logic that similarly assume every CompositeLit has a type and that could crash if invoked on ill-formed code. Let's proceed with 606535 and add workarounds for the others as they come up. The underlying issues are not new, so it appears to be rare.
Change https://go.dev/cl/612042 mentions this issue: gopls/internal/golang: Highlight: work around go/types bug
Change https://go.dev/cl/617475 mentions this issue: gopls/internal/test/marker: update regression test issue68918.txt