[release-branch.go1.12] 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 <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 0349f29a55fc194e3d51f748ec9ddceab87a5668)
Reviewed-on: https://go-review.googlesource.com/c/163203
Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go
index 322b2dc..bd0fb82 100644
--- a/src/cmd/compile/internal/gc/esc.go
+++ b/src/cmd/compile/internal/gc/esc.go
@@ -2105,6 +2105,16 @@
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 3b67ff9..64fa28d 100644
--- a/test/escape_because.go
+++ b/test/escape_because.go
@@ -43,7 +43,7 @@
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 dff13b6..175a4f0 100644
--- a/test/escape_param.go
+++ b/test/escape_param.go
@@ -424,3 +424,18 @@
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"
+}
