x/tools/gopls: Highlight: nil deref panic on CompositeLiteral with no type #68918
Comments
gopherbot
added
Tools
|
This appears to be a recent regression caused by https://go.dev/cl/597675. |
|
Change https://go.dev/cl/606535 mentions this issue: |
findleyr
added
NeedsFix
|
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 litHowever, 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: to trigger the panic. That offset corresponds to the 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 |
|
Even smaller minification: From types playground: |
|
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: I've filed an issue against the type checker: 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 #58833 (comment). |
|
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: |
|
Change https://go.dev/cl/617475 mentions this issue: |
The fix for golang/go#69092 produces an extra error message in Go 1.24. Ignore it. Updates golang/go#68918. Updates golang/go#69092. Change-Id: I41ecff6fe916a04ed943e29ad10184d340ef84d5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/617475 Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The text was updated successfully, but these errors were encountered: