x/tools/gopls: nil Signature (?) deref in Completion

[adonovan]

#!stacks
"runtime.sigpanic" && "go/types.(*Signature).Results:=94" && "expectedReturnStmtType"

Issue created by stacks.

// expectedReturnStmtType returns the expected type of a return statement.
// Returns nil if enclosingSig is nil.
func expectedReturnStmtType(enclosingSig *types.Signature, node *ast.ReturnStmt, pos token.Pos) types.Type {
	if enclosingSig != nil {
		if resultIdx := exprAtPos(pos, node.Results); resultIdx < len(node.Results) {
			return enclosingSig.Results().At(resultIdx).Type()
		}
	}
	return nil
}

This stack sZBuvA was reported by telemetry:

• crash/crash
• runtime.gopanic:+69
• runtime.panicmem:=262
• runtime.sigpanic:+19
• go/types.(*Signature).Results:=94
• golang.org/x/tools/gopls/internal/golang/completion.expectedReturnStmtType:+3
• golang.org/x/tools/gopls/internal/golang/completion.expectedCandidate:+37
• golang.org/x/tools/gopls/internal/golang/completion.Completion:+150
• golang.org/x/tools/gopls/internal/server.(*server).Completion:+19
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+193
• 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/[email protected] go1.23.3 darwin/arm64 other,vscode (10)

Dups: mCLZHw

[gabyhelp]

Related Issues

• x/tools/gopls: crash in Hover (telemetry) #69362
• x/tools/gopls: automated issue report (crash) - unhandled basic type #40956 (closed)
• x/tools/gopls: SignatureHelp bug reported by telemetry [_mSPWg] #64234 (closed)
• x/tools/gopls: out-of-bounds index in "stub methods" code action #64087 (closed)
• x/tools/gopls: crash during SignatureHelp #40230 (closed)
• x/tools/gopls: SignatureHelp bug reported by telemetry #64245 (closed)
• x/tools/gopls: panic in completion.enclosingFunction #49397 (closed)
• x/tools/gopls: analysisinternal.ZeroValue crash on (presumably) unsafe.Pointer #70585 (closed)
• x/tools/gopls: panic in deslice when completing unknown type in append #38091 (closed)
• x/tools/gopls: Hover: panic in lookupObjectByName #69616

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

[adonovan]

This is a strange one: the nil deref is dominated by a non-nil check.

[adonovan]

This stack mCLZHw was reported by telemetry:

• crash/crash
• runtime.gopanic:+69
• runtime.panicmem:=262
• runtime.sigpanic:+9
• go/types.(*Signature).Results:=94
• golang.org/x/tools/gopls/internal/golang/completion.expectedReturnStmtType:+3
• golang.org/x/tools/gopls/internal/golang/completion.expectedCandidate:+37
• golang.org/x/tools/gopls/internal/golang/completion.Completion:+150
• golang.org/x/tools/gopls/internal/server.(*server).Completion:+19
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+193
• 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/[email protected] go1.23.1 windows/amd64 vscode (2)

[adonovan]

Closely related to #70636.

[findleyr]

There's definitely a bug here though. The condition checked is against len(node.Results), but in the presence of ill typed code, the signature length need not match the length of the return statement.

if resultIdx := exprAtPos(pos, node.Results); resultIdx < len(node.Results) {
    return enclosingSig.Results().At(resultIdx).Type()
}

My guess is that the compiler is somehow reporting the panic from the wrong call.

[findleyr]

Aha, well the bug I spotted is exactly #70636.

Therefore, I'm not sure what this bug is. If it were a bug in the compiler's pclntab, I'd have expected it to always report the wrong function.

[findleyr]

For triaging: we expect that this is really a dupe of #70636, somehow obscured by a misreported stack from the compiler. I'll leave this in the v0.17.0 milestone, but if it does not recur in the next prerelease, where #70636 is fixed, I think we can safely transfer this to a compiler/runtime bug.

[findleyr]

FWIW, I can reproduce this exact crash with [email protected], and the repro from my fix for #70636. Therefore, this is a dupe.

[adonovan]

https://go.dev/cl/634038 is a reproducer for the problem that doesn't depend on telemetry.