cmd/compile: fix ICE from go/defer call to variadic function
The special case logic for go/defer arguments in Escape.call was
scattered around a bit and was somewhat inconsistently handled across
different types of function calls and parameters. This CL pulls the
logic out into a separate callStmt method that's used uniformly for
all kinds of function calls and arguments.
Fixes #31573.
Change-Id: Icdcdf611754dc3fcf1af7cb52879fb4b73a7a31f
Reviewed-on: https://go-review.googlesource.com/c/go/+/173019
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
diff --git a/src/cmd/compile/internal/gc/escape.go b/src/cmd/compile/internal/gc/escape.go
index 61be503b..88dc9ef 100644
--- a/src/cmd/compile/internal/gc/escape.go
+++ b/src/cmd/compile/internal/gc/escape.go
@@ -746,14 +746,7 @@
var recvK EscHole
var paramKs []EscHole
- if where != nil && !(where.Op == ODEFER && e.loopDepth == 1) {
- if recv != nil {
- recvK = e.heapHole()
- }
- for range args {
- paramKs = append(paramKs, e.heapHole())
- }
- } else if static && fn.Name.Defn != nil && fn.Name.Defn.Esc < EscFuncTagged {
+ if static && fn.Name.Defn != nil && fn.Name.Defn.Esc < EscFuncTagged {
// Static call to function in same mutually recursive
// group; incorporate into data flow graph.
@@ -778,32 +771,25 @@
// function. Setup flows to heap and/or ks according
// to parameter tags.
if r := fntype.Recv(); r != nil {
- recvK = e.tagHole(ks, r, static, where)
+ recvK = e.tagHole(ks, r, static)
}
for _, param := range fntype.Params().FieldSlice() {
- paramKs = append(paramKs, e.tagHole(ks, param, static, where))
+ paramKs = append(paramKs, e.tagHole(ks, param, static))
}
} else {
// Handle escape analysis for builtins.
-
- // By default, we just discard everything. However, if
- // we're in a top-level defer statement, we can't
- // allow transient values.
- k := e.discardHole()
- if where != nil {
- k = e.newLoc(where, false).asHole()
- }
+ // By default, we just discard everything.
for range args {
- paramKs = append(paramKs, k)
+ paramKs = append(paramKs, e.discardHole())
}
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.
+ // 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"))
@@ -829,6 +815,22 @@
}
}
+ if call.Op == OCALLFUNC {
+ // Evaluate callee function expression.
+ e.expr(e.augmentParamHole(e.discardHole(), where), call.Left)
+ }
+
+ if recv != nil {
+ // TODO(mdempsky): Handle go:uintptrescapes here too?
+ e.expr(e.augmentParamHole(recvK, 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, where)
+ }
+
// TODO(mdempsky): Remove after early ddd-ification.
if fntype != nil && fntype.IsVariadic() && !call.IsDDD() {
vi := fntype.NumParams() - 1
@@ -849,24 +851,6 @@
}
}
- if call.Op == OCALLFUNC {
- // Evaluate callee function expression.
- k := e.discardHole()
- if where != nil {
- if where.Op == ODEFER && e.loopDepth == 1 {
- k = e.newLoc(nil, false).asHole()
- } else {
- k = e.heapHole()
- }
- }
- e.expr(k, call.Left)
- }
-
- if recv != nil {
- // TODO(mdempsky): Handle go:uintptrescapes here too?
- e.expr(recvK, recv)
- }
-
for i, arg := range args {
// For arguments to go:uintptrescapes, peel
// away an unsafe.Pointer->uintptr conversion,
@@ -881,15 +865,35 @@
}
}
+ // 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, where *Node) EscHole {
+ 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 {
+ // TODO(mdempsky): Eliminate redundant EscLocation allocs.
+ return e.teeHole(k, e.newLoc(nil, false).asHole())
+ }
+
+ return e.heapHole()
+}
+
// tagHole returns a hole for evaluating an argument passed to param.
// ks should contain the holes representing where the function
// callee's results flows; static indicates whether this is a static
-// call; where is the OGO/ODEFER context of the call, if any.
-func (e *Escape) tagHole(ks []EscHole, param *types.Field, static bool, where *Node) EscHole {
+// call.
+func (e *Escape) tagHole(ks []EscHole, param *types.Field, static bool) EscHole {
// If this is a dynamic call, we can't rely on param.Note.
if !static {
return e.heapHole()
@@ -902,10 +906,6 @@
}
var tagKs []EscHole
- if where != nil {
- tagKs = append(tagKs, e.newLoc(nil, false).asHole())
- }
-
if esc&EscContentEscapes != 0 {
tagKs = append(tagKs, e.heapHole().shift(1))
}
diff --git a/test/fixedbugs/issue31573.go b/test/fixedbugs/issue31573.go
new file mode 100644
index 0000000..fb4fdc8
--- /dev/null
+++ b/test/fixedbugs/issue31573.go
@@ -0,0 +1,49 @@
+// errorcheck -0 -m
+
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+func f(...*int) {} // ERROR "can inline f$"
+
+func g() {
+ defer f() // ERROR "... argument does not escape$"
+ 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$"
+
+ defer f(nil...)
+ defer f([]*int{}...) // ERROR "\[\]\*int literal does not escape$"
+ 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(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$"
+
+ go f(nil...)
+ go f([]*int{}...) // ERROR "\[\]\*int literal escapes to heap$"
+ go f([]*int{new(int)}...) // ERROR "\[\]\*int literal escapes to heap$" "new\(int\) escapes to heap$"
+ 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(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$"
+
+ defer f(nil...)
+ defer f([]*int{}...) // ERROR "\[\]\*int literal escapes to heap$"
+ 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(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$"
+
+ go f(nil...)
+ go f([]*int{}...) // ERROR "\[\]\*int literal escapes to heap$"
+ go f([]*int{new(int)}...) // ERROR "\[\]\*int literal escapes to heap$" "new\(int\) escapes to heap$"
+ go f([]*int{new(int), new(int)}...) // ERROR "\[\]\*int literal escapes to heap$" "new\(int\) escapes to heap$"
+ }
+}
