Skip to content

Commit

Permalink
cmd/compile: fix unnamed parameter handling in escape analysis
Browse files Browse the repository at this point in the history
For recursive functions, the parameters were iterated using
fn.Name.Defn.Func.Dcl, which does not include unnamed/blank
parameters. This results in a mismatch in formal-actual
assignments, for example,

func f(_ T, x T)

f(a, b) should result in { _=a, x=b }, but the escape analysis
currently sees only { x=a } and drops b on the floor. This may
cause b to not escape when it should (or a escape when it should
not).

Fix this by using fntype.Params().FieldSlice() instead, which
does include unnamed parameters.

Also add a sanity check that ensures all the actual parameters
are consumed.

Fixes #29000

Change-Id: Icd86f2b5d71e7ebbab76e375b7702f62efcf59ae
Reviewed-on: https://go-review.googlesource.com/c/152617
Reviewed-by: Keith Randall <[email protected]>
  • Loading branch information
cherrymui committed Dec 4, 2018
1 parent 8a5797a commit be09bdf
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 34 deletions.
98 changes: 64 additions & 34 deletions src/cmd/compile/internal/gc/esc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1652,49 +1652,79 @@ func (e *EscState) esccall(call *Node, parent *Node) {
Fatalf("graph inconsistency")
}

sawRcvr := false
for _, n := range fn.Name.Defn.Func.Dcl {
switch n.Class() {
case PPARAM:
if call.Op != OCALLFUNC && !sawRcvr {
e.escassignWhyWhere(n, call.Left.Left, "call receiver", call)
sawRcvr = true
continue
}
if len(args) == 0 {
continue
}
arg := args[0]
if n.IsDDD() && !call.IsDDD() {
// Introduce ODDDARG node to represent ... allocation.
arg = nod(ODDDARG, nil, nil)
arr := types.NewArray(n.Type.Elem(), int64(len(args)))
arg.Type = types.NewPtr(arr) // make pointer so it will be tracked
arg.Pos = call.Pos
e.track(arg)
call.Right = arg
i := 0

// Receiver.
if call.Op != OCALLFUNC {
rf := fntype.Recv()
if rf.Sym != nil && !rf.Sym.IsBlank() {
n := fn.Name.Defn.Func.Dcl[0]
i++
if n.Class() != PPARAM {
Fatalf("esccall: not a parameter %+v", n)
}
e.escassignWhyWhere(n, arg, "arg to recursive call", call) // TODO this message needs help.
if arg == args[0] {
e.escassignWhyWhere(n, call.Left.Left, "recursive call receiver", call)
}
}

// Parameters.
for _, param := range fntype.Params().FieldSlice() {
if param.Sym == nil || param.Sym.IsBlank() {
// Unnamed parameter is not listed in Func.Dcl.
// But we need to consume the arg.
if param.IsDDD() && !call.IsDDD() {
args = nil
} else {
args = args[1:]
continue
}
// "..." arguments are untracked
for _, a := range args {
if Debug['m'] > 3 {
fmt.Printf("%v::esccall:: ... <- %S, untracked\n", linestr(lineno), a)
}
e.escassignSinkWhyWhere(arg, a, "... arg to recursive call", call)
continue
}

n := fn.Name.Defn.Func.Dcl[i]
i++
if n.Class() != PPARAM {
Fatalf("esccall: not a parameter %+v", n)
}
if len(args) == 0 {
continue
}
arg := args[0]
if n.IsDDD() && !call.IsDDD() {
// Introduce ODDDARG node to represent ... allocation.
arg = nod(ODDDARG, nil, nil)
arr := types.NewArray(n.Type.Elem(), int64(len(args)))
arg.Type = types.NewPtr(arr) // make pointer so it will be tracked
arg.Pos = call.Pos
e.track(arg)
call.Right = arg
}
e.escassignWhyWhere(n, arg, "arg to recursive call", call) // TODO this message needs help.
if arg == args[0] {
args = args[1:]
continue
}
// "..." arguments are untracked
for _, a := range args {
if Debug['m'] > 3 {
fmt.Printf("%v::esccall:: ... <- %S, untracked\n", linestr(lineno), a)
}
// No more PPARAM processing, but keep
// going for PPARAMOUT.
args = nil
e.escassignSinkWhyWhere(arg, a, "... arg to recursive call", call)
}
// ... arg consumes all remaining arguments
args = nil
}

case PPARAMOUT:
// Results.
for _, n := range fn.Name.Defn.Func.Dcl[i:] {
if n.Class() == PPARAMOUT {
cE.Retval.Append(n)
}
}

// Sanity check: all arguments must be consumed.
if len(args) != 0 {
Fatalf("esccall not consumed all args %+v\n", call)
}
return
}

Expand Down
17 changes: 17 additions & 0 deletions test/escape5.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,20 @@ func f15730c(args ...interface{}) { // ERROR "leaking param content: args"
}
}
}

// Issue 29000: unnamed parameter is not handled correctly

var sink4 interface{}
var alwaysFalse = false

func f29000(_ int, x interface{}) { // ERROR "leaking param: x"
sink4 = x
if alwaysFalse {
g29000()
}
}

func g29000() {
x := 1
f29000(2, x) // ERROR "x escapes to heap"
}

0 comments on commit be09bdf

Please sign in to comment.