From 33738ddd0a99991459d3bf215004e4327c2f8af2 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Fri, 9 Sep 2022 16:23:39 -0400 Subject: cmd/compile: eagerly create LSym for closures The linker needs FuncInfo metadata for all inlined functions. This is typically handled by gc.enqueueFunc calling ir.InitLSym for all function declarations in typecheck.Target.Decls (ir.UseClosure adds all closures to Decls). However, non-trivial closures in Decls are ignored, and are insteaded enqueued when walk of the calling function discovers them. This presents a problem for direct calls to closures. Inlining will replace the entire closure definition with its body, which hides the closure from walk and thus suppresses symbol creation. Explicitly create a symbol early in this edge case to ensure we keep this metadata. InitLSym needs to move out of ssagen to avoid a circular dependency (it doesn't have anything to do with ssa anyway). There isn't a great place for it, so I placed it in ir, which seemed least objectionable. The added test triggers one of these inlined direct non-trivial closure calls, though the test needs CL 429637 to fail, which adds a FuncInfo assertion to the linker. Note that the test must use "run" instead of "compile" since the assertion is in the linker, and "compiler" doesn't run the linker. Fixes #54959. Change-Id: I0bd1db4f3539a78da260934cd968372b7aa92546 Reviewed-on: https://go-review.googlesource.com/c/go/+/436240 Run-TryBot: Michael Pratt Reviewed-by: Matthew Dempsky TryBot-Result: Gopher Robot --- src/cmd/compile/internal/inline/inl.go | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) (limited to 'src/cmd/compile/internal/inline') diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 14adbf5d43..fe042dd024 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -837,6 +837,48 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.Inlin inlIndex := base.Ctxt.InlTree.Add(parent, n.Pos(), sym) + closureInitLSym := func(n *ir.CallExpr, fn *ir.Func) { + // The linker needs FuncInfo metadata for all inlined + // functions. This is typically handled by gc.enqueueFunc + // calling ir.InitLSym for all function declarations in + // typecheck.Target.Decls (ir.UseClosure adds all closures to + // Decls). + // + // However, non-trivial closures in Decls are ignored, and are + // insteaded enqueued when walk of the calling function + // discovers them. + // + // This presents a problem for direct calls to closures. + // Inlining will replace the entire closure definition with its + // body, which hides the closure from walk and thus suppresses + // symbol creation. + // + // Explicitly create a symbol early in this edge case to ensure + // we keep this metadata. + // + // TODO: Refactor to keep a reference so this can all be done + // by enqueueFunc. + + if n.Op() != ir.OCALLFUNC { + // Not a standard call. + return + } + if n.X.Op() != ir.OCLOSURE { + // Not a direct closure call. + return + } + + clo := n.X.(*ir.ClosureExpr) + if ir.IsTrivialClosure(clo) { + // enqueueFunc will handle trivial closures anyways. + return + } + + ir.InitLSym(fn, true) + } + + closureInitLSym(n, fn) + if base.Flag.GenDwarfInl > 0 { if !sym.WasInlined() { base.Ctxt.DwFixups.SetPrecursorFunc(sym, fn) -- cgit v1.3