mirror of
https://github.com/golang/go
synced 2024-11-22 15:44:53 -07:00
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>
This commit is contained in:
parent
8d77e45064
commit
8462169b5a
@ -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)
|
||||
|
48
test/abi/part_live.go
Normal file
48
test/abi/part_live.go
Normal file
@ -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
|
53
test/abi/part_live_2.go
Normal file
53
test/abi/part_live_2.go
Normal file
@ -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
|
Loading…
Reference in New Issue
Block a user