diff options
author | Dan Scales <danscales@google.com> | 2021-02-25 12:13:23 -0800 |
---|---|---|
committer | Dan Scales <danscales@google.com> | 2021-02-26 02:11:50 +0000 |
commit | 9a555fc24c318bf1b07bdc07d5c02e372681e401 (patch) | |
tree | dd079c4e836413524928a567b1b7503dccd0e68b | |
parent | a61524d1033632f733f69bf6635e767d70d95cdd (diff) | |
download | go-9a555fc24c318bf1b07bdc07d5c02e372681e401.tar.gz go-9a555fc24c318bf1b07bdc07d5c02e372681e401.zip |
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 <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
-rw-r--r-- | src/cmd/compile/internal/base/flag.go | 1 | ||||
-rw-r--r-- | src/cmd/compile/internal/inline/inl.go | 4 | ||||
-rw-r--r-- | src/cmd/compile/internal/typecheck/subr.go | 9 | ||||
-rw-r--r-- | test/fixedbugs/issue44370.dir/a.go | 20 | ||||
-rw-r--r-- | test/fixedbugs/issue44370.dir/b.go | 11 | ||||
-rw-r--r-- | test/fixedbugs/issue44370.go | 7 |
6 files changed, 47 insertions, 5 deletions
diff --git a/src/cmd/compile/internal/base/flag.go b/src/cmd/compile/internal/base/flag.go index d8ca9885cb..ade17fc0cd 100644 --- a/src/cmd/compile/internal/base/flag.go +++ b/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) diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 1703be74e9..e961b10844 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/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 } diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index b88a9f2283..c40cfa2288 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/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) } } diff --git a/test/fixedbugs/issue44370.dir/a.go b/test/fixedbugs/issue44370.dir/a.go new file mode 100644 index 0000000000..c5bf1bcbc7 --- /dev/null +++ b/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 }(), + } +} diff --git a/test/fixedbugs/issue44370.dir/b.go b/test/fixedbugs/issue44370.dir/b.go new file mode 100644 index 0000000000..f0e0b4e55f --- /dev/null +++ b/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() +} diff --git a/test/fixedbugs/issue44370.go b/test/fixedbugs/issue44370.go new file mode 100644 index 0000000000..d406838588 --- /dev/null +++ b/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 |