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 bcd3a83dabb83a1982c93fffd0fb4c4fd2403247..6f894c9165ba8d60dd14d8389289e501f3bea3bb 100644 (file)
--- a/src/cmd/internal/gc/esc.go
+++ b/src/cmd/internal/gc/esc.go
@@ -874,12 +874,19 @@ func escassign(e *EscState, dst *Node, src *Node) {
                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 @@ func escwalk(e *EscState, level int, dst *Node, src *Node) {
                        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 65dbd7a2fef46da96cb51090a0dc66733b95deb4..6b25e68616d6f8c93c256f5d726ddab4e0e77ebc 100644 (file)
--- a/test/escape2.go
+++ b/test/escape2.go
@@ -1797,3 +1797,25 @@ func nonescapingEface(m map[interface{}]bool) bool { // ERROR "m does not escape
 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 59f64c01ebb4a903c40efa9e5e8ea389e017dbd4..fff1f95958275ee9578a0760adc094c322f2c648 100644 (file)
--- a/test/escape2n.go
+++ b/test/escape2n.go
@@ -1797,3 +1797,25 @@ func nonescapingEface(m map[interface{}]bool) bool { // ERROR "m does not escape
 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 (file)
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)
+}