cmd/compile: fix reentrancy issue in unified IR function body reading

We shouldn't need to read in function bodies for new functions found
during inlining, but something is expecting them to still be read
in. We should fix that code to not depend on them being read in, but
in the mean time reading them in anyway is at least correct, albeit
less efficient in time and space.

Fixes #49536.
Updates #50552.

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

2 files changed, 30 insertions, 24 deletions

diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go
index 2b1636588e..3207e3f85b 100644
--- a/src/cmd/compile/internal/noder/reader.go
+++ b/src/cmd/compile/internal/noder/reader.go
@@ -930,11 +930,6 @@ var bodyReader = map[*ir.Func]pkgReaderIndex{}
 // constructed.
 var todoBodies []*ir.Func
 
-// todoBodiesDone signals that we constructed all function in todoBodies.
-// This is necessary to prevent reader.addBody adds thing to todoBodies
-// when nested inlining happens.
-var todoBodiesDone = false
-
 func (r *reader) addBody(fn *ir.Func) {
 	pri := pkgReaderIndex{r.p, r.Reloc(pkgbits.RelocBody), r.dict}
 	bodyReader[fn] = pri
@@ -945,7 +940,7 @@ func (r *reader) addBody(fn *ir.Func) {
 		return
 	}
 
-	if r.curfn == nil && !todoBodiesDone {
+	if r.curfn == nil {
 		todoBodies = append(todoBodies, fn)
 		return
 	}
@@ -1974,6 +1969,13 @@ func InlineCall(call *ir.CallExpr, fn *ir.Func, inlIndex int) *ir.InlinedCallExp
 		r.curfn.Body = r.stmts()
 		r.curfn.Endlineno = r.pos()
 
+		// TODO(mdempsky): This shouldn't be necessary. Inlining might
+		// read in new function/method declarations, which could
+		// potentially be recursively inlined themselves; but we shouldn't
+		// need to read in the non-inlined bodies for the declarations
+		// themselves. But currently it's an easy fix to #50552.
+		readBodies(typecheck.Target)
+
 		deadcode.Func(r.curfn)
 
 		// Replace any "return" statements within the function body.
diff --git a/src/cmd/compile/internal/noder/unified.go b/src/cmd/compile/internal/noder/unified.go
index ac82f2df03..ca01c0da95 100644
--- a/src/cmd/compile/internal/noder/unified.go
+++ b/src/cmd/compile/internal/noder/unified.go
@@ -116,6 +116,28 @@ func unified(noders []*noder) {
 		}
 	}
 
+	readBodies(target)
+
+	// Check that nothing snuck past typechecking.
+	for _, n := range target.Decls {
+		if n.Typecheck() == 0 {
+			base.FatalfAt(n.Pos(), "missed typecheck: %v", n)
+		}
+
+		// For functions, check that at least their first statement (if
+		// any) was typechecked too.
+		if fn, ok := n.(*ir.Func); ok && len(fn.Body) != 0 {
+			if stmt := fn.Body[0]; stmt.Typecheck() == 0 {
+				base.FatalfAt(stmt.Pos(), "missed typecheck: %v", stmt)
+			}
+		}
+	}
+
+	base.ExitIfErrors() // just in case
+}
+
+// readBodies reads in bodies for any
+func readBodies(target *ir.Package) {
 	// Don't use range--bodyIdx can add closures to todoBodies.
 	for len(todoBodies) > 0 {
 		// The order we expand bodies doesn't matter, so pop from the end
@@ -134,24 +156,6 @@ func unified(noders []*noder) {
 		}
 	}
 	todoBodies = nil
-	todoBodiesDone = true
-
-	// Check that nothing snuck past typechecking.
-	for _, n := range target.Decls {
-		if n.Typecheck() == 0 {
-			base.FatalfAt(n.Pos(), "missed typecheck: %v", n)
-		}
-
-		// For functions, check that at least their first statement (if
-		// any) was typechecked too.
-		if fn, ok := n.(*ir.Func); ok && len(fn.Body) != 0 {
-			if stmt := fn.Body[0]; stmt.Typecheck() == 0 {
-				base.FatalfAt(stmt.Pos(), "missed typecheck: %v", stmt)
-			}
-		}
-	}
-
-	base.ExitIfErrors() // just in case
 }
 
 // writePkgStub type checks the given parsed source files,