cmd/compile: use fixVariadicCall in escape analysis

This CL uses fixVariadicCall before escape analyzing function calls.
This has a number of benefits, though also some minor obstacles:

Most notably, it allows us to remove ODDDARG along with the logic
involved in setting it up, manipulating EscHoles, and later copying
its escape analysis flags to the actual slice argument. Instead, we
uniformly handle all variadic calls the same way. (E.g., issue31573.go
is updated because now f() and f(nil...) are handled identically.)

It also allows us to simplify handling of builtins and generic
function calls. Previously handling of calls was hairy enough to
require multiple dispatches on n.Op, whereas now the logic is uniform
enough that we can easily handle it with a single dispatch.

The downside is handling //go:uintptrescapes is now somewhat clumsy.
(It used to be clumsy, but it still is, too.) The proper fix here is
probably to stop using escape analysis tags for //go:uintptrescapes
and unsafe-uintptr, and have an earlier pass responsible for them.

Finally, note that while we now call fixVariadicCall in Escape, we
still have to call it in Order, because we don't (yet) run Escape on
all compiler-generated functions. In particular, the generated "init"
function for initializing package-level variables can contain calls to
variadic functions and isn't escape analyzed.

Passes toolstash-check -race.

Change-Id: I4cdb92a393ac487910aeee58a5cb8c1500eef881
Reviewed-on: https://go-review.googlesource.com/c/go/+/229759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

src/cmd/compile/internal/gc/escape.go

@@ -428,7 +428,12 @@ func (e *Escape) exprSkipInit(k EscHole, n *Node) {
 		lineno = lno
 	}()
 
-	if k.derefs >= 0 && !types.Haspointers(n.Type) {
+	uintptrEscapesHack := k.uintptrEscapesHack
+	k.uintptrEscapesHack = false
+
+	if uintptrEscapesHack && n.Op == OCONVNOP && n.Left.Type.IsUnsafePtr() {
+		// nop
+	} else if k.derefs >= 0 && !types.Haspointers(n.Type) {
 		k = e.discardHole()
 	}
 
@@ -556,6 +561,7 @@ func (e *Escape) exprSkipInit(k EscHole, n *Node) {
 
 	case OSLICELIT:
 		k = e.spill(k, n)
+		k.uintptrEscapesHack = uintptrEscapesHack // for ...uintptr parameters
 
 		for _, elt := range n.List.Slice() {
 			if elt.Op == OKEY {
@@ -734,43 +740,51 @@ func (e *Escape) assignHeap(src *Node, why string, where *Node) {
 // should contain the holes representing where the function callee's
 // results flows; where is the OGO/ODEFER context of the call, if any.
 func (e *Escape) call(ks []EscHole, call, where *Node) {
-	// First, pick out the function callee (if statically known),
+	topLevelDefer := where != nil && where.Op == ODEFER && e.loopDepth == 1
-	// its type, and receiver (if any) and normal arguments list.
+	if topLevelDefer {
-	var fn, recv *Node
+		// force stack allocation of defer record, unless
-	var fntype *types.Type
+		// open-coded defers are used (see ssa.go)
-	args := call.List.Slice()
+		where.Esc = EscNever
+	}
+
+	argument := func(k EscHole, arg *Node) {
+		if topLevelDefer {
+			// Top level defers arguments don't escape to
+			// heap, but they do need to last until end of
+			// function.
+			k = e.later(k)
+		} else if where != nil {
+			k = e.heapHole()
+		}
+
+		e.expr(k.note(call, "call parameter"), arg)
+	}
+
+	switch call.Op {
+	default:
+		Fatalf("unexpected call op: %v", call.Op)
+
+	case OCALLFUNC, OCALLMETH, OCALLINTER:
+		fixVariadicCall(call)
+
+		// Pick out the function callee, if statically known.
+		var fn *Node
 		switch call.Op {
 		case OCALLFUNC:
+			if call.Left.Op == ONAME && call.Left.Class() == PFUNC {
 				fn = call.Left
-		if fn.Op == OCLOSURE {
+			} else if call.Left.Op == OCLOSURE {
-			fn = fn.Func.Closure.Func.Nname
+				fn = call.Left.Func.Closure.Func.Nname
-		}
-		fntype = fn.Type
-		if !(fn.Op == ONAME && fn.Class() == PFUNC) {
-			fn = nil // dynamic call
 			}
 		case OCALLMETH:
 			fn = asNode(call.Left.Type.FuncType().Nname)
-		fntype = fn.Type
-		recv = call.Left.Left
-	case OCALLINTER:
-		fntype = call.Left.Type
-		recv = call.Left.Left
-	case OAPPEND, ODELETE, OPRINT, OPRINTN, ORECOVER:
-		// ok
-	case OLEN, OCAP, OREAL, OIMAG, OCLOSE, OPANIC:
-		args = []*Node{call.Left}
-	case OCOMPLEX, OCOPY:
-		args = []*Node{call.Left, call.Right}
-	default:
-		Fatalf("unexpected call op: %v", call.Op)
 		}
 
-	// Setup evaluation holes for each receiver/argument.
+		fntype := call.Left.Type
-	var recvK EscHole
+		if fn != nil {
-	var paramKs []EscHole
+			fntype = fn.Type
+		}
 
-	if call.Op == OCALLFUNC || call.Op == OCALLMETH || call.Op == OCALLINTER {
 		if ks != nil && fn != nil && e.inMutualBatch(fn) {
 			for i, result := range fn.Type.Results().FieldSlice() {
 				e.expr(ks[i], asNode(result.Nname))
@@ -778,125 +792,64 @@ func (e *Escape) call(ks []EscHole, call, where *Node) {
 		}
 
 		if r := fntype.Recv(); r != nil {
-			recvK = e.tagHole(ks, fn, r)
+			argument(e.tagHole(ks, fn, r), call.Left.Left)
-		}
-		for _, param := range fntype.Params().FieldSlice() {
-			paramKs = append(paramKs, e.tagHole(ks, fn, param))
-		}
 		} else {
-		// Handle escape analysis for builtins.
+			// Evaluate callee function expression.
-		// By default, we just discard everything.
+			argument(e.discardHole(), call.Left)
-		for range args {
-			paramKs = append(paramKs, e.discardHole())
 		}
 
-		switch call.Op {
+		args := call.List.Slice()
-		case OAPPEND:
+		for i, param := range fntype.Params().FieldSlice() {
-			// Appendee slice may flow directly to the
+			argument(e.tagHole(ks, fn, param), args[i])
-			// result, if it has enough capacity.
-			// Alternatively, a new heap slice might be
-			// allocated, and all slice elements might
-			// flow to heap.
-			paramKs[0] = e.teeHole(paramKs[0], ks[0])
-			if types.Haspointers(args[0].Type.Elem()) {
-				paramKs[0] = e.teeHole(paramKs[0], e.heapHole().deref(call, "appendee slice"))
 		}
 
+	case OAPPEND:
+		args := call.List.Slice()
+
+		// Appendee slice may flow directly to the result, if
+		// it has enough capacity. Alternatively, a new heap
+		// slice might be allocated, and all slice elements
+		// might flow to heap.
+		appendeeK := ks[0]
+		if types.Haspointers(args[0].Type.Elem()) {
+			appendeeK = e.teeHole(appendeeK, e.heapHole().deref(call, "appendee slice"))
+		}
+		argument(appendeeK, args[0])
+
 		if call.IsDDD() {
+			appendedK := e.discardHole()
 			if args[1].Type.IsSlice() && types.Haspointers(args[1].Type.Elem()) {
-					paramKs[1] = e.teeHole(paramKs[1], e.heapHole().deref(call, "appended slice..."))
+				appendedK = e.heapHole().deref(call, "appended slice...")
 			}
+			argument(appendedK, args[1])
 		} else {
-				for i := 1; i < len(args); i++ {
+			for _, arg := range args[1:] {
-					paramKs[i] = e.heapHole()
+				argument(e.heapHole(), arg)
 			}
 		}
 
 	case OCOPY:
+		argument(e.discardHole(), call.Left)
+
+		copiedK := e.discardHole()
 		if call.Right.Type.IsSlice() && types.Haspointers(call.Right.Type.Elem()) {
-				paramKs[1] = e.teeHole(paramKs[1], e.heapHole().deref(call, "copied slice"))
+			copiedK = e.heapHole().deref(call, "copied slice")
 		}
+		argument(copiedK, call.Right)
 
 	case OPANIC:
-			paramKs[0] = e.heapHole()
+		argument(e.heapHole(), call.Left)
-		}
-	}
 
-	if call.Op == OCALLFUNC {
+	case OCOMPLEX:
-		// Evaluate callee function expression.
+		argument(e.discardHole(), call.Left)
-		e.expr(e.augmentParamHole(e.discardHole(), call, where), call.Left)
+		argument(e.discardHole(), call.Right)
+	case ODELETE, OPRINT, OPRINTN, ORECOVER:
+		for _, arg := range call.List.Slice() {
+			argument(e.discardHole(), arg)
 		}
-
+	case OLEN, OCAP, OREAL, OIMAG, OCLOSE:
-	if recv != nil {
+		argument(e.discardHole(), call.Left)
-		// TODO(mdempsky): Handle go:uintptrescapes here too?
-		e.expr(e.augmentParamHole(recvK, call, where), recv)
 	}
-
-	// Apply augmentParamHole before ODDDARG so that it affects
-	// the implicit slice allocation for variadic calls, if any.
-	for i, paramK := range paramKs {
-		paramKs[i] = e.augmentParamHole(paramK, call, where)
-	}
-
-	// TODO(mdempsky): Remove after early ddd-ification.
-	if fntype != nil && fntype.IsVariadic() && !call.IsDDD() {
-		vi := fntype.NumParams() - 1
-
-		elt := fntype.Params().Field(vi).Type.Elem()
-		nva := call.List.Len()
-		nva -= vi
-
-		// Introduce ODDDARG node to represent ... allocation.
-		ddd := nodl(call.Pos, ODDDARG, nil, nil)
-		ddd.Type = types.NewPtr(types.NewArray(elt, int64(nva)))
-		call.Right = ddd
-
-		dddK := e.spill(paramKs[vi], ddd)
-		paramKs = paramKs[:vi]
-		for i := 0; i < nva; i++ {
-			paramKs = append(paramKs, dddK)
-		}
-	}
-
-	for i, arg := range args {
-		// For arguments to go:uintptrescapes, peel
-		// away an unsafe.Pointer->uintptr conversion,
-		// if present.
-		if fn != nil && arg.Op == OCONVNOP && arg.Type.Etype == TUINTPTR && arg.Left.Type.Etype == TUNSAFEPTR {
-			x := i
-			if fntype.IsVariadic() && x >= fntype.NumParams() {
-				x = fntype.NumParams() - 1
-			}
-			if fntype.Params().Field(x).Note == uintptrEscapesTag {
-				arg = arg.Left
-			}
-		}
-
-		// no augmentParamHole here; handled in loop before ODDDARG
-		e.expr(paramKs[i], arg)
-	}
-}
-
-// augmentParamHole augments parameter holes as necessary for use in
-// go/defer statements.
-func (e *Escape) augmentParamHole(k EscHole, call, where *Node) EscHole {
-	k = k.note(call, "call parameter")
-	if where == nil {
-		return k
-	}
-
-	// Top level defers arguments don't escape to heap, but they
-	// do need to last until end of function. Tee with a
-	// non-transient location to avoid arguments from being
-	// transiently allocated.
-	if where.Op == ODEFER && e.loopDepth == 1 {
-		// force stack allocation of defer record, unless open-coded
-		// defers are used (see ssa.go)
-		where.Esc = EscNever
-		return e.later(k)
-	}
-
-	return e.heapHole().note(where, "call parameter")
 }
 
 // tagHole returns a hole for evaluating an argument passed to param.
@@ -914,6 +867,13 @@ func (e *Escape) tagHole(ks []EscHole, fn *Node, param *types.Field) EscHole {
 	}
 
 	// Call to previously tagged function.
+
+	if param.Note == uintptrEscapesTag {
+		k := e.heapHole()
+		k.uintptrEscapesHack = true
+		return k
+	}
+
 	var tagKs []EscHole
 
 	esc := ParseLeaks(param.Note)
@@ -954,6 +914,10 @@ type EscHole struct {
 	dst    *EscLocation
 	derefs int // >= -1
 	notes  *EscNote
+
+	// uintptrEscapesHack indicates this context is evaluating an
+	// argument for a //go:uintptrescapes function.
+	uintptrEscapesHack bool
 }
 
 type EscNote struct {

src/cmd/compile/internal/gc/fmt.go

@@ -1404,6 +1404,10 @@ func (n *Node) exprfmt(s fmt.State, prec int, mode fmtMode) {
 
 	case OCOMPLIT:
 		if mode == FErr {
+			if n.Implicit() {
+				mode.Fprintf(s, "... argument")
+				return
+			}
 			if n.Right != nil {
 				mode.Fprintf(s, "%v literal", n.Right)
 				return

src/cmd/compile/internal/gc/syntax.go

@@ -686,10 +686,8 @@ const (
 
 	// OCALLFUNC, OCALLMETH, and OCALLINTER have the same structure.
 	// Prior to walk, they are: Left(List), where List is all regular arguments.
-	// If present, Right is an ODDDARG that holds the
-	// generated slice used in a call to a variadic function.
 	// After walk, List is a series of assignments to temporaries,
-	// and Rlist is an updated set of arguments, including any ODDDARG slice.
+	// and Rlist is an updated set of arguments.
 	// TODO(josharian/khr): Use Ninit instead of List for the assignments to temporaries. See CL 114797.
 	OCALLFUNC  // Left(List/Rlist) (function call f(args))
 	OCALLMETH  // Left(List/Rlist) (direct method call x.Method(args))

src/cmd/compile/internal/gc/walk.go

@@ -1725,6 +1725,7 @@ func mkdotargslice(typ *types.Type, args []*Node) *Node {
 	} else {
 		n = nod(OCOMPLIT, nil, typenod(typ))
 		n.List.Append(args...)
+		n.SetImplicit(true)
 	}
 
 	n = typecheck(n, ctxExpr)
@@ -1752,11 +1753,6 @@ func fixVariadicCall(call *Node) {
 		extra[i] = nil // allow GC
 	}
 
-	if ddd := call.Right; ddd != nil && slice.Op == OSLICELIT {
-		slice.Esc = ddd.Esc
-		slice.SetTransient(ddd.Transient())
-	}
-
 	call.List.Set(append(args[:vi], slice))
 	call.SetIsDDD(true)
 }

test/fixedbugs/issue31573.go

@@ -9,7 +9,7 @@ package p
 func f(...*int) {} // ERROR "can inline f$"
 
 func g() {
-	defer f()                   // ERROR "... argument does not escape$"
+	defer f()
 	defer f(new(int))           // ERROR "... argument does not escape$" "new\(int\) does not escape$"
 	defer f(new(int), new(int)) // ERROR "... argument does not escape$" "new\(int\) does not escape$"
 
@@ -18,7 +18,7 @@ func g() {
 	defer f([]*int{new(int)}...)           // ERROR "\[\]\*int literal does not escape$" "new\(int\) does not escape$"
 	defer f([]*int{new(int), new(int)}...) // ERROR "\[\]\*int literal does not escape$" "new\(int\) does not escape$"
 
-	go f()                   // ERROR "... argument escapes to heap$"
+	go f()
 	go f(new(int))           // ERROR "... argument escapes to heap$" "new\(int\) escapes to heap$"
 	go f(new(int), new(int)) // ERROR "... argument escapes to heap$" "new\(int\) escapes to heap$"
 
@@ -28,7 +28,7 @@ func g() {
 	go f([]*int{new(int), new(int)}...) // ERROR "\[\]\*int literal escapes to heap$" "new\(int\) escapes to heap$"
 
 	for {
-		defer f()                   // ERROR "... argument escapes to heap$"
+		defer f()
 		defer f(new(int))           // ERROR "... argument escapes to heap$" "new\(int\) escapes to heap$"
 		defer f(new(int), new(int)) // ERROR "... argument escapes to heap$" "new\(int\) escapes to heap$"
 
@@ -37,7 +37,7 @@ func g() {
 		defer f([]*int{new(int)}...)           // ERROR "\[\]\*int literal escapes to heap$" "new\(int\) escapes to heap$"
 		defer f([]*int{new(int), new(int)}...) // ERROR "\[\]\*int literal escapes to heap$" "new\(int\) escapes to heap$"
 
-		go f()                   // ERROR "... argument escapes to heap$"
+		go f()
 		go f(new(int))           // ERROR "... argument escapes to heap$" "new\(int\) escapes to heap$"
 		go f(new(int), new(int)) // ERROR "... argument escapes to heap$" "new\(int\) escapes to heap$"