Revert "cmd/compile: rework marking of dead hidden closure functions"

This reverts commit http://go.dev/cl//484859

Reason for revert: causes linker errors in a number of google-internal tests.

Change-Id: I322252f784a46d2b1d447ebcdca86ce14bc0cc91
Reviewed-on: https://go-review.googlesource.com/c/go/+/485755
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>

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

@@ -229,71 +229,24 @@ func InlineDecls(p *pgo.Profile, decls []ir.Node, doInline bool) {
 		}
 	})
 
-	// Perform a garbage collection of hidden closures functions that
+	// Rewalk post-inlining functions to check for closures that are
-	// are no longer reachable from top-level functions following
+	// still visible but were (over-agressively) marked as dead, and
-	// inlining. See #59404 and #59638 for more context.
+	// undo that marking here. See #59404 for more context.
-	garbageCollectUnreferencedHiddenClosures()
+	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)
+				}
+			})
+		}
+	})
 
 	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.
@@ -940,6 +893,30 @@ 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)
+			}
 		}
 	}
 

test/fixedbugs/issue59638.go

@@ -1,65 +0,0 @@
-// 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
-}