cmd/compile: fix missing descend in Addrtaken for closures.

ComputeAddrtaken needs to descend into closures, now that imported
bodies can include closures. The bug was that we weren't properly
setting Addrtaken for a variable inside a closure inside a function that
we were importing.

For now, still disable inlining of functions with closures for -G mode.
I'll enable it in a later change -- there are just a few fixes related
to the fact that we don't need to set Ntype for closure functions.

Added a test derived from the cilium repro in the issue.

Fixes #44370

Change-Id: Ida2a403636bf8740b471b3ad68b5474951811e19
Reviewed-on: https://go-review.googlesource.com/c/go/+/296649
Run-TryBot: Dan Scales <[email protected]>
Trust: Dan Scales <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

Author: danscales

src/cmd/compile/internal/base/flag.go

@@ -159,6 +159,7 @@ func ParseFlags() {
 	Flag.LinkShared = &Ctxt.Flag_linkshared
 	Flag.Shared = &Ctxt.Flag_shared
 	Flag.WB = true
+	Debug.InlFuncsWithClosures = 1
 
 	Flag.Cfg.ImportMap = make(map[string]string)
 

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

@@ -354,9 +354,7 @@ func (v *hairyVisitor) doNode(n ir.Node) bool {
 		return true
 
 	case ir.OCLOSURE:
-		if base.Debug.InlFuncsWithClosures == 0 {
-			// TODO(danscales): change default of InlFuncsWithClosures
-			// to 1 when #44370 is fixed
+		if base.Debug.InlFuncsWithClosures == 0 || base.Flag.G > 0 {
 			v.reason = "not inlining functions with closures"
 			return true
 		}

src/cmd/compile/internal/typecheck/subr.go

@@ -106,7 +106,8 @@ var DirtyAddrtaken = false
 
 func ComputeAddrtaken(top []ir.Node) {
 	for _, n := range top {
-		ir.Visit(n, func(n ir.Node) {
+		var doVisit func(n ir.Node)
+		doVisit = func(n ir.Node) {
 			if n.Op() == ir.OADDR {
 				if x := ir.OuterValue(n.(*ir.AddrExpr).X); x.Op() == ir.ONAME {
 					x.Name().SetAddrtaken(true)
@@ -117,7 +118,11 @@ func ComputeAddrtaken(top []ir.Node) {
 					}
 				}
 			}
-		})
+			if n.Op() == ir.OCLOSURE {
+				ir.VisitList(n.(*ir.ClosureExpr).Func.Body, doVisit)
+			}
+		}
+		ir.Visit(n, doVisit)
 	}
 }
 

test/fixedbugs/issue44370.dir/a.go

@@ -0,0 +1,20 @@
+// Copyright 2021 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 a
+
+// A StoppableWaitGroup waits for a collection of goroutines to finish.
+type StoppableWaitGroup struct {
+	// i is the internal counter which can store tolerate negative values
+	// as opposed the golang's library WaitGroup.
+	i *int64
+}
+
+// NewStoppableWaitGroup returns a new StoppableWaitGroup. When the 'Stop' is
+// executed, following 'Add()' calls won't have any effect.
+func NewStoppableWaitGroup() *StoppableWaitGroup {
+	return &StoppableWaitGroup{
+		i: func() *int64 { i := int64(0); return &i }(),
+	}
+}

test/fixedbugs/issue44370.dir/b.go

@@ -0,0 +1,11 @@
+// Copyright 2021 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 b
+
+import "./a"
+
+func JoinClusterServices() {
+	_ = a.NewStoppableWaitGroup()
+}

test/fixedbugs/issue44370.go

@@ -0,0 +1,7 @@
+// compiledir
+
+// Copyright 2021 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 ignored