cmd/compile: make sure output params are live if there is a defer

If there is a defer, and that defer recovers, then the caller
can see all of the output parameters.  That means that we must
mark all the output parameters live at any point which might panic.

If there is no defer then this is not necessary.  This is implemented.

We could also detect whether there is a recover in any of the defers.
If not, we would need to mark only output params that the defer
actually references (and the closure mechanism already does that).
This is not implemented.

Fixes #18860.

Change-Id: If984fe6686eddce9408bf25e725dd17fc16b8578
Reviewed-on: https://go-review.googlesource.com/36030
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/compile/internal/gc/plive.go b/src/cmd/compile/internal/gc/plive.go
index 03161f889f7bd103ff13450e01ee5238b0cd7c0f..dad9ab5acf4dac45eb19cff2f95f1d15eae1fcb6 100644 (file)
--- a/src/cmd/compile/internal/gc/plive.go
+++ b/src/cmd/compile/internal/gc/plive.go
@@ -1192,18 +1192,32 @@ func livenessepilogue(lv *Liveness) {
        avarinit := bvalloc(nvars)
        any := bvalloc(nvars)
        all := bvalloc(nvars)
-       pparamout := bvalloc(localswords())
-
-       // Record pointers to heap-allocated pparamout variables.  These
-       // are implicitly read by post-deferreturn code and thus must be
-       // kept live throughout the function (if there is any defer that
-       // recovers).
+       outLive := bvalloc(argswords())       // always-live output params
+       outLiveHeap := bvalloc(localswords()) // always-live pointers to heap-allocated copies of output params
+
+       // If there is a defer (that could recover), then all output
+       // parameters are live all the time.  In addition, any locals
+       // that are pointers to heap-allocated output parameters are
+       // also always live (post-deferreturn code needs these
+       // pointers to copy values back to the stack).
+       // TODO: if the output parameter is heap-allocated, then we
+       // don't need to keep the stack copy live?
        if hasdefer {
                for _, n := range lv.vars {
+                       if n.Class == PPARAMOUT {
+                               if n.IsOutputParamHeapAddr() {
+                                       // Just to be paranoid.
+                                       Fatalf("variable %v both output param and heap output param", n)
+                               }
+                               // Needzero not necessary, as the compiler
+                               // explicitly zeroes output vars at start of fn.
+                               xoffset := n.Xoffset
+                               onebitwalktype1(n.Type, &xoffset, outLive)
+                       }
                        if n.IsOutputParamHeapAddr() {
                                n.Name.Needzero = true
                                xoffset := n.Xoffset + stkptrsize
-                               onebitwalktype1(n.Type, &xoffset, pparamout)
+                               onebitwalktype1(n.Type, &xoffset, outLiveHeap)
                        }
                }
        }
@@ -1357,7 +1371,8 @@ func livenessepilogue(lv *Liveness) {
 
                                // Mark pparamout variables (as described above)
                                if p.As == obj.ACALL {
-                                       locals.Or(locals, pparamout)
+                                       args.Or(args, outLive)
+                                       locals.Or(locals, outLiveHeap)
                                }
 
                                // Show live pointer bitmaps.

diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index dac3787dc9a05187d5a20c16e77fcb1836e2e6ac..05e97a904f35ff85b67d210f23850df1697d4cde 100644 (file)
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -3200,6 +3200,8 @@ func (s *state) canSSA(n *Node) bool {
                        // TODO: handle this case?  Named return values must be
                        // in memory so that the deferred function can see them.
                        // Maybe do: if !strings.HasPrefix(n.String(), "~") { return false }
+                       // Or maybe not, see issue 18860.  Even unnamed return values
+                       // must be written back so if a defer recovers, the caller can see them.
                        return false
                }
                if s.cgoUnsafeArgs {

diff --git a/test/live.go b/test/live.go
index 462f3ef12e92965d6b5fbb9707d20c09f797e226..0f2d81336d7bb609b0be17e9af844f979733d671 100644 (file)
--- a/test/live.go
+++ b/test/live.go
@@ -674,3 +674,15 @@ type T struct{}
 func (*T) Foo(ptr *int) {}
 
 type R struct{ *T } // ERRORAUTO "live at entry to \(\*R\)\.Foo: \.this ptr" "live at entry to R\.Foo: \.this ptr"
+
+// issue 18860: output arguments must be live all the time if there is a defer.
+// In particular, at printint r must be live.
+func f41(p, q *int) (r *int) { // ERROR "live at entry to f41: p q$"
+       r = p
+       defer func() {
+               recover()
+       }() // ERROR "live at call to deferproc: q r$" "live at call to deferreturn: r$"
+       printint(0) // ERROR "live at call to printint: q r$"
+       r = q
+       return // ERROR "live at call to deferreturn: r$"
+}