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>

diff --git a/src/runtime/syscall_windows.go b/src/runtime/syscall_windows.go
index 6b9195bcd5253394fd988a9bc0ecce6d832f9528..4763a440e7ef1c47ba322727dfd7bee3bcbb482f 100644 (file)
--- a/src/runtime/syscall_windows.go
+++ b/src/runtime/syscall_windows.go
@@ -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.
        //

diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go
index 5e9694d444c2d12eb443bb3b4aa32047739703d2..e3f772ac4bb9457a5ed94ff944cfc466b67afca2 100644 (file)
--- a/src/runtime/syscall_windows_test.go
+++ b/src/runtime/syscall_windows_test.go
@@ -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 {