From a44d06d3b4d665cee14342df1c81a385f9d2055f Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Tue, 21 Apr 2020 19:48:02 -0700 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/gc/escape.go | 240 +++++++++++--------------- src/cmd/compile/internal/gc/fmt.go | 4 + src/cmd/compile/internal/gc/syntax.go | 4 +- src/cmd/compile/internal/gc/walk.go | 6 +- test/fixedbugs/issue31573.go | 8 +- 5 files changed, 112 insertions(+), 150 deletions(-) diff --git a/src/cmd/compile/internal/gc/escape.go b/src/cmd/compile/internal/gc/escape.go index 7a6b84d2c1..5dc755186e 100644 --- a/src/cmd/compile/internal/gc/escape.go +++ b/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), - // its type, and receiver (if any) and normal arguments list. - var fn, recv *Node - var fntype *types.Type - args := call.List.Slice() - switch call.Op { - case OCALLFUNC: - fn = call.Left - if fn.Op == OCLOSURE { - fn = fn.Func.Closure.Func.Nname - } - fntype = fn.Type - if !(fn.Op == ONAME && fn.Class() == PFUNC) { - fn = nil // dynamic call + topLevelDefer := where != nil && where.Op == ODEFER && e.loopDepth == 1 + if topLevelDefer { + // force stack allocation of defer record, unless + // open-coded defers are used (see ssa.go) + 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() } - 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} + + e.expr(k.note(call, "call parameter"), arg) + } + + switch call.Op { default: Fatalf("unexpected call op: %v", call.Op) - } - // Setup evaluation holes for each receiver/argument. - var recvK EscHole - var paramKs []EscHole + 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 + } else if call.Left.Op == OCLOSURE { + fn = call.Left.Func.Closure.Func.Nname + } + case OCALLMETH: + fn = asNode(call.Left.Type.FuncType().Nname) + } + + fntype := call.Left.Type + if fn != nil { + 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) - } - for _, param := range fntype.Params().FieldSlice() { - paramKs = append(paramKs, e.tagHole(ks, fn, param)) - } - } else { - // Handle escape analysis for builtins. - // By default, we just discard everything. - for range args { - paramKs = append(paramKs, e.discardHole()) + argument(e.tagHole(ks, fn, r), call.Left.Left) + } else { + // Evaluate callee function expression. + argument(e.discardHole(), call.Left) } - switch call.Op { - case OAPPEND: - // 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. - 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")) - } - - if call.IsDDD() { - if args[1].Type.IsSlice() && types.Haspointers(args[1].Type.Elem()) { - paramKs[1] = e.teeHole(paramKs[1], e.heapHole().deref(call, "appended slice...")) - } - } else { - for i := 1; i < len(args); i++ { - paramKs[i] = e.heapHole() - } - } - - case OCOPY: - if call.Right.Type.IsSlice() && types.Haspointers(call.Right.Type.Elem()) { - paramKs[1] = e.teeHole(paramKs[1], e.heapHole().deref(call, "copied slice")) - } - - case OPANIC: - paramKs[0] = e.heapHole() + args := call.List.Slice() + for i, param := range fntype.Params().FieldSlice() { + argument(e.tagHole(ks, fn, param), args[i]) } - } - - if call.Op == OCALLFUNC { - // Evaluate callee function expression. - e.expr(e.augmentParamHole(e.discardHole(), call, where), call.Left) - } - - if recv != nil { - // 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 + case OAPPEND: + args := call.List.Slice() - // 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) + // 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]) - 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 call.IsDDD() { + appendedK := e.discardHole() + if args[1].Type.IsSlice() && types.Haspointers(args[1].Type.Elem()) { + appendedK = e.heapHole().deref(call, "appended slice...") } - if fntype.Params().Field(x).Note == uintptrEscapesTag { - arg = arg.Left + argument(appendedK, args[1]) + } else { + for _, arg := range args[1:] { + argument(e.heapHole(), arg) } } - // no augmentParamHole here; handled in loop before ODDDARG - e.expr(paramKs[i], arg) - } -} + case OCOPY: + argument(e.discardHole(), call.Left) -// 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 - } + copiedK := e.discardHole() + if call.Right.Type.IsSlice() && types.Haspointers(call.Right.Type.Elem()) { + copiedK = e.heapHole().deref(call, "copied slice") + } + argument(copiedK, call.Right) - // 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) - } + case OPANIC: + argument(e.heapHole(), call.Left) - return e.heapHole().note(where, "call parameter") + case OCOMPLEX: + argument(e.discardHole(), 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: + argument(e.discardHole(), call.Left) + } } // 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 { diff --git a/src/cmd/compile/internal/gc/fmt.go b/src/cmd/compile/internal/gc/fmt.go index 1c6195aa17..8b9c04d24e 100644 --- a/src/cmd/compile/internal/gc/fmt.go +++ b/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 diff --git a/src/cmd/compile/internal/gc/syntax.go b/src/cmd/compile/internal/gc/syntax.go index 940105a345..6d8e023a4b 100644 --- a/src/cmd/compile/internal/gc/syntax.go +++ b/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)) diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 8adaabc694..8589c3e72b 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/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) } diff --git a/test/fixedbugs/issue31573.go b/test/fixedbugs/issue31573.go index fb4fdc81e7..c9ea84bbae 100644 --- a/test/fixedbugs/issue31573.go +++ b/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$" -- 2.50.0