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/8408Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

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))
 			}
 		}
 

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
+}

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
+}

test/fixedbugs/issue10353.go

+// 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)
+}