cmd/gc: fix escape analysis of closures
Fixes #10353
See test/escape2.go:issue10353. Previously new(int) did not escape to heap,
and so heap-allcated closure was referencing a stack var. This breaks
the invariant that heap must not contain pointers to stack.
Look at the following program:
package main
func main() {
foo(new(int))
bar(new(int))
}
func foo(x *int) func() {
return func() {
println(*x)
}
}
// Models what foo effectively does.
func bar(x *int) *C {
return &C{x}
}
type C struct {
x *int
}
Without this patch escape analysis works as follows:
$ go build -gcflags="-m -m -m -l" esc.go
escflood:1: dst ~r1 scope:foo[0]
escwalk: level:0 depth:0 func literal( l(9) f(1) esc(no) ld(1)) scope:foo[1]
/tmp/live2.go:9: func literal escapes to heap
escwalk: level:0 depth:1 x( l(8) class(PPARAM) f(1) esc(no) ld(1)) scope:foo[1]
/tmp/live2.go:8: leaking param: x to result ~r1
escflood:2: dst ~r1 scope:bar[0]
escwalk: level:0 depth:0 &C literal( l(15) esc(no) ld(1)) scope:bar[1]
/tmp/live2.go:15: &C literal escapes to heap
escwalk: level:-1 depth:1 &C literal( l(15)) scope:bar[0]
escwalk: level:-1 depth:2 x( l(14) class(PPARAM) f(1) esc(no) ld(1)) scope:bar[1]
/tmp/live2.go:14: leaking param: x
/tmp/live2.go:5: new(int) escapes to heap
/tmp/live2.go:4: main new(int) does not escape
new(int) does not escape while being captured by the closure.
With this patch escape analysis of foo and bar works similarly:
$ go build -gcflags="-m -m -m -l" esc.go
escflood:1: dst ~r1 scope:foo[0]
escwalk: level:0 depth:0 &(func literal)( l(9)) scope:foo[0]
escwalk: level:-1 depth:1 func literal( l(9) f(1) esc(no) ld(1)) scope:foo[1]
/tmp/live2.go:9: func literal escapes to heap
escwalk: level:-1 depth:2 x( l(8) class(PPARAM) f(1) esc(no) ld(1)) scope:foo[1]
/tmp/live2.go:8: leaking param: x
escflood:2: dst ~r1 scope:bar[0]
escwalk: level:0 depth:0 &C literal( l(15) esc(no) ld(1)) scope:bar[1]
/tmp/live2.go:15: &C literal escapes to heap
escwalk: level:-1 depth:1 &C literal( l(15)) scope:bar[0]
escwalk: level:-1 depth:2 x( l(14) class(PPARAM) f(1) esc(no) ld(1)) scope:bar[1]
/tmp/live2.go:14: leaking param: x
/tmp/live2.go:4: new(int) escapes to heap
/tmp/live2.go:5: new(int) escapes to heap
Change-Id: Ifd14b7ae3fc11820e3b5eb31eb07f35a22ed0932
Reviewed-on: https://go-review.googlesource.com/8408
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/src/cmd/internal/gc/esc.go b/src/cmd/internal/gc/esc.go
index bcd3a83..6f894c9 100644
--- a/src/cmd/internal/gc/esc.go
+++ b/src/cmd/internal/gc/esc.go
@@ -874,12 +874,19 @@
OSTRARRAYBYTE,
OADDSTR,
ONEW,
- OCLOSURE,
OCALLPART,
ORUNESTR,
OCONVIFACE:
escflows(e, dst, src)
+ case OCLOSURE:
+ // OCLOSURE is lowered to OPTRLIT,
+ // insert OADDR to account for the additional indirection.
+ a := Nod(OADDR, src, nil)
+ a.Lineno = src.Lineno
+ a.Escloopdepth = src.Escloopdepth
+ escflows(e, dst, a)
+
// Flowing multiple returns to a single dst happens when
// analyzing "go f(g())": here g() flows to sink (issue 4529).
case OCALLMETH, OCALLFUNC, OCALLINTER:
@@ -1306,7 +1313,11 @@
src.Esc = EscHeap
addrescapes(src.Left)
if Debug['m'] != 0 {
- Warnl(int(src.Lineno), "%v escapes to heap", Nconv(src, obj.FmtShort))
+ p := src
+ if p.Left.Op == OCLOSURE {
+ p = p.Left // merely to satisfy error messages in tests
+ }
+ Warnl(int(src.Lineno), "%v escapes to heap", Nconv(p, obj.FmtShort))
}
}
diff --git a/test/escape2.go b/test/escape2.go
index 65dbd7a..6b25e68 100644
--- a/test/escape2.go
+++ b/test/escape2.go
@@ -1797,3 +1797,25 @@
func nonescapingIface(m map[M]bool) bool { // ERROR "m does not escape"
return m[MV(0)] // ERROR "MV\(0\) does not escape"
}
+
+func issue10353() {
+ x := new(int) // ERROR "new\(int\) escapes to heap"
+ issue10353a(x)()
+}
+
+func issue10353a(x *int) func() { // ERROR "leaking param: x"
+ return func() { // ERROR "func literal escapes to heap"
+ println(*x)
+ }
+}
+
+func issue10353b() {
+ var f func()
+ for {
+ x := new(int) // ERROR "new\(int\) escapes to heap"
+ f = func() { // ERROR "func literal escapes to heap"
+ println(*x)
+ }
+ }
+ _ = f
+}
diff --git a/test/escape2n.go b/test/escape2n.go
index 59f64c0..fff1f95 100644
--- a/test/escape2n.go
+++ b/test/escape2n.go
@@ -1797,3 +1797,25 @@
func nonescapingIface(m map[M]bool) bool { // ERROR "m does not escape"
return m[MV(0)] // ERROR "MV\(0\) does not escape"
}
+
+func issue10353() {
+ x := new(int) // ERROR "new\(int\) escapes to heap"
+ issue10353a(x)()
+}
+
+func issue10353a(x *int) func() { // ERROR "leaking param: x"
+ return func() { // ERROR "func literal escapes to heap"
+ println(*x)
+ }
+}
+
+func issue10353b() {
+ var f func()
+ for {
+ x := new(int) // ERROR "new\(int\) escapes to heap"
+ f = func() { // ERROR "func literal escapes to heap"
+ println(*x)
+ }
+ }
+ _ = f
+}
diff --git a/test/fixedbugs/issue10353.go b/test/fixedbugs/issue10353.go
new file mode 100644
index 0000000..4886337
--- /dev/null
+++ b/test/fixedbugs/issue10353.go
@@ -0,0 +1,49 @@
+// run
+
+// Copyright 2015 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.
+
+// issue 10253: cmd/gc: incorrect escape analysis of closures
+// Partial call x.foo was not promoted to heap.
+
+package main
+
+func main() {
+ c := make(chan bool)
+ // Create a new goroutine to get a default-size stack segment.
+ go func() {
+ x := new(X)
+ clos(x.foo)()
+ c <- true
+ }()
+ <-c
+}
+
+type X int
+
+func (x *X) foo() {
+}
+
+func clos(x func()) func() {
+ f := func() {
+ print("")
+ x() // This statement crashed, because the partial call was allocated on the old stack.
+ }
+ // Grow stack so that partial call x becomes invalid if allocated on stack.
+ growstack(10000)
+ c := make(chan bool)
+ // Spoil the previous stack segment.
+ go func() {
+ c <- true
+ }()
+ <-c
+ return f
+}
+
+func growstack(x int) {
+ if x == 0 {
+ return
+ }
+ growstack(x-1)
+}
