Skip to content

Commit 31d13f4

Browse files
committed
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 <[email protected]>
1 parent fb9aafa commit 31d13f4

File tree

3 files changed

+44
-1
lines changed

3 files changed

+44
-1
lines changed

src/cmd/compile/internal/gc/ssa.go

+5
Original file line numberDiff line numberDiff line change
@@ -3608,6 +3608,11 @@ func (s *state) addNamedValue(n *Node, v *ssa.Value) {
36083608
// pseudos in the right place when we spill to these nodes.
36093609
return
36103610
}
3611+
if n.Class == PPARAMOUT {
3612+
// Don't track named output values. This prevents return values
3613+
// from being assigned too early. See #14591 and #14762. TODO: allow this.
3614+
return
3615+
}
36113616
if n.Class == PAUTO && n.Xoffset != 0 {
36123617
s.Fatalf("AUTO var with offset %s %d", n, n.Xoffset)
36133618
}

src/cmd/compile/internal/ssa/copyelim.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func copyelim(f *Func) {
2828
x = x.Args[0]
2929
}
3030
if x != v {
31-
values[i] = v
31+
values[i] = x
3232
}
3333
}
3434
}

test/fixedbugs/issue14591.go

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// run
2+
3+
// Copyright 2016 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Test to make sure we don't think values are dead
8+
// when they are assigned to a PPARAMOUT slot before
9+
// the last GC safepoint.
10+
11+
package main
12+
13+
import (
14+
"fmt"
15+
"runtime"
16+
)
17+
18+
// When a T is deallocated, T[1] is certain to
19+
// get clobbered (the runtime writes 0xdeaddeaddeaddead there).
20+
type T [4]int
21+
22+
func f() (r, s *T) {
23+
r = &T{0x30, 0x31, 0x32, 0x33}
24+
runtime.GC()
25+
s = &T{0x40, 0x41, 0x42, 0x43}
26+
runtime.GC()
27+
return
28+
}
29+
30+
func main() {
31+
r, s := f()
32+
if r[1] != 0x31 {
33+
fmt.Printf("bad r[1], want 0x31 got %x\n", r[1])
34+
}
35+
if s[1] != 0x41 {
36+
fmt.Printf("bad s[1], want 0x41 got %x\n", s[1])
37+
}
38+
}

0 commit comments

Comments
 (0)