From 0349f29a55fc194e3d51f748ec9ddceab87a5668 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Sun, 17 Feb 2019 23:12:55 -0500 Subject: [PATCH] cmd/compile: flow interface data to heap if CONVIFACE of a non-direct interface escapes Consider the following code: func f(x []*T) interface{} { return x } It returns an interface that holds a heap copy of x (by calling convT2I or friend), therefore x escape to heap. The current escape analysis only recognizes that x flows to the result. This is not sufficient, since if the result does not escape, x's content may be stack allocated and this will result a heap-to-stack pointer, which is bad. Fix this by realizing that if a CONVIFACE escapes and we're converting from a non-direct interface type, the data needs to escape to heap. Running "toolstash -cmp" on std & cmd, the generated machine code are identical for all packages. However, the export data (escape tags) differ in the following packages. It looks to me that all are similar to the "f" above, where the parameter should escape to heap. io/ioutil/ioutil.go:118 old: leaking param: r to result ~r1 level=0 new: leaking param: r image/image.go:943 old: leaking param: p to result ~r0 level=1 new: leaking param content: p net/url/url.go:200 old: leaking param: s to result ~r2 level=0 new: leaking param: s (as a consequence) net/url/url.go:183 old: leaking param: s to result ~r1 level=0 new: leaking param: s net/url/url.go:194 old: leaking param: s to result ~r1 level=0 new: leaking param: s net/url/url.go:699 old: leaking param: u to result ~r0 level=1 new: leaking param: u net/url/url.go:775 old: (*URL).String u does not escape new: leaking param content: u net/url/url.go:1038 old: leaking param: u to result ~r0 level=1 new: leaking param: u net/url/url.go:1099 old: (*URL).MarshalBinary u does not escape new: leaking param content: u flag/flag.go:235 old: leaking param: s to result ~r0 level=1 new: leaking param content: s go/scanner/errors.go:105 old: leaking param: p to result ~r0 level=0 new: leaking param: p database/sql/sql.go:204 old: leaking param: ns to result ~r0 level=0 new: leaking param: ns go/constant/value.go:303 old: leaking param: re to result ~r2 level=0, leaking param: im to result ~r2 level=0 new: leaking param: re, leaking param: im go/constant/value.go:846 old: leaking param: x to result ~r1 level=0 new: leaking param: x encoding/xml/xml.go:518 old: leaking param: d to result ~r1 level=2 new: leaking param content: d encoding/xml/xml.go:122 old: leaking param: leaking param: t to result ~r1 level=0 new: leaking param: t crypto/x509/verify.go:506 old: leaking param: c to result ~r8 level=0 new: leaking param: c crypto/x509/verify.go:563 old: leaking param: c to result ~r3 level=0, leaking param content: c new: leaking param: c crypto/x509/verify.go:615 old: (nothing) new: leaking closure reference c crypto/x509/verify.go:996 old: leaking param: c to result ~r1 level=0, leaking param content: c new: leaking param: c net/http/filetransport.go:30 old: leaking param: fs to result ~r1 level=0 new: leaking param: fs net/http/h2_bundle.go:2684 old: leaking param: mh to result ~r0 level=2 new: leaking param content: mh net/http/h2_bundle.go:7352 old: http2checkConnHeaders req does not escape new: leaking param content: req net/http/pprof/pprof.go:221 old: leaking param: name to result ~r1 level=0 new: leaking param: name cmd/internal/bio/must.go:21 old: leaking param: w to result ~r1 level=0 new: leaking param: w Fixes #29353. Change-Id: I7e7798ae773728028b0dcae5bccb3ada51189c68 Reviewed-on: https://go-review.googlesource.com/c/162829 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky Reviewed-by: David Chase --- src/cmd/compile/internal/gc/esc.go | 10 ++++++++++ test/escape_because.go | 2 +- test/escape_param.go | 15 +++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go index 322b2dcd0b..bd0fb82554 100644 --- a/src/cmd/compile/internal/gc/esc.go +++ b/src/cmd/compile/internal/gc/esc.go @@ -2105,6 +2105,16 @@ func (e *EscState) escwalkBody(level Level, dst *Node, src *Node, step *EscStep, step.describe(src) } extraloopdepth = modSrcLoopdepth + if src.Op == OCONVIFACE { + lt := src.Left.Type + if !lt.IsInterface() && !isdirectiface(lt) && types.Haspointers(lt) { + // We're converting from a non-direct interface type. + // The interface will hold a heap copy of the data + // (by calling convT2I or friend). Flow the data to heap. + // See issue 29353. + e.escwalk(level, &e.theSink, src.Left, e.stepWalk(dst, src.Left, "interface-converted", step)) + } + } } case ODOT, diff --git a/test/escape_because.go b/test/escape_because.go index 3b67ff9e4b..64fa28ddda 100644 --- a/test/escape_because.go +++ b/test/escape_because.go @@ -43,7 +43,7 @@ func f2(q *int) { // ERROR "from &u \(address-of\) at escape_because.go:43$" "fr sink = &u // ERROR "&u escapes to heap$" "from &u \(interface-converted\) at escape_because.go:43$" "from sink \(assigned to top level variable\) at escape_because.go:43$" } -func f3(r *int) interface{} { // ERROR "from \[\]\*int literal \(slice-literal-element\) at escape_because.go:47$" "from c \(assigned\) at escape_because.go:47$" "from c \(interface-converted\) at escape_because.go:48$" "from ~r1 \(return\) at escape_because.go:48$" "leaking param: r to result ~r1 level=-1$" +func f3(r *int) interface{} { // ERROR "from \[\]\*int literal \(slice-literal-element\) at escape_because.go:47$" "from c \(assigned\) at escape_because.go:47$" "from c \(interface-converted\) at escape_because.go:48$" "from ~r1 \(return\) at escape_because.go:48$" "leaking param: r" c := []*int{r} // ERROR "\[\]\*int literal escapes to heap$" "from c \(assigned\) at escape_because.go:47$" "from c \(interface-converted\) at escape_because.go:48$" "from ~r1 \(return\) at escape_because.go:48$" return c // "return" // ERROR "c escapes to heap$" "from ~r1 \(return\) at escape_because.go:48$" } diff --git a/test/escape_param.go b/test/escape_param.go index dff13b6f7c..175a4f03dd 100644 --- a/test/escape_param.go +++ b/test/escape_param.go @@ -424,3 +424,18 @@ func h(x *Node) { // ERROR "leaking param: x" Sink = g(y) f(y) } + +// interface(in) -> out +// See also issue 29353. + +// Convert to a non-direct interface, require an allocation and +// copy x to heap (not to result). +func param14a(x [4]*int) interface{} { // ERROR "leaking param: x$" + return x // ERROR "x escapes to heap" +} + +// Convert to a direct interface, does not need an allocation. +// So x only leaks to result. +func param14b(x *int) interface{} { // ERROR "leaking param: x to result ~r1 level=0" + return x // ERROR "x escapes to heap" +}