/cc @aclements @davidlazar

@gopherbot please open a backport tracking issue, as this is a 1.10 regression.

Backport issue(s) opened: #25041 (for 1.10).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

On examining the history of the runtime.Caller edits in extern.go [1]

1. At 0ea120a, the posted repro panics

$ go run main.go 
devel +0ea120a70c Tue Apr 18 19:56:30 2017 +0000

4717371 /home/emmanuel/Desktop/opensrc/bugs/golang/24108/main.go 24 true main.st.M
4718642 <autogenerated> 1 true main.(*st).M
4718747 <autogenerated> 1 true main.(*wrapper).M
4717275 /home/emmanuel/Desktop/opensrc/bugs/golang/24108/main.go 15 true main.missingFunction
4717145 /home/emmanuel/Desktop/opensrc/bugs/golang/24108/main.go 11 true main.main
4359668 /home/emmanuel/go/src/go.googlesource.com/go/src/runtime/proc.go 185 true runtime.main
4524096 /home/emmanuel/go/src/go.googlesource.com/go/src/runtime/asm_amd64.s 2347 true runtime.goexit
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4402e2]

goroutine 1 [running]:
main.st.M()
	/home/emmanuel/Desktop/opensrc/bugs/golang/24108/main.go:25 +0xaf
main.(*st).M(0x533078)
	<autogenerated>:1 +0x33
main.(*wrapper).M(0xc42000e2a0)
	<autogenerated>:1 +0x3c
main.missingFunction()
	/home/emmanuel/Desktop/opensrc/bugs/golang/24108/main.go:15 +0x6c
main.main()
	/home/emmanuel/Desktop/opensrc/bugs/golang/24108/main.go:11 +0xba
exit status 2

2. 032678e is the origin story of the current report

I've spent a while thinking about this one now, and I'm worried we've managed to back ourselves into a corner with the skip count and how tracebacking is currently structured. The logic behind ignoring frames in CallersFrames (internally, the stackExpander) is pretty complicated. However, the skip count isn't handled by stackExpander; it's handled much earlier, in gentraceback when it's walking the stack. That's why skipping N frames is different from getting the whole traceback and then discarding the first N frames.

It's hard to make these agree. I would love to make stack unwinding internally demand-driven (not unlike CallersFrames), but right now it's all sort of inside-out. Then the unwinder wouldn't have to worry about skips at all and runtime.Caller could just apply all of the frame filtering, discard the first N frames of the filtered stream, and return the next frame. But that's a really complicated change.

As a stop-gap, it may be possible to keep runtime.Caller efficient in the common case of small skip values by unwinding some of the stack without any skip and taking the Nth frame if there is one. If it turns out we didn't unwind enough of the stack, just unwind it again with a larger buffer. We could keep small unwinds on the stack and only heap-allocate for large skip values, which should be incredible rare.

The exact same problem affects runtime.Callers' skip count, and I think the solution there would be roughly the same.

This should now be fixed by https://go-review.googlesource.com/c/go/+/152537