From b19ec6842d3d3bdc6d7b67fa065121a9d317cff7 Mon Sep 17 00:00:00 2001 From: David Chase Date: Thu, 21 May 2015 12:40:25 -0400 Subject: [PATCH] cmd/internal/gc: make indirect calls properly escape-y Indirect function and method calls should leak everything, but they didn't. This fix had no particular effect on the cost of running the compiler on html/template/*.go and added a single new "escape" to the standard library: syscall/syscall_unix.go:85: &b[0] escapes to heap in if errno := m.munmap(uintptr(unsafe.Pointer(&b[0])), uintptr(len(b))); errno != nil { Added specific escape testing to escape_calls.go (and verified that it fails without this patch) I also did a little code cleanup around the changes in esc.c. Fixes #10925 Change-Id: I9984b701621ad4c49caed35b01e359295c210033 Reviewed-on: https://go-review.googlesource.com/10295 Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/esc.go | 67 ++++++++++++++++++++---------- test/escape_calls.go | 10 +++++ test/fixedbugs/issue10925.go | 23 ++++++++++ 3 files changed, 79 insertions(+), 21 deletions(-) create mode 100644 test/fixedbugs/issue10925.go diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go index a9a1748b9a..4c1f52521d 100644 --- a/src/cmd/compile/internal/gc/esc.go +++ b/src/cmd/compile/internal/gc/esc.go @@ -1269,6 +1269,24 @@ func escNoteOutputParamFlow(e uint16, vargen int32, level Level) uint16 { return (e &^ (bitsMaskForTag << shift)) | encodedFlow } +func initEscretval(e *EscState, n *Node, fntype *Type) { + i := 0 + n.Escretval = nil // Suspect this is not nil for indirect calls. + for t := getoutargx(fntype).Type; t != nil; t = t.Down { + src := Nod(ONAME, nil, nil) + buf := fmt.Sprintf(".out%d", i) + i++ + src.Sym = Lookup(buf) + src.Type = t.Type + src.Class = PAUTO + src.Curfn = Curfn + src.Escloopdepth = e.loopdepth + src.Used = true + src.Lineno = n.Lineno + n.Escretval = list(n.Escretval, src) + } +} + // This is a bit messier than fortunate, pulled out of esc's big // switch for clarity. We either have the paramnodes, which may be // connected to other things through flows or we have the parameter type @@ -1277,7 +1295,7 @@ func escNoteOutputParamFlow(e uint16, vargen int32, level Level) uint16 { // this-package func esccall(e *EscState, n *Node, up *Node) { var fntype *Type - + var indirect bool var fn *Node switch n.Op { default: @@ -1286,6 +1304,7 @@ func esccall(e *EscState, n *Node, up *Node) { case OCALLFUNC: fn = n.Left fntype = fn.Type + indirect = fn.Op != ONAME || fn.Class != PFUNC case OCALLMETH: fn = n.Left.Right.Sym.Def @@ -1297,6 +1316,7 @@ func esccall(e *EscState, n *Node, up *Node) { case OCALLINTER: fntype = n.Left.Type + indirect = true } ll := n.List @@ -1307,6 +1327,28 @@ func esccall(e *EscState, n *Node, up *Node) { } } + if indirect { + // We know nothing! + // Leak all the parameters + for ; ll != nil; ll = ll.Next { + escassign(e, &e.theSink, ll.N) + if Debug['m'] > 2 { + fmt.Printf("%v::esccall:: indirect call <- %v, untracked\n", Ctxt.Line(int(lineno)), Nconv(ll.N, obj.FmtShort)) + } + } + // Set up bogus outputs + initEscretval(e, n, fntype) + // If there is a receiver, it also leaks to heap. + if n.Op != OCALLFUNC { + t := getthisx(fntype).Type + src := n.Left.Left + if haspointers(t.Type) { + escassign(e, &e.theSink, src) + } + } + return + } + if fn != nil && fn.Op == ONAME && fn.Class == PFUNC && fn.Defn != nil && fn.Defn.Nbody != nil && fn.Ntype != nil && fn.Defn.Esc < EscFuncTagged { if Debug['m'] > 2 { @@ -1376,23 +1418,7 @@ func esccall(e *EscState, n *Node, up *Node) { } // set up out list on this call node with dummy auto ONAMES in the current (calling) function. - i := 0 - - var src *Node - var buf string - for t := getoutargx(fntype).Type; t != nil; t = t.Down { - src = Nod(ONAME, nil, nil) - buf = fmt.Sprintf(".out%d", i) - i++ - src.Sym = Lookup(buf) - src.Type = t.Type - src.Class = PAUTO - src.Curfn = Curfn - src.Escloopdepth = e.loopdepth - src.Used = true - src.Lineno = n.Lineno - n.Escretval = list(n.Escretval, src) - } + initEscretval(e, n, fntype) // print("esc analyzed fn: %#N (%+T) returning (%+H)\n", fn, fntype, n->escretval); @@ -1405,9 +1431,8 @@ func esccall(e *EscState, n *Node, up *Node) { } } - var a *Node for t := getinargx(fntype).Type; ll != nil; ll = ll.Next { - src = ll.N + src := ll.N if t.Isddd && !n.Isddd { // Introduce ODDDARG node to represent ... allocation. src = Nod(ODDDARG, nil, nil) @@ -1425,7 +1450,7 @@ func esccall(e *EscState, n *Node, up *Node) { if haspointers(t.Type) { if escassignfromtag(e, t.Note, n.Escretval, src) == EscNone && up.Op != ODEFER && up.Op != OPROC { - a = src + a := src for a.Op == OCONVNOP { a = a.Left } diff --git a/test/escape_calls.go b/test/escape_calls.go index f289670091..8c9a6dadda 100644 --- a/test/escape_calls.go +++ b/test/escape_calls.go @@ -42,3 +42,13 @@ func walk(np **Node) int { // ERROR "leaking param content: np" *np = n return w + wl + wr } + +// Test for bug where func var f used prototype's escape analysis results. +func prototype(xyz []string) {} // ERROR "prototype xyz does not escape" +func bar() { + var got [][]string + f := prototype + f = func(ss []string) { got = append(got, ss) } // ERROR "leaking param: ss" "func literal does not escape" + s := "string" + f([]string{s}) // ERROR "\[\]string literal escapes to heap" +} diff --git a/test/fixedbugs/issue10925.go b/test/fixedbugs/issue10925.go new file mode 100644 index 0000000000..30add82c78 --- /dev/null +++ b/test/fixedbugs/issue10925.go @@ -0,0 +1,23 @@ +// 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. + +package main + +import "fmt" + +func prototype(xyz []string) {} +func main() { + var got [][]string + f := prototype + f = func(ss []string) { got = append(got, ss) } + for _, s := range []string{"one", "two", "three"} { + f([]string{s}) + } + if got[0][0] != "one" || got[1][0] != "two" || got[2][0] != "three" { + // Bug's wrong output was [[three] [three] [three]] + fmt.Println("Expected [[one] [two] [three]], got", got) + } +}