]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: un-hide closure func if parent expr moved to staticinit
authorThan McIntosh <thanm@google.com>
Wed, 3 May 2023 16:38:50 +0000 (12:38 -0400)
committerThan McIntosh <thanm@google.com>
Fri, 5 May 2023 21:04:38 +0000 (21:04 +0000)
If the function referenced by a closure expression is incorporated
into a static init, be sure to mark it as non-hidden, since otherwise
it will be live but no longer reachable from the init func, hence it
will be skipped during escape analysis, which can lead to
miscompilations.

Fixes #59680.

Change-Id: Ib858aee296efcc0b7655d25c23ab8a6a8dbdc5f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/492135
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
src/cmd/compile/internal/staticinit/sched.go
test/fixedbugs/issue59680.go [new file with mode: 0644]

index e5f7be4c5f4cac6f2f64402d639f0b28e3e646ef..7d1dfcbbb3bbed9eae94c5ee3fd9dcea8693fef9 100644 (file)
@@ -330,6 +330,12 @@ func (s *Schedule) StaticAssign(l *ir.Name, loff int64, r ir.Node, typ *types.Ty
                        if base.Debug.Closure > 0 {
                                base.WarnfAt(r.Pos(), "closure converted to global")
                        }
+                       // Issue 59680: if the closure we're looking at was produced
+                       // by inlining, it could be marked as hidden, which we don't
+                       // want (moving the func to a static init will effectively
+                       // hide it from escape analysis). Mark as non-hidden here.
+                       // so that it will participated in escape analysis.
+                       r.Func.SetIsHiddenClosure(false)
                        // Closures with no captured variables are globals,
                        // so the assignment can be done at link time.
                        // TODO if roff != 0 { panic }
diff --git a/test/fixedbugs/issue59680.go b/test/fixedbugs/issue59680.go
new file mode 100644 (file)
index 0000000..d21f61f
--- /dev/null
@@ -0,0 +1,100 @@
+// run
+
+// Copyright 2023 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 main
+
+import (
+       "sync"
+       "time"
+)
+
+type B struct {
+       pid int
+       f   func() (uint64, error)
+       wg  sync.WaitGroup
+       v   uint64
+}
+
+func newB(pid int) *B {
+       return &B{
+               pid: pid,
+       }
+}
+
+//go:noinline
+func Sq(i int) uint64 {
+       S++
+       return uint64(i * i)
+}
+
+type RO func(*B)
+
+var ROSL = []RO{
+       Bad(),
+}
+
+func Bad() RO {
+       return func(b *B) {
+               b.f = func() (uint64, error) {
+                       return Sq(b.pid), nil
+               }
+       }
+}
+
+func (b *B) startit() chan<- struct{} {
+       stop := make(chan struct{})
+       b.wg.Add(1)
+       go func() {
+               defer b.wg.Done()
+               var v uint64
+               for {
+                       select {
+                       case <-stop:
+                               b.v = v
+                               return
+                       case <-time.After(1 * time.Millisecond):
+                               r, err := b.f()
+                               if err != nil {
+                                       panic("bad")
+                               }
+                               v = r
+                       }
+               }
+       }()
+       return stop
+}
+
+var S, G int
+
+//go:noinline
+func rec(x int) int {
+       if x == 0 {
+               return 9
+       }
+       return rec(x-1) + 1
+}
+
+//go:noinline
+func recur(x int) {
+       for i := 0; i < x; i++ {
+               G = rec(i)
+       }
+}
+
+func main() {
+       b := newB(17)
+       for _, opt := range ROSL {
+               opt(b)
+       }
+       stop := b.startit()
+
+       // see if we can get some stack growth/moving
+       recur(10101)
+
+       if stop != nil {
+               stop <- struct{}{}
+       }
+}