From be09bdf589a5c25e5d8b68a546e9b84bc1a1977b Mon Sep 17 00:00:00 2001
From: Cherry Zhang <cherryyz@google.com>
Date: Tue, 4 Dec 2018 15:54:14 -0500
Subject: [PATCH] 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>
---
 src/cmd/compile/internal/gc/esc.go | 98 +++++++++++++++++++-----------
 test/escape5.go                    | 17 ++++++
 2 files changed, 81 insertions(+), 34 deletions(-)

diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go
index c003964fa74..322b2dcd0bc 100644
--- 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 03283a37f84..e26ecd52750 100644
--- 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"
+}