From: Dmitry Vyukov Date: Mon, 6 Apr 2015 15:17:20 +0000 (+0300) Subject: cmd/gc: fix escape analysis of closures X-Git-Tag: go1.5beta1~1220 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=878a86a129ceb771f677c8391a9f5c891e5be7c3;p=gostls13.git 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 Run-TryBot: Dmitry Vyukov TryBot-Result: Gobot Gobot --- diff --git a/src/cmd/internal/gc/esc.go b/src/cmd/internal/gc/esc.go index bcd3a83dab..6f894c9165 100644 --- 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 65dbd7a2fe..6b25e68616 100644 --- 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 59f64c01eb..fff1f95958 100644 --- 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 index 0000000000..4886337db9 --- /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) +}