runtime: account for spill slots in Windows callback compilation

The Go ABI, as it stands, requires spill space to be reserved for
register arguments. syscall.NewCallback (because of compileCallback)
does not actually reserve this space, leading to issues if the Go code
it invokes actually makes use of it.

Fixes #46301.

Change-Id: Idbc3578accaaaa29e4ba32291ef08d464da0b7b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/322029
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Egon Elbre <egonelbre@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This commit is contained in:
Michael Anthony Knyszek 2021-05-21 20:30:02 +00:00 committed by Michael Knyszek
parent 52d7033ff6
commit 4356e7e85f
2 changed files with 35 additions and 3 deletions

View file

@ -64,6 +64,7 @@ type abiDesc struct {
srcStackSize uintptr // stdcall/fastcall stack space tracking
dstStackSize uintptr // Go stack space used
dstSpill uintptr // Extra stack space for argument spill slots
dstRegisters int // Go ABI int argument registers used
// retOffset is the offset of the uintptr-sized result in the Go
@ -110,7 +111,14 @@ func (p *abiDesc) assignArg(t *_type) {
// arguments. The same is true on arm.
oldParts := p.parts
if !p.tryRegAssignArg(t, 0) {
if p.tryRegAssignArg(t, 0) {
// Account for spill space.
//
// TODO(mknyszek): Remove this when we no longer have
// caller reserved spill space.
p.dstSpill = alignUp(p.dstSpill, uintptr(t.align))
p.dstSpill += t.size
} else {
// Register assignment failed.
// Undo the work and stack assign.
p.parts = oldParts
@ -277,7 +285,11 @@ func compileCallback(fn eface, cdecl bool) (code uintptr) {
abiMap.dstStackSize += sys.PtrSize
}
if abiMap.dstStackSize > callbackMaxFrame {
// TODO(mknyszek): Remove dstSpill from this calculation when we no longer have
// caller reserved spill space.
frameSize := alignUp(abiMap.dstStackSize, sys.PtrSize)
frameSize += abiMap.dstSpill
if frameSize > callbackMaxFrame {
panic("compileCallback: function argument frame too large")
}
@ -356,9 +368,14 @@ func callbackWrap(a *callbackArgs) {
}
}
// TODO(mknyszek): Remove this when we no longer have
// caller reserved spill space.
frameSize := alignUp(c.abiMap.dstStackSize, sys.PtrSize)
frameSize += c.abiMap.dstSpill
// Even though this is copying back results, we can pass a nil
// type because those results must not require write barriers.
reflectcall(nil, unsafe.Pointer(c.fn), noescape(goArgs), uint32(c.abiMap.dstStackSize), uint32(c.abiMap.retOffset), uint32(c.abiMap.dstStackSize), &regs)
reflectcall(nil, unsafe.Pointer(c.fn), noescape(goArgs), uint32(c.abiMap.dstStackSize), uint32(c.abiMap.retOffset), uint32(frameSize), &regs)
// Extract the result.
//

View file

@ -389,6 +389,10 @@ var cbFuncs = []cbFunc{
{func(i1, i2, i3, i4, i5 uint8Pair) uintptr {
return uintptr(i1.x + i1.y + i2.x + i2.y + i3.x + i3.y + i4.x + i4.y + i5.x + i5.y)
}},
{func(i1, i2, i3, i4, i5, i6, i7, i8, i9 uint32) uintptr {
runtime.GC()
return uintptr(i1 + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9)
}},
}
//go:registerparams
@ -461,6 +465,16 @@ func sum5andPair(i1, i2, i3, i4, i5 uint8Pair) uintptr {
return uintptr(i1.x + i1.y + i2.x + i2.y + i3.x + i3.y + i4.x + i4.y + i5.x + i5.y)
}
// This test forces a GC. The idea is to have enough arguments
// that insufficient spill slots allocated (according to the ABI)
// may cause compiler-generated spills to clobber the return PC.
// Then, the GC stack scanning will catch that.
//go:registerparams
func sum9andGC(i1, i2, i3, i4, i5, i6, i7, i8, i9 uint32) uintptr {
runtime.GC()
return uintptr(i1 + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9)
}
// TODO(register args): Remove this once we switch to using the register
// calling convention by default, since this is redundant with the existing
// tests.
@ -479,6 +493,7 @@ var cbFuncsRegABI = []cbFunc{
{sum9int8},
{sum5mix},
{sum5andPair},
{sum9andGC},
}
func getCallbackTestFuncs() []cbFunc {