cmd/compile: don't use PPARAMOUT names for temps

The location of VARDEFs is incorrect for PPARAMOUT variables
which are also used as temporary locations.  We put in VARDEFs
when setting the variable at return time, but when the location
is also used as a temporary the lifetime values are wrong.

Fix copyelim to update the names map properly.  This is a
real name bug fix which, as a result, allows me to
write a reasonable test to trigger the PPARAMOUT bug.

This is kind of a band-aid fix for #14591.  A more pricipled
fix (which allows values to be stored in the return variable
earlier than the return point) will be harder.

Fixes #14591

Change-Id: I7df8ae103a982d1f218ed704c080d7b83cdcfdd9
Reviewed-on: https://go-review.googlesource.com/20457
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index afba7db6381c5917ca355aed51d6a5068a62bd5a..87d23742161ee64b3cf27c5096fad06b0d545db5 100644 (file)
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -3608,6 +3608,11 @@ func (s *state) addNamedValue(n *Node, v *ssa.Value) {
                // pseudos in the right place when we spill to these nodes.
                return
        }
+       if n.Class == PPARAMOUT {
+               // Don't track named output values.  This prevents return values
+               // from being assigned too early. See #14591 and #14762. TODO: allow this.
+               return
+       }
        if n.Class == PAUTO && n.Xoffset != 0 {
                s.Fatalf("AUTO var with offset %s %d", n, n.Xoffset)
        }

diff --git a/src/cmd/compile/internal/ssa/copyelim.go b/src/cmd/compile/internal/ssa/copyelim.go
index cfeff21e84f4b5873c47c290d92badfa9ac826e4..548813412283581bd09111fab8752822013f433d 100644 (file)
--- a/src/cmd/compile/internal/ssa/copyelim.go
+++ b/src/cmd/compile/internal/ssa/copyelim.go
@@ -28,7 +28,7 @@ func copyelim(f *Func) {
                                x = x.Args[0]
                        }
                        if x != v {
-                               values[i] = v
+                               values[i] = x
                        }
                }
        }

diff --git a/test/fixedbugs/issue14591.go b/test/fixedbugs/issue14591.go
new file mode 100644 (file)
index 0000000..e4fa80a
--- /dev/null
+++ b/test/fixedbugs/issue14591.go
@@ -0,0 +1,38 @@
+// 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.
+
+// Test to make sure we don't think values are dead
+// when they are assigned to a PPARAMOUT slot before
+// the last GC safepoint.
+
+package main
+
+import (
+       "fmt"
+       "runtime"
+)
+
+// When a T is deallocated, T[1] is certain to
+// get clobbered (the runtime writes 0xdeaddeaddeaddead there).
+type T [4]int
+
+func f() (r, s *T) {
+       r = &T{0x30, 0x31, 0x32, 0x33}
+       runtime.GC()
+       s = &T{0x40, 0x41, 0x42, 0x43}
+       runtime.GC()
+       return
+}
+
+func main() {
+       r, s := f()
+       if r[1] != 0x31 {
+               fmt.Printf("bad r[1], want 0x31 got %x\n", r[1])
+       }
+       if s[1] != 0x41 {
+               fmt.Printf("bad s[1], want 0x41 got %x\n", s[1])
+       }
+}