Skip to content

Commit f1caf1a

Browse files
committed
cmd/compile: deadcode unreferenced hidden closures during inlining
When a closure is inlined, it may contain other hidden closures, which the inliner will duplicate, rendering the original nested closures as unreachable. Because they are unreachable, they don't get processed in escape analysis, meaning that go/defer statements don't get rewritten, which can then in turn trigger errors in walk. This patch looks for nested hidden closures and marks them as dead, so that they can be skipped later on in the compilation flow. NB: if during escape analysis we rediscover a hidden closure (due to an explicit reference) that was previously marked dead, revive it at that point. Fixes #59404. Change-Id: I76db1e9cf1ee38bd1147aeae823f916dbbbf081b Reviewed-on: https://go-review.googlesource.com/c/go/+/482355 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]> Run-TryBot: Than McIntosh <[email protected]> Reviewed-by: Cuong Manh Le <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
1 parent d7d235c commit f1caf1a

File tree

3 files changed

+122
-0
lines changed

3 files changed

+122
-0
lines changed

src/cmd/compile/internal/inline/inl.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,19 @@ func InlineDecls(p *pgo.Profile, decls []ir.Node, doInline bool) {
229229
}
230230
})
231231

232+
// Rewalk post-inlining functions to check for closures that are
233+
// still visible but were (over-agressively) marked as dead, and
234+
// undo that marking here. See #59404 for more context.
235+
ir.VisitFuncsBottomUp(decls, func(list []*ir.Func, recursive bool) {
236+
for _, n := range list {
237+
ir.Visit(n, func(node ir.Node) {
238+
if clo, ok := node.(*ir.ClosureExpr); ok && clo.Func.IsHiddenClosure() {
239+
clo.Func.SetIsDeadcodeClosure(false)
240+
}
241+
})
242+
}
243+
})
244+
232245
if p != nil {
233246
pgoInlineEpilogue(p, decls)
234247
}
@@ -883,6 +896,30 @@ func inlnode(n ir.Node, maxCost int32, inlCalls *[]*ir.InlinedCallExpr, edit fun
883896
}
884897
if fn := inlCallee(call.X, profile); fn != nil && typecheck.HaveInlineBody(fn) {
885898
n = mkinlcall(call, fn, maxCost, inlCalls, edit)
899+
if fn.IsHiddenClosure() {
900+
// Visit function to pick out any contained hidden
901+
// closures to mark them as dead, since they will no
902+
// longer be reachable (if we leave them live, they
903+
// will get skipped during escape analysis, which
904+
// could mean that go/defer statements don't get
905+
// desugared, causing later problems in walk). See
906+
// #59404 for more context. Note also that the code
907+
// below can sometimes be too aggressive (marking a closure
908+
// dead even though it was captured by a local var).
909+
// In this case we'll undo the dead marking in a cleanup
910+
// pass that happens at the end of InlineDecls.
911+
var vis func(node ir.Node)
912+
vis = func(node ir.Node) {
913+
if clo, ok := node.(*ir.ClosureExpr); ok && clo.Func.IsHiddenClosure() && !clo.Func.IsDeadcodeClosure() {
914+
if base.Flag.LowerM > 2 {
915+
fmt.Printf("%v: closure %v marked as dead\n", ir.Line(clo.Func), clo.Func)
916+
}
917+
clo.Func.SetIsDeadcodeClosure(true)
918+
ir.Visit(clo.Func, vis)
919+
}
920+
}
921+
ir.Visit(fn, vis)
922+
}
886923
}
887924
}
888925

test/fixedbugs/issue59404.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// build -gcflags=-l=4
2+
3+
// Copyright 2023 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package p
8+
9+
type Interface interface {
10+
MonitoredResource() (resType string, labels map[string]string)
11+
Done()
12+
}
13+
14+
func Autodetect() Interface {
15+
return func() Interface {
16+
Do(func() {
17+
var ad, gd Interface
18+
19+
go func() {
20+
defer gd.Done()
21+
ad = aad()
22+
}()
23+
go func() {
24+
defer ad.Done()
25+
gd = aad()
26+
defer func() { recover() }()
27+
}()
28+
29+
autoDetected = ad
30+
if gd != nil {
31+
autoDetected = gd
32+
}
33+
})
34+
return autoDetected
35+
}()
36+
}
37+
38+
var autoDetected Interface
39+
var G int
40+
41+
type If int
42+
43+
func (x If) MonitoredResource() (resType string, labels map[string]string) {
44+
return "", nil
45+
}
46+
47+
//go:noinline
48+
func (x If) Done() {
49+
G++
50+
}
51+
52+
//go:noinline
53+
func Do(fn func()) {
54+
fn()
55+
}
56+
57+
//go:noinline
58+
func aad() Interface {
59+
var x If
60+
return x
61+
}

test/fixedbugs/issue59404part2.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// run
2+
3+
// Copyright 2023 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
var G func(int) int
10+
11+
//go:noinline
12+
func callclo(q, r int) int {
13+
p := func(z int) int {
14+
G = func(int) int { return 1 }
15+
return z + 1
16+
}
17+
res := p(q) ^ p(r) // These calls to "p" will be inlined
18+
G = p
19+
return res
20+
}
21+
22+
func main() {
23+
callclo(1, 2)
24+
}

0 commit comments

Comments
 (0)