From f1caf1aa1c45e014e35316da8e0d2edf48795bf6 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 4 Apr 2023 18:56:21 -0400 Subject: [PATCH] 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 Reviewed-by: Matthew Dempsky Run-TryBot: Than McIntosh Reviewed-by: Cuong Manh Le Reviewed-by: Cherry Mui --- src/cmd/compile/internal/inline/inl.go | 37 ++++++++++++++++ test/fixedbugs/issue59404.go | 61 ++++++++++++++++++++++++++ test/fixedbugs/issue59404part2.go | 24 ++++++++++ 3 files changed, 122 insertions(+) create mode 100644 test/fixedbugs/issue59404.go create mode 100644 test/fixedbugs/issue59404part2.go diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 80be841efa..3b42818c57 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -229,6 +229,19 @@ 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) + } + }) + } + }) + if p != nil { pgoInlineEpilogue(p, decls) } @@ -883,6 +896,30 @@ func inlnode(n ir.Node, maxCost int32, inlCalls *[]*ir.InlinedCallExpr, edit fun } if fn := inlCallee(call.X, profile); fn != nil && typecheck.HaveInlineBody(fn) { n = mkinlcall(call, fn, maxCost, 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/issue59404.go b/test/fixedbugs/issue59404.go new file mode 100644 index 0000000000..0f391e6af1 --- /dev/null +++ b/test/fixedbugs/issue59404.go @@ -0,0 +1,61 @@ +// 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() Interface { + return func() Interface { + 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 + }() +} + +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 +} diff --git a/test/fixedbugs/issue59404part2.go b/test/fixedbugs/issue59404part2.go new file mode 100644 index 0000000000..37fa029a1d --- /dev/null +++ b/test/fixedbugs/issue59404part2.go @@ -0,0 +1,24 @@ +// run + +// 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 main + +var G func(int) int + +//go:noinline +func callclo(q, r int) int { + p := func(z int) int { + G = func(int) int { return 1 } + return z + 1 + } + res := p(q) ^ p(r) // These calls to "p" will be inlined + G = p + return res +} + +func main() { + callclo(1, 2) +} -- 2.48.1