cmd/compile: ICE 'invalid defer call' due to CL 479095

[thanm]

What version of Go are you using (go version)?

$ go version
go version devel go1.21-8edcdddb23 Fri Mar 31 23:25:07 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

Only on tip.

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

$ git clone https://github.com/census-ecosystem/opencensus-go-exporter-stackdriver.git
$ cd opencensus-go-exporter-stackdriver
$ git checkout v0.13.5
...
$ go test -race -short ./...

What did you expect to see?

Clean build of test

What did you see instead?

# contrib.go.opencensus.io/exporter/stackdriver/monitoredresource
monitoredresource/monitored_resources.go:56:5: internal compiler error: invalid defer call: (*sync.WaitGroup).Done(wg)

goroutine 1 [running]:
runtime/debug.Stack()
	/ssd2/xgo/src/runtime/debug/stack.go:24 +0x5e
cmd/compile/internal/base.FatalfAt({0xe9ace0?, 0x0?}, {0xd4d827, 0x13}, {0xc0005515a8, 0x2, 0x2})
	/ssd2/xgo/src/cmd/compile/internal/base/print.go:234 +0x1d7
cmd/compile/internal/walk.walkGoDefer(0xc000656780)
	/ssd2/xgo/src/cmd/compile/internal/walk/stmt.go:207 +0xba
cmd/compile/internal/walk.walkStmt({0xe9b5e0, 0xc000656780?})
	/ssd2/xgo/src/cmd/compile/internal/walk/stmt.go:123 +0x3c5
cmd/compile/internal/walk.walkStmtList(...)
	/ssd2/xgo/src/cmd/compile/internal/walk/stmt.go:175
cmd/compile/internal/walk.Walk(0xc000654dc0)
	/ssd2/xgo/src/cmd/compile/internal/walk/walk.go:43 +0x14d
cmd/compile/internal/gc.prepareFunc(0xc000654dc0)
	/ssd2/xgo/src/cmd/compile/internal/gc/compile.go:105 +0x107
cmd/compile/internal/gc.enqueueFunc(0xc00011f8c0)
	/ssd2/xgo/src/cmd/compile/internal/gc/compile.go:71 +0x30a
cmd/compile/internal/gc.Main(0xd75678)
	/ssd2/xgo/src/cmd/compile/internal/gc/main.go:363 +0x1b0a
main.main()
	/ssd2/xgo/src/cmd/compile/main.go:57 +0xf9

?   	contrib.go.opencensus.io/exporter/stackdriver/internal/testpb	[no test files]
FAIL	contrib.go.opencensus.io/exporter/stackdriver [build failed]

At first blush what appears to be happening is that the code in escape analysis that "normalizes" defers (converts them via wrapping into calls to no-args/no-results functions) is missing a defer. More work needed to find out exactly why.

Note that CL 479095 has since been reverted upstream, so the bug will no longer manifest directly on tip.

[thanm]

This appears to be a problem in escape analysis and/or ir.VisitFuncsBottomUp.

Here is a somewhat smaller example to repro: https://go.dev/play/p/UUFnYaFwv50

Here's what typecheck.Target.Decls looks like just before escape analysis if inlining is disabled:

 Autodetect 
 If.MonitoredResource
 aad 
 Autodetect.func1.1.1 hidden=true
 Autodetect.func1.1.2 hidden=true
 Autodetect.func1.1 hidden=true
 Autodetect.func1 hidden=true
 init 
 Interface.MonitoredResource
 (*If).MonitoredResource

Note that all of the closures have the "hidden closure" flag set, since they are all reachable from the top level routine Autodetect.

Now here's what the same list looks like with inlining turned on:

 ./repro.go:34:3: inlining call to Autodetect.func1
<autogenerated>:1: inlining call to If.MonitoredResource
Autodetect
 If.MonitoredResource
 aad
 Autodetect.func1.1.1 hidden=true
 Autodetect.func1.1.2 hidden=true
 Autodetect.func1.1 hidden=true
 Autodetect.func1 hidden=true
 init
 Autodetect.func2.1 hidden=true
 Autodetect.func2.2 hidden=true
 Autodetect.func2 hidden=true
 Interface.MonitoredResource
 (*If).MonitoredResource

What's happened here is that when Autodetect.func1 is inlined, we wind up duplicating the closure functions within it (this happens here).

Once the call to Autodetect.func1 is complete, the closures it contains are no longer reachable from any top level routine, but they are still marked with the "Hidden" flag. This means that they get skipped by ir.VisitFuncsBottomUp, due to this code here.

Because they are not exposed to escape analysis, the code that canonicalizes go/defer (here) never kicks in, but since the functions are still on the top level Decls list, they still get compiled, thus the assert in walk.

Seems to me the best way to handle this would be in escape analysis, I'll look into cooking up a CL of some sort for that.

[gopherbot]

Change https://go.dev/cl/482355 mentions this issue: cmd/compile: deadcode unreferenced hidden closures in escape analysis

[gopherbot]

Change https://go.dev/cl/482715 mentions this issue: cmd/compile: mark unreference hidden closures dead

[gopherbot]

Change https://go.dev/cl/484859 mentions this issue: cmd/compile: rework marking of dead hidden closure functions

[gopherbot]

Change https://go.dev/cl/486377 mentions this issue: cmd/compile: rework marking of dead hidden closure functions