cmd/compile: fix unnamed parameter handling in escape analysis

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 <khr@golang.org>

diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go
index c003964fa74665bfe355b4bbdfc9c827318ac079..322b2dcd0bca2658846914f18b551bcb897f424d 100644 (file)
--- a/src/cmd/compile/internal/gc/esc.go
+++ b/src/cmd/compile/internal/gc/esc.go
@@ -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
        }
 

diff --git a/test/escape5.go b/test/escape5.go
index 03283a37f8470ac283061ee98459641c7c8ee979..e26ecd5275032d2b0ed8793f73d3c0d23cdce8b5 100644 (file)
--- a/test/escape5.go
+++ b/test/escape5.go
@@ -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"
+}