cmd/compile: yet another incorrect recover behavior due to defer a method wrapper

[cherrymui]

Go version

Go 1.20 - Go 1.24

Output of go env in your module/workspace:

darwin/arm64 (or any)

What did you do?

https://go.dev/play/p/D621WgLz6y2

package main

func callRecover() {
	if recover() != nil {
		println("recovered")
	}
}

type T int

func (*T) M() { callRecover() }

type S struct{ *T } // has a wrapper (*S).M wrapping (*T.M)

var p = &S{new(T)}

var fn = (*S).M // using a function pointer to force using the wrapper

func main() {
	defer fn(p)
	panic("XXX")
}

What did you see happen?

The panic is recovered. The program prints "recovered" and exits normally.

What did you expect to see?

Similar to #73917, it is expected to not recover.

This is pretty much the same as #73917, except that it defers a method call with a pointer receiver, whereas it is a value receiver in #73917. I'm filing a separate issue as

• it started to fail from a different commit,
• this actually passes on tip, but fails with Go 1.24.

Similarly, it panics as expected if inlining is disabled.

$ go run -gcflags=-l x3.go
panic: XXX

goroutine 1 [running]:
main.main()
	/tmp/x3.go:21 +0x68
exit status 2

Go 1.19 got it right. Go 1.18 and later fail. Bisection points to CL https://go.dev/cl/422235 , which apparently has a lot of effects (I could bisect further with GOEXPERIMENT=unified, but I'll defer it for later). Before that CL, the wrapper (*S).M does a tail call to (*T).M with no inlining. After that CL, the wrapper (*S).M inlines (*T).M, while keeping the WRAPPER attribute. Similar to #73916 and #73917, this causes the panic to be treated one frame too deep.

It's been the same behavior up to Go 1.24. On tip, it changes, since CL https://go.dev/cl/650455 . Before that CL, tail call is not used for that type of wrappers, because the wrapped function (*T).M is inlineable (and actually being inlined). After that CL, we start with emitting an OTAILCALL without the WRAPPER attribute, and later inlines the wrapped function into it, still without the WRAPPER attribute. This corrects the panic stack depth issue (while leaving the issue of the wrapper not being omitted from traceback #73747).

The proposed fix for #73747, CL 675916 (as of PS 1), sets the WRAPPER attribute, which would reintroduce this bug.

[gabyhelp]

Related Issues

• cmd/compile: incorrect recover behavior due to defer a method wrapper #73917
• cmd/compile: incorrect recover behavior due to defer wrapper #73916
• cmd/compile: mid-stack inlining in a method wrapper can cause recover to fail #23557 (closed)
• cmd/compile: "Open defer" of method call miscompiled at tip #45062 (closed)
• affected/package: builtin Inlining causes spec violation in recover() #54119 (closed)
• `defer recover()` does not recover from panic #53169 (closed)

Related Code Changes

• cmd/compile: mark caller as wrapper when tail call is inlined
• runtime: restore caller's frame pointer when recovering from panic
• runtime: don't elide wrapper functions that call panic or at TOS
• cmd/compile: don't inline functions that call recover

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

[zigo101]

Go 1.18 and later fail.

typo? It should be 1.20.

[gopherbot]

Change https://go.dev/cl/677675 mentions this issue: cmd/compile: Modify inline tree to avoid parent node for tail call site.