runtime: CallersFrame returns incorrect frame for panics in inlined frame

[adonovan]

We've noticed a number of recent gopls panics in which the leaf function in the backtrace doesn't appear to contain a fallible operation.

• x/tools/gopls: Hover: panic in lookup{ObjectByName,DocLinkSymbol} #69616
• x/tools/gopls: OOB index crash in completion.expectedReturnStmtType #70636
• x/tools/gopls: Hover: nil deref in parseDocLink #70635
• x/tools/gopls: nil Signature (?) deref in Completion #70634
• x/tools/gopls: stubmethods: "could not find the enclosing function of the return statement" bug in GetIfaceStubInfo #70666

I wonder whether the pclntab is incorrect in these cases.

@findleyr

[mknyszek]

CC @golang/compiler

[findleyr]

The most stark example of this is https://go.dev/issue/70634, where the panic is immediately preceded by a nil check.

In that case, there is a plausible panic, on the same line (see my comment), which is correctly reported in #70636. Perhaps that provides a clue.

[prattmic]

For cases that don't appear to have a fallable operation at all, perhaps we could build the exact binary and dump out all of the PCs that report this location to see if anything there looks fallable.

[prattmic]

The most stark example of this is https://go.dev/issue/70634, where the panic is immediately preceded by a nil check.

In that case, there is a plausible panic, on the same line (see my comment), which is correctly reported in #70636. Perhaps that provides a clue.

In that case, one panic (the plausible one) is goPanicIndex while the other is panicmem, so it seems unlikely that these are the exact same panic but with different locations.

[adonovan]

For cases that don't appear to have a fallible operation at all, perhaps we could build the exact binary and dump out all of the PCs that report this location to see if anything there looks fallible.

Yeah, I had done this with the GetIfaceStubInfo crash, but nothing stood out. I've dumped the listing for that function at #70666 (comment) so you can take a look for yourself.

If I can reproduce it locally it would be interesting to see whether the panic backtrace corresponds to the telemetry trace.

[findleyr]

In that case, one panic (the plausible one) is goPanicIndex while the other is panicmem, so it seems unlikely that these are the exact same panic but with different locations.

types.Signature.results can be nil if there are no results, or have length less than the number of syntactic results in the return statement. So both the panicmem and goPanicIndex could have the same root cause.

[prattmic]

Another possibility not yet mentioned is memory corruption. However, given this are coming from multiple GOOS/GOARCH, it isn't just one user, and the relatively small scale of telemetry collection makes it seem unlikely there would be so many unique corrupt reporters.

[findleyr]

@prattmic I can reproduce the crash report of #70634 with gopls@v0.17.0-pre.3, and the repro from my fix for #70636.

Repro:

go install golang.org/x/tools/gopls@v0.17.0-pre.3

Paste this into a buffer:

var xx int
var xy string


func _() {
	return Foo(x) // <- trigger completion after 'x'
}

func Foo[T any](t T) T {}

Trigger completion where noted.

[findleyr]

@adonovan, @prattmic, and I looked into this together: looks like an off-by-one error for inlined calls in CallersFrames or the output fed to SetCrashOutput.