cmd/compile: rework marking of dead hidden closure functions

This patch generalizes the code in the inliner that marks unreferenced
hidden closure functions as dead. Rather than doing the marking on the
fly (previous approach), this new approach does a single pass at the
end of inlining, which catches more dead functions.

Fixes #59638.
Updates #59404.
Updates #59547.

Change-Id: I54fd63e9e37c9123b08a3e7def7d1989919bba91
Reviewed-on: https://go-review.googlesource.com/c/go/+/484859
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

2 files changed, 124 insertions, 36 deletions

diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go
index 1a65e16f51..24812c8c0d 100644
--- a/src/cmd/compile/internal/inline/inl.go
+++ b/src/cmd/compile/internal/inline/inl.go
@@ -229,24 +229,71 @@ func InlineDecls(p *pgo.Profile, decls []ir.Node, doInline bool) {
 		}
 	})
 
-	// Rewalk post-inlining functions to check for closures that are
-	// still visible but were (over-agressively) marked as dead, and
-	// undo that marking here. See #59404 for more context.
-	ir.VisitFuncsBottomUp(decls, func(list []*ir.Func, recursive bool) {
-		for _, n := range list {
-			ir.Visit(n, func(node ir.Node) {
-				if clo, ok := node.(*ir.ClosureExpr); ok && clo.Func.IsHiddenClosure() {
-					clo.Func.SetIsDeadcodeClosure(false)
-				}
-			})
-		}
-	})
+	// Perform a garbage collection of hidden closures functions that
+	// are no longer reachable from top-level functions following
+	// inlining. See #59404 and #59638 for more context.
+	garbageCollectUnreferencedHiddenClosures()
 
 	if p != nil {
 		pgoInlineEpilogue(p, decls)
 	}
 }
 
+// garbageCollectUnreferencedHiddenClosures makes a pass over all the
+// top-level (non-hidden-closure) functions looking for nested closure
+// functions that are reachable, then sweeps through the Target.Decls
+// list and marks any non-reachable hidden closure function as dead.
+// See issues #59404 and #59638 for more context.
+func garbageCollectUnreferencedHiddenClosures() {
+
+	liveFuncs := make(map[*ir.Func]bool)
+
+	var markLiveFuncs func(fn *ir.Func)
+	markLiveFuncs = func(fn *ir.Func) {
+		liveFuncs[fn] = true
+		var vis func(node ir.Node)
+		vis = func(node ir.Node) {
+			if clo, ok := node.(*ir.ClosureExpr); ok {
+				if !liveFuncs[clo.Func] {
+					liveFuncs[clo.Func] = true
+					markLiveFuncs(clo.Func)
+				}
+			}
+		}
+		ir.Visit(fn, vis)
+	}
+
+	for i := 0; i < len(typecheck.Target.Decls); i++ {
+		if fn, ok := typecheck.Target.Decls[i].(*ir.Func); ok {
+			if fn.IsHiddenClosure() {
+				continue
+			}
+			markLiveFuncs(fn)
+		}
+	}
+
+	for i := 0; i < len(typecheck.Target.Decls); i++ {
+		if fn, ok := typecheck.Target.Decls[i].(*ir.Func); ok {
+			if !fn.IsHiddenClosure() {
+				continue
+			}
+			if fn.IsDeadcodeClosure() {
+				continue
+			}
+			if liveFuncs[fn] {
+				continue
+			}
+			fn.SetIsDeadcodeClosure(true)
+			if base.Flag.LowerM > 2 {
+				fmt.Printf("%v: unreferenced closure %v marked as dead\n", ir.Line(fn), fn)
+			}
+			if fn.Inl != nil && fn.LSym == nil {
+				ir.InitLSym(fn, true)
+			}
+		}
+	}
+}
+
 // CanInline determines whether fn is inlineable.
 // If so, CanInline saves copies of fn.Body and fn.Dcl in fn.Inl.
 // fn and fn.Body will already have been typechecked.
@@ -893,30 +940,6 @@ func inlnode(n ir.Node, bigCaller bool, inlCalls *[]*ir.InlinedCallExpr, edit fu
 		}
 		if fn := inlCallee(call.X, profile); fn != nil && typecheck.HaveInlineBody(fn) {
 			n = mkinlcall(call, fn, bigCaller, inlCalls, edit)
-			if fn.IsHiddenClosure() {
-				// Visit function to pick out any contained hidden
-				// closures to mark them as dead, since they will no
-				// longer be reachable (if we leave them live, they
-				// will get skipped during escape analysis, which
-				// could mean that go/defer statements don't get
-				// desugared, causing later problems in walk). See
-				// #59404 for more context. Note also that the code
-				// below can sometimes be too aggressive (marking a closure
-				// dead even though it was captured by a local var).
-				// In this case we'll undo the dead marking in a cleanup
-				// pass that happens at the end of InlineDecls.
-				var vis func(node ir.Node)
-				vis = func(node ir.Node) {
-					if clo, ok := node.(*ir.ClosureExpr); ok && clo.Func.IsHiddenClosure() && !clo.Func.IsDeadcodeClosure() {
-						if base.Flag.LowerM > 2 {
-							fmt.Printf("%v: closure %v marked as dead\n", ir.Line(clo.Func), clo.Func)
-						}
-						clo.Func.SetIsDeadcodeClosure(true)
-						ir.Visit(clo.Func, vis)
-					}
-				}
-				ir.Visit(fn, vis)
-			}
 		}
 	}
 
diff --git a/test/fixedbugs/issue59638.go b/test/fixedbugs/issue59638.go
new file mode 100644
index 0000000000..bba6265322
--- /dev/null
+++ b/test/fixedbugs/issue59638.go
@@ -0,0 +1,65 @@
+// build -gcflags=-l=4
+
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type Interface interface {
+	MonitoredResource() (resType string, labels map[string]string)
+	Done()
+}
+
+func Autodetect(x int) Interface {
+	return func() Interface {
+		func() Interface {
+			x++
+			Do(func() {
+				var ad, gd Interface
+
+				go func() {
+					defer gd.Done()
+					ad = aad()
+				}()
+				go func() {
+					defer ad.Done()
+					gd = aad()
+					defer func() { recover() }()
+				}()
+
+				autoDetected = ad
+				if gd != nil {
+					autoDetected = gd
+				}
+			})
+			return autoDetected
+		}()
+		return nil
+	}()
+}
+
+var autoDetected Interface
+var G int
+
+type If int
+
+func (x If) MonitoredResource() (resType string, labels map[string]string) {
+	return "", nil
+}
+
+//go:noinline
+func (x If) Done() {
+	G++
+}
+
+//go:noinline
+func Do(fn func()) {
+	fn()
+}
+
+//go:noinline
+func aad() Interface {
+	var x If
+	return x
+}