Skip to content

Commit 88be85f

Browse files
cherrymuiheschi
authored andcommitted
[release-branch.go1.17] runtime: count spill slot for frame size at finalizer call
The finalizer is called using reflectcall. When register ABI is used, the finalizer's argument is passed in register(s). But the frame size calculation does not include the spill slot. When the argument actually spills, it may clobber the caller's stack frame. This CL fixes it. Updates #51457. Fixes #51458. Change-Id: Ibcc7507c518ba65c1c5a7759e5cab0ae3fc7efce Reviewed-on: https://go-review.googlesource.com/c/go/+/389574 Trust: Cherry Mui <[email protected]> Run-TryBot: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> (cherry picked from commit 58804ea) Reviewed-on: https://go-review.googlesource.com/c/go/+/389794
1 parent 7dd10d4 commit 88be85f

File tree

2 files changed

+18
-15
lines changed

2 files changed

+18
-15
lines changed

src/runtime/mfinal.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -187,21 +187,15 @@ func runfinq() {
187187
f := &fb.fin[i-1]
188188

189189
var regs abi.RegArgs
190-
var framesz uintptr
191-
if argRegs > 0 {
192-
// The args can always be passed in registers if they're
193-
// available, because platforms we support always have no
194-
// argument registers available, or more than 2.
195-
//
196-
// But unfortunately because we can have an arbitrary
197-
// amount of returns and it would be complex to try and
198-
// figure out how many of those can get passed in registers,
199-
// just conservatively assume none of them do.
200-
framesz = f.nret
201-
} else {
202-
// Need to pass arguments on the stack too.
203-
framesz = unsafe.Sizeof((interface{})(nil)) + f.nret
204-
}
190+
// The args may be passed in registers or on stack. Even for
191+
// the register case, we still need the spill slots.
192+
// TODO: revisit if we remove spill slots.
193+
//
194+
// Unfortunately because we can have an arbitrary
195+
// amount of returns and it would be complex to try and
196+
// figure out how many of those can get passed in registers,
197+
// just conservatively assume none of them do.
198+
framesz := unsafe.Sizeof((interface{})(nil)) + f.nret
205199
if framecap < framesz {
206200
// The frame does not contain pointers interesting for GC,
207201
// all not yet finalized objects are stored in finq.

src/runtime/mfinal_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ func TestFinalizerType(t *testing.T) {
4242
{func(x *int) interface{} { return Tintptr(x) }, func(v *int) { finalize(v) }},
4343
{func(x *int) interface{} { return (*Tint)(x) }, func(v *Tint) { finalize((*int)(v)) }},
4444
{func(x *int) interface{} { return (*Tint)(x) }, func(v Tinter) { finalize((*int)(v.(*Tint))) }},
45+
// Test case for argument spill slot.
46+
// If the spill slot was not counted for the frame size, it will (incorrectly) choose
47+
// call32 as the result has (exactly) 32 bytes. When the argument actually spills,
48+
// it clobbers the caller's frame (likely the return PC).
49+
{func(x *int) interface{} { return x }, func(v interface{}) [4]int64 {
50+
print() // force spill
51+
finalize(v.(*int))
52+
return [4]int64{}
53+
}},
4554
}
4655

4756
for i, tt := range finalizerTests {

0 commit comments

Comments
 (0)