]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: use fixVariadicCall in escape analysis
authorMatthew Dempsky <mdempsky@google.com>
Wed, 22 Apr 2020 02:48:02 +0000 (19:48 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Thu, 23 Apr 2020 22:02:12 +0000 (22:02 +0000)
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
src/cmd/compile/internal/gc/fmt.go
src/cmd/compile/internal/gc/syntax.go
src/cmd/compile/internal/gc/walk.go
test/fixedbugs/issue31573.go

index 7a6b84d2c1ff1a915d402c10b843555a4f2e5713..5dc755186e979266d976ac4ebd8800f25e25770e 100644 (file)
@@ -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 {
index 1c6195aa17149c00331d03ac3cc2f3551912915f..8b9c04d24e1e98baecc1bb079d1714c0eac900d1 100644 (file)
@@ -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
index 940105a3459ab4c2a178ea8462d947d9655eb44a..6d8e023a4bfc966d90d7651c6b547df25417ae7f 100644 (file)
@@ -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))
index 8adaabc69446f79186a1f237bfc622a517c21006..8589c3e72b5f89c086e340bf834c12699d2c800b 100644 (file)
@@ -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)
 }
index fb4fdc81e7702a118a5b330b471c6d59cc701d58..c9ea84bbae97b023e7c3ac48ff7ed8cdf3435697 100644 (file)
@@ -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$"