diff options
| author | Cherry Mui <cherryyz@google.com> | 2022-03-03 13:35:44 -0500 |
|---|---|---|
| committer | Heschi Kreinick <heschi@google.com> | 2022-03-14 17:45:32 +0000 |
| commit | 88be85f18bf0244a2470fdf6719e1b5ca5a5e50a (patch) | |
| tree | b3434de78c0c013cf13af648c2cea64eae0d6d37 | |
| parent | 7dd10d4ce20e64d96a10cb67794851a58d96a2aa (diff) | |
| download | go-88be85f18bf0244a2470fdf6719e1b5ca5a5e50a.tar.gz go-88be85f18bf0244a2470fdf6719e1b5ca5a5e50a.zip | |
[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 <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 58804ea67a28c1d8e37ed548b685bc0c09638886)
Reviewed-on: https://go-review.googlesource.com/c/go/+/389794
| -rw-r--r-- | src/runtime/mfinal.go | 24 | ||||
| -rw-r--r-- | src/runtime/mfinal_test.go | 9 |
2 files changed, 18 insertions, 15 deletions
diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go index c134a0f22d..a6653032d7 100644 --- a/src/runtime/mfinal.go +++ b/src/runtime/mfinal.go @@ -187,21 +187,15 @@ func runfinq() { f := &fb.fin[i-1] var regs abi.RegArgs - var framesz uintptr - if argRegs > 0 { - // The args can always be passed in registers if they're - // available, because platforms we support always have no - // argument registers available, or more than 2. - // - // But unfortunately because we can have an arbitrary - // amount of returns and it would be complex to try and - // figure out how many of those can get passed in registers, - // just conservatively assume none of them do. - framesz = f.nret - } else { - // Need to pass arguments on the stack too. - framesz = unsafe.Sizeof((interface{})(nil)) + f.nret - } + // The args may be passed in registers or on stack. Even for + // the register case, we still need the spill slots. + // TODO: revisit if we remove spill slots. + // + // Unfortunately because we can have an arbitrary + // amount of returns and it would be complex to try and + // figure out how many of those can get passed in registers, + // just conservatively assume none of them do. + framesz := unsafe.Sizeof((interface{})(nil)) + f.nret if framecap < framesz { // The frame does not contain pointers interesting for GC, // all not yet finalized objects are stored in finq. diff --git a/src/runtime/mfinal_test.go b/src/runtime/mfinal_test.go index 3ca8d31c60..8827d55af1 100644 --- a/src/runtime/mfinal_test.go +++ b/src/runtime/mfinal_test.go @@ -42,6 +42,15 @@ func TestFinalizerType(t *testing.T) { {func(x *int) interface{} { return Tintptr(x) }, func(v *int) { finalize(v) }}, {func(x *int) interface{} { return (*Tint)(x) }, func(v *Tint) { finalize((*int)(v)) }}, {func(x *int) interface{} { return (*Tint)(x) }, func(v Tinter) { finalize((*int)(v.(*Tint))) }}, + // Test case for argument spill slot. + // If the spill slot was not counted for the frame size, it will (incorrectly) choose + // call32 as the result has (exactly) 32 bytes. When the argument actually spills, + // it clobbers the caller's frame (likely the return PC). + {func(x *int) interface{} { return x }, func(v interface{}) [4]int64 { + print() // force spill + finalize(v.(*int)) + return [4]int64{} + }}, } for i, tt := range finalizerTests { |