From d20298e1c7d1df794a11ce7768e027c6759df2a4 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Mon, 14 Sep 2020 10:38:45 +0700 Subject: [PATCH] cmd/compile: make funccompile non-reentrant Currently, there's awkward reentrancy issue with funccompile: funccompile -> compile -> dtypesym -> geneq/genhash/genwrapper -> funccompile Though it's not a problem at this moment, some attempts by @mdempsky to move order/walk/instrument into buildssa was failed, due to SSA cache corruption. This commit fixes that reentrancy issue, by making generated functions to be pumped through the same compile workqueue that normal functions are compiled. We do this by adding them to xtop, instead of calling funccompile directly in geneq/genhash/genwrapper. In dumpdata, we look for uncompiled functions in xtop instead of compilequeue, then finish compiling them. Updates #38463 Fixes #33485 Change-Id: Ic9f0ce45b56ae2ff3862f17fd979253ddc144bb5 Reviewed-on: https://go-review.googlesource.com/c/go/+/254617 Run-TryBot: Cuong Manh Le Reviewed-by: Matthew Dempsky TryBot-Result: Go Bot Trust: Keith Randall Trust: Cuong Manh Le --- src/cmd/compile/internal/gc/alg.go | 4 ++-- src/cmd/compile/internal/gc/obj.go | 26 +++++++++++++++++++++++++- src/cmd/compile/internal/gc/subr.go | 2 +- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/cmd/compile/internal/gc/alg.go b/src/cmd/compile/internal/gc/alg.go index c9d71ea00b..6302b88f59 100644 --- a/src/cmd/compile/internal/gc/alg.go +++ b/src/cmd/compile/internal/gc/alg.go @@ -392,7 +392,7 @@ func genhash(t *types.Type) *obj.LSym { } fn.Func.SetNilCheckDisabled(true) - funccompile(fn) + xtop = append(xtop, fn) // Build closure. It doesn't close over any variables, so // it contains just the function pointer. @@ -754,7 +754,7 @@ func geneq(t *types.Type) *obj.LSym { // neither of which can be nil, and our comparisons // are shallow. fn.Func.SetNilCheckDisabled(true) - funccompile(fn) + xtop = append(xtop, fn) // Generate a closure which points at the function we just generated. dsymptr(closure, 0, sym.Linksym(), 0) diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index af5037c5a8..b55331a948 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -113,12 +113,16 @@ func dumpCompilerObj(bout *bio.Writer) { func dumpdata() { externs := len(externdcl) + xtops := len(xtop) dumpglobls() addptabs() + exportlistLen := len(exportlist) addsignats(externdcl) dumpsignats() dumptabs() + ptabsLen := len(ptabs) + itabsLen := len(itabs) dumpimportstrings() dumpbasictypes() @@ -129,9 +133,19 @@ func dumpdata() { // number of types in a finite amount of code. // In the typical case, we loop 0 or 1 times. // It was not until issue 24761 that we found any code that required a loop at all. - for len(compilequeue) > 0 { + for { + for i := xtops; i < len(xtop); i++ { + n := xtop[i] + if n.Op == ODCLFUNC { + funccompile(n) + } + } + xtops = len(xtop) compileFunctions() dumpsignats() + if xtops == len(xtop) { + break + } } // Dump extra globals. @@ -149,6 +163,16 @@ func dumpdata() { } addGCLocals() + + if exportlistLen != len(exportlist) { + Fatalf("exportlist changed after compile functions loop") + } + if ptabsLen != len(ptabs) { + Fatalf("ptabs changed after compile functions loop") + } + if itabsLen != len(itabs) { + Fatalf("itabs changed after compile functions loop") + } } func dumpLinkerObj(bout *bio.Writer) { diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index d3ba53ff0c..5a5833d19f 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -1615,7 +1615,7 @@ func genwrapper(rcvr *types.Type, method *types.Field, newnam *types.Sym) { escapeFuncs([]*Node{fn}, false) Curfn = nil - funccompile(fn) + xtop = append(xtop, fn) } func paramNnames(ft *types.Type) []*Node { -- 2.48.1