cmd/compile: pre-spill pointers in aggregate-typed register args

There's a problem in liveness, where liveness of any
part of an aggregate keeps the whole aggregate alive,
but the not-live parts don't get spilled.  The GC
can observe those live-but-not-spilled slots, which
can contain junk.

A better fix is to change liveness to work
pointer-by-pointer, but that is also a riskier,
trickier fix.

To avoid this, in the case of

(1) an aggregate input parameter
(2) containing pointers
(3) passed in registers

pre-spill the pointers.

Updates #40724.

Change-Id: I6beb8e0a353b1ae3c68c16072f56698061922c04
Reviewed-on: https://go-review.googlesource.com/c/go/+/307909
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index 48102e53984f5955aab060fc4cd58d1a42ce9a53..97b970012ddd91da33b30098a003171edc4e85d2 100644 (file)
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -558,6 +558,11 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
                                v := s.newValue0A(ssa.OpArg, n.Type(), n)
                                s.vars[n] = v
                                s.addNamedValue(n, v) // This helps with debugging information, not needed for compilation itself.
+                               // TODO(register args) Make liveness more fine-grained to that partial spilling is okay.
+                               paramAssignment := ssa.ParamAssignmentForArgName(s.f, n)
+                               if len(paramAssignment.Registers) > 1 && n.Type().HasPointers() { // 1 cannot be partially live
+                                       s.storeParameterRegsToStack(s.f.ABISelf, paramAssignment, n, s.decladdrs[n], true)
+                               }
                        } else { // address was taken AND/OR too large for SSA
                                paramAssignment := ssa.ParamAssignmentForArgName(s.f, n)
                                if len(paramAssignment.Registers) > 0 {
@@ -567,9 +572,7 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
                                        } else { // Too big for SSA.
                                                // Brute force, and early, do a bunch of stores from registers
                                                // TODO fix the nasty storeArgOrLoad recursion in ssa/expand_calls.go so this Just Works with store of a big Arg.
-                                               abi := s.f.ABISelf
-                                               addr := s.decladdrs[n]
-                                               s.storeParameterRegsToStack(abi, paramAssignment, n, addr)
+                                               s.storeParameterRegsToStack(s.f.ABISelf, paramAssignment, n, s.decladdrs[n], false)
                                        }
                                }
                        }
@@ -645,9 +648,12 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
        return s.f
 }
 
-func (s *state) storeParameterRegsToStack(abi *abi.ABIConfig, paramAssignment *abi.ABIParamAssignment, n *ir.Name, addr *ssa.Value) {
+func (s *state) storeParameterRegsToStack(abi *abi.ABIConfig, paramAssignment *abi.ABIParamAssignment, n *ir.Name, addr *ssa.Value, pointersOnly bool) {
        typs, offs := paramAssignment.RegisterTypesAndOffsets()
        for i, t := range typs {
+               if pointersOnly && !t.IsPtrShaped() {
+                       continue
+               }
                r := paramAssignment.Registers[i]
                o := offs[i]
                op, reg := ssa.ArgOpAndRegisterFor(r, abi)

diff --git a/test/abi/part_live.go b/test/abi/part_live.go
new file mode 100644 (file)
index 0000000..592b6b3
--- /dev/null
+++ b/test/abi/part_live.go
@@ -0,0 +1,48 @@
+// run
+
+// Copyright 2021 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.
+
+// A test for partial liveness / partial spilling / compiler-induced GC failure
+
+package main
+
+import "runtime"
+import "unsafe"
+
+//go:registerparams
+func F(s []int) {
+       for i, x := range s {
+               G(i, x)
+       }
+       GC()
+       G(len(s), cap(s))
+       GC()
+}
+
+//go:noinline
+//go:registerparams
+func G(int, int) {}
+
+//go:registerparams
+func GC() { runtime.GC(); runtime.GC() }
+
+func main() {
+       s := make([]int, 3)
+       escape(s)
+       p := int(uintptr(unsafe.Pointer(&s[2])) + 42) // likely point to unallocated memory
+       poison([3]int{p, p, p})
+       F(s)
+}
+
+//go:noinline
+//go:registerparams
+func poison([3]int) {}
+
+//go:noinline
+//go:registerparams
+func escape(s []int) {
+       g = s
+}
+var g []int

diff --git a/test/abi/part_live_2.go b/test/abi/part_live_2.go
new file mode 100644 (file)
index 0000000..f9d7b01
--- /dev/null
+++ b/test/abi/part_live_2.go
@@ -0,0 +1,53 @@
+// run
+
+// Copyright 2021 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.
+
+// A test for partial liveness / partial spilling / compiler-induced GC failure
+
+package main
+
+import "runtime"
+import "unsafe"
+
+//go:registerparams
+func F(s []int) {
+       for i, x := range s {
+               G(i, x)
+       }
+       GC()
+       H(&s[0]) // It's possible that this will make the spill redundant, but there's a bug in spill slot allocation.
+       G(len(s), cap(s))
+       GC()
+}
+
+//go:noinline
+//go:registerparams
+func G(int, int) {}
+
+//go:noinline
+//go:registerparams
+func H(*int) {}
+
+//go:registerparams
+func GC() { runtime.GC(); runtime.GC() }
+
+func main() {
+       s := make([]int, 3)
+       escape(s)
+       p := int(uintptr(unsafe.Pointer(&s[2])) + 42) // likely point to unallocated memory
+       poison([3]int{p, p, p})
+       F(s)
+}
+
+//go:noinline
+//go:registerparams
+func poison([3]int) {}
+
+//go:noinline
+//go:registerparams
+func escape(s []int) {
+       g = s
+}
+var g []int