x/tools/gopls: sigpanic in persistent.(*mapNode).forEach (bad SP)

[adonovan]

#!stacks
"runtime.sigpanic" && "persistent.(*mapNode).forEach:+1"

Issue created by stacks.

This stack looks wrong, perhaps another example of the off-by-one problems in the traceback?
(Is it possible that somehow the tree has become a cyclic graph and what we are seeing is a stack overflow? The line number supports that, but I would expect the failure in that case to occur in runtime.morestack_noctxt.)

func (node *mapNode) forEach(f func(key, value any)) {
	if node == nil {   // <--- sigpanic
		return
	}
	node.left.forEach(f)
	f(node.key, node.value.value)
	node.right.forEach(f)
}

This stack dVh6PA was reported by telemetry:

• crash/crash
• runtime.throw:+9
• runtime.sigpanic:+33
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+1
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+6
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+6
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+6
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+6
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+6
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4
• golang.org/x/tools/gopls/internal/util/persistent.(*mapNode).forEach:+4

golang.org/x/tools/[email protected] go1.23.2 linux/amd64 vscode (1)

[adonovan]

cc @prattmic

[gabyhelp]

Related Issues

• x/tools/gopls: xrefs.Index panics after "can't fail" pgf.PosRange #70446 (closed)
• x/tools/gopls: crash in Hover (telemetry) #69362
• x/tools/gopls: Hover: panic in lookup{ObjectByName,DocLinkSymbol} #69616
• x/tools/gopls: analysisinternal.ZeroValue crash on (presumably) unsafe.Pointer #70585 (closed)
• x/tools/gopls: OOM reported by fatal in runtime.mallocgc (windows) #70445 (closed)
• x/tools/gopls: stack overflow with type parameters (built with go1.18beta1) #50881 (closed)
• x/tools/gopls: CodeAction: nil panic in findMatchingDiagnostics #71028 (closed)
• x/tools/gopls: panic in token.File.Name within PackageDocHTML (telemetry) #70149 (closed)
• x/tools/gopls: out of memory (crash) #42889 (closed)
• x/tools/gopls: slice OOB panic in fixAST on array type #70745 (closed)

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

[prattmic]

Is it possible that somehow the tree has become a cyclic graph and what we are seeing is a stack overflow? The line number supports that, but I would expect the failure in that case to occur in runtime.morestack_noctxt.

I would expect to see morestack as well, though I need to double check to be certain. But besides that, the stack check should be at line +0, not +1. i.e., we report the call to morestack as the same line number as the func keyword.

[adonovan]

The only fallible code on the offending line is two parameter spills, so this can't be a stack overflow, it must be a bad SP.

	0x000e 00014 	L0169	TESTQ	AX, AX
	0x0011 00017 	L0169	JEQ	107
	0x0013 00019 	L0169	MOVQ	AX, golang.org/x/tools/gopls/internal/util/persistent.node+48(SP)
	0x0018 00024 	L0169	MOVQ	BX, golang.org/x/tools/gopls/internal/util/persistent.f+56(SP)
	0x001d 00029 	L0169	PCDATA	$3, $-1