]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: keep heap pointer for escaping output parameters live
authorKeith Randall <khr@golang.org>
Sun, 19 Jun 2016 02:40:57 +0000 (19:40 -0700)
committerKeith Randall <khr@golang.org>
Mon, 27 Jun 2016 16:48:48 +0000 (16:48 +0000)
Make sure the pointer to the heap copy of an output parameter is kept
live throughout the function.  The function could panic at any point,
and then a defer could recover.  Thus, we need the pointer to the heap
copy always available so the post-deferreturn code can copy the return
value back to the stack.

Before this CL, the pointer to the heap copy could be considered dead in
certain situations, like code which is reverse dominated by a panic call.

Fixes #16095.

Change-Id: Ic3800423e563670e5b567b473bf4c84cddb49a4c
Reviewed-on: https://go-review.googlesource.com/24213
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
src/cmd/compile/internal/gc/gen.go
src/cmd/compile/internal/gc/plive.go
src/cmd/compile/internal/gc/syntax.go
test/fixedbugs/issue16095.go [new file with mode: 0644]

index 3faf6d4a63cc9c28eef7ffb898d5d30a60f06def..fc0003da816af4342870d43f61d336605fb45787 100644 (file)
@@ -160,6 +160,14 @@ func moveToHeap(n *Node) {
                if n.Class == PPARAM {
                        stackcopy.SetNotLiveAtEnd(true)
                }
+               if n.Class == PPARAMOUT {
+                       // Make sure the pointer to the heap copy is kept live throughout the function.
+                       // The function could panic at any point, and then a defer could recover.
+                       // Thus, we need the pointer to the heap copy always available so the
+                       // post-deferreturn code can copy the return value back to the stack.
+                       // See issue 16095.
+                       heapaddr.setIsOutputParamHeapAddr(true)
+               }
                n.Name.Param.Stackcopy = stackcopy
 
                // Substitute the stackcopy into the function variable list so that
index 7d0d2dd89435f7c9b6dde6a5efe95b33a45f9b30..9c39ca7022398cde3a64da8c955c918e81522ea5 100644 (file)
@@ -1175,6 +1175,18 @@ func livenessepilogue(lv *Liveness) {
        all := bvalloc(nvars)
        ambig := bvalloc(localswords())
 
+       // Set ambig bit for the 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).
+       if hasdefer {
+               for _, n := range lv.vars {
+                       if n.IsOutputParamHeapAddr() {
+                               xoffset := n.Xoffset + stkptrsize
+                               onebitwalktype1(n.Type, &xoffset, ambig)
+                       }
+               }
+       }
+
        for _, bb := range lv.cfg {
                // Compute avarinitany and avarinitall for entry to block.
                // This duplicates information known during livenesssolve
index e673db9004804c5502c81c42f30c6ac1494487bd..fab86976271b85b52c1d5d37ffeb0374e0ec81d8 100644 (file)
@@ -79,6 +79,7 @@ const (
        hasBreak = 1 << iota
        notLiveAtEnd
        isClosureVar
+       isOutputParamHeapAddr
 )
 
 func (n *Node) HasBreak() bool {
@@ -112,6 +113,17 @@ func (n *Node) setIsClosureVar(b bool) {
        }
 }
 
+func (n *Node) IsOutputParamHeapAddr() bool {
+       return n.flags&isOutputParamHeapAddr != 0
+}
+func (n *Node) setIsOutputParamHeapAddr(b bool) {
+       if b {
+               n.flags |= isOutputParamHeapAddr
+       } else {
+               n.flags &^= isOutputParamHeapAddr
+       }
+}
+
 // Val returns the Val for the node.
 func (n *Node) Val() Val {
        if n.hasVal != +1 {
diff --git a/test/fixedbugs/issue16095.go b/test/fixedbugs/issue16095.go
new file mode 100644 (file)
index 0000000..864b4b7
--- /dev/null
@@ -0,0 +1,104 @@
+// run
+
+// Copyright 2016 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 main
+
+import (
+       "fmt"
+       "runtime"
+)
+
+var sink *[20]byte
+
+func f() (x [20]byte) {
+       // Initialize x.
+       for i := range x {
+               x[i] = byte(i)
+       }
+
+       // Force x to be allocated on the heap.
+       sink = &x
+       sink = nil
+
+       // Go to deferreturn after the panic below.
+       defer func() {
+               recover()
+       }()
+
+       // This call collects the heap-allocated version of x (oops!)
+       runtime.GC()
+
+       // Allocate that same object again and clobber it.
+       y := new([20]byte)
+       for i := 0; i < 20; i++ {
+               y[i] = 99
+       }
+       // Make sure y is heap allocated.
+       sink = y
+
+       panic(nil)
+
+       // After the recover we reach the deferreturn, which
+       // copies the heap version of x back to the stack.
+       // It gets the pointer to x from a stack slot that was
+       // not marked as live during the call to runtime.GC().
+}
+
+var sinkint int
+
+func g(p *int) (x [20]byte) {
+       // Initialize x.
+       for i := range x {
+               x[i] = byte(i)
+       }
+
+       // Force x to be allocated on the heap.
+       sink = &x
+       sink = nil
+
+       // Go to deferreturn after the panic below.
+       defer func() {
+               recover()
+       }()
+
+       // This call collects the heap-allocated version of x (oops!)
+       runtime.GC()
+
+       // Allocate that same object again and clobber it.
+       y := new([20]byte)
+       for i := 0; i < 20; i++ {
+               y[i] = 99
+       }
+       // Make sure y is heap allocated.
+       sink = y
+
+       // panic with a non-call (with no fallthrough)
+       for {
+               sinkint = *p
+       }
+
+       // After the recover we reach the deferreturn, which
+       // copies the heap version of x back to the stack.
+       // It gets the pointer to x from a stack slot that was
+       // not marked as live during the call to runtime.GC().
+}
+
+func main() {
+       x := f()
+       for i, v := range x {
+               if v != byte(i) {
+                       fmt.Printf("%v\n", x)
+                       panic("bad f")
+               }
+       }
+       x = g(nil)
+       for i, v := range x {
+               if v != byte(i) {
+                       fmt.Printf("%v\n", x)
+                       panic("bad g")
+               }
+       }
+}