[release-branch.go1.17] reflect: keep pointer in aggregate-typed args live in Call

When register ABI is used, reflect.Value.Call prepares the call
arguments in a memory representation of the argument registers.
It has special handling to keep the pointers in arguments live.
Currently, this handles pointer-typed arguments. But when an
argument is an aggregate-type that contains pointers and passed
in registers, it currently doesn't keep the pointers live. Do
so in this CL.

Fixes #49961

Change-Id: I9264a8767e2a2c48573f6047144759b845dcf480
Reviewed-on: https://go-review.googlesource.com/c/go/+/369098
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/internal/abi/abi.go b/src/internal/abi/abi.go
index aaff9cece35720aef56455742b2f4556ba1f98d8..aa5083a666101a51f3ff5daa43c41728be82d5c8 100644 (file)
--- a/src/internal/abi/abi.go
+++ b/src/internal/abi/abi.go
@@ -33,6 +33,24 @@ type RegArgs struct {
        ReturnIsPtr IntArgRegBitmap
 }
 
+func (r *RegArgs) Dump() {
+       print("Ints:")
+       for _, x := range r.Ints {
+               print(" ", x)
+       }
+       println()
+       print("Floats:")
+       for _, x := range r.Floats {
+               print(" ", x)
+       }
+       println()
+       print("Ptrs:")
+       for _, x := range r.Ptrs {
+               print(" ", x)
+       }
+       println()
+}
+
 // IntArgRegBitmap is a bitmap large enough to hold one bit per
 // integer argument/return register.
 type IntArgRegBitmap [(IntArgRegs + 7) / 8]uint8

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index eac27e886f21d6c8d8325d50ad5e9c615457d929..6f350affd67d27fa676a65037bdbba373cc9d0e5 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -6270,6 +6270,29 @@ func TestCallMethodJump(t *testing.T) {
        *CallGC = false
 }
 
+func TestCallArgLive(t *testing.T) {
+       type T struct{ X, Y *string } // pointerful aggregate
+
+       F := func(t T) { *t.X = "ok" }
+
+       // In reflect.Value.Call, trigger a garbage collection in reflect.call
+       // between marshaling argument and the actual call.
+       *CallGC = true
+
+       x := new(string)
+       runtime.SetFinalizer(x, func(p *string) {
+               if *p != "ok" {
+                       t.Errorf("x dead prematurely")
+               }
+       })
+       v := T{x, nil}
+
+       ValueOf(F).Call([]Value{ValueOf(v)})
+
+       // Stop garbage collecting during reflect.call.
+       *CallGC = false
+}
+
 func TestMakeFuncStackCopy(t *testing.T) {
        target := func(in []Value) []Value {
                runtime.GC()

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 6f878eba5b0411af35e44d27ea6a64a2205248e7..520dc69137e868a3f7c2c439bb60ad7471dac1fb 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -352,7 +352,7 @@ func (v Value) CallSlice(in []Value) []Value {
        return v.call("CallSlice", in)
 }
 
-var callGC bool // for testing; see TestCallMethodJump
+var callGC bool // for testing; see TestCallMethodJump and TestCallArgLive
 
 const debugReflectCall = false
 
@@ -509,12 +509,16 @@ func (v Value) call(op string, in []Value) []Value {
                                // Copy values to "integer registers."
                                if v.flag&flagIndir != 0 {
                                        offset := add(v.ptr, st.offset, "precomputed value offset")
-                                       memmove(unsafe.Pointer(&regArgs.Ints[st.ireg]), offset, st.size)
-                               } else {
                                        if st.kind == abiStepPointer {
                                                // Duplicate this pointer in the pointer area of the
                                                // register space. Otherwise, there's the potential for
                                                // this to be the last reference to v.ptr.
+                                               regArgs.Ptrs[st.ireg] = *(*unsafe.Pointer)(offset)
+                                       }
+                                       memmove(unsafe.Pointer(&regArgs.Ints[st.ireg]), offset, st.size)
+                               } else {
+                                       if st.kind == abiStepPointer {
+                                               // See the comment in abiStepPointer case above.
                                                regArgs.Ptrs[st.ireg] = v.ptr
                                        }
                                        regArgs.Ints[st.ireg] = uintptr(v.ptr)
@@ -539,6 +543,15 @@ func (v Value) call(op string, in []Value) []Value {
        // Mark pointers in registers for the return path.
        regArgs.ReturnIsPtr = abi.outRegPtrs
 
+       if debugReflectCall {
+               regArgs.Dump()
+       }
+
+       // For testing; see TestCallArgLive.
+       if callGC {
+               runtime.GC()
+       }
+
        // Call.
        call(frametype, fn, stackArgs, uint32(frametype.size), uint32(abi.retOffset), uint32(frameSize), &regArgs)