reflect: handle stack-to-register translation in callMethod

callMethod previously assumed erroneously that between the "value" and
"method" ABIs (that is, the ABI the caller is following to call this
method value and the actual ABI of the method), it could never happen
that an argument passed on the stack in the former could be passed in
registers in the latter. The cited reason was that the latter always
uses strictly more registers.

However, there are situations where the value ABI could pass a value on
the stack, but later is passed in a register. For instance, if the
receiver pushes a value passed in registers that uses multiple registers
to be passed on the stack, later arguments which were passed on the
stack may now be passed in registers.

This change fixes callMethod to no longer makes this assumption, and
handles the stack-to-register translation explicitly.

Fixes #46696.

Change-Id: I7100a664d97bbe401302cc893b3a98b28cdcdfc0
Reviewed-on: https://go-review.googlesource.com/c/go/+/327089
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/reflect/abi_test.go b/src/reflect/abi_test.go
index 1a2a48b5ed92d876af7a657965a8c5d867a732f0..5a0130f7b452b988bae5cb1eea552c635236633d 100644 (file)
--- a/src/reflect/abi_test.go
+++ b/src/reflect/abi_test.go
@@ -79,7 +79,34 @@ func TestMethodValueCallABI(t *testing.T) {
                t.Errorf("bad method value call: got %#v, want %#v", r2, a2)
        }
        if s.Value != 3 {
-               t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 1)
+               t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 3)
+       }
+
+       s, i = makeMethodValue("ValueRegMethodSpillInt")
+       f3 := i.(func(StructFillRegs, int, MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, int))
+       r3a, r3b := f3(a2, 42, MagicLastTypeNameForTestingRegisterABI{})
+       if r3a != a2 {
+               t.Errorf("bad method value call: got %#v, want %#v", r3a, a2)
+       }
+       if r3b != 42 {
+               t.Errorf("bad method value call: got %#v, want %#v", r3b, 42)
+       }
+       if s.Value != 4 {
+               t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 4)
+       }
+
+       s, i = makeMethodValue("ValueRegMethodSpillPtr")
+       f4 := i.(func(StructFillRegs, *byte, MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, *byte))
+       vb := byte(10)
+       r4a, r4b := f4(a2, &vb, MagicLastTypeNameForTestingRegisterABI{})
+       if r4a != a2 {
+               t.Errorf("bad method value call: got %#v, want %#v", r4a, a2)
+       }
+       if r4b != &vb {
+               t.Errorf("bad method value call: got %#v, want %#v", r4b, &vb)
+       }
+       if s.Value != 5 {
+               t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 5)
        }
 }
 
@@ -112,6 +139,20 @@ func (m *StructWithMethods) SpillStructCall(s StructFillRegs, _ MagicLastTypeNam
        return s
 }
 
+// When called as a method value, i is passed on the stack.
+// When called as a method, i is passed in a register.
+func (m *StructWithMethods) ValueRegMethodSpillInt(s StructFillRegs, i int, _ MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, int) {
+       m.Value = 4
+       return s, i
+}
+
+// When called as a method value, i is passed on the stack.
+// When called as a method, i is passed in a register.
+func (m *StructWithMethods) ValueRegMethodSpillPtr(s StructFillRegs, i *byte, _ MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, *byte) {
+       m.Value = 5
+       return s, i
+}
+
 func TestReflectCallABI(t *testing.T) {
        // Enable register-based reflect.Call and ensure we don't
        // use potentially incorrect cached versions by clearing

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 418dff781f4ecc8635cb7acab44146a31758be19..c963a407bcc80cce6c8b324a30f212be5b4d6d22 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -952,25 +952,47 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer, retValid *bool, regs *a
                        continue
                }
 
-               // There are three cases to handle in translating each
+               // There are four cases to handle in translating each
                // argument:
                // 1. Stack -> stack translation.
-               // 2. Registers -> stack translation.
-               // 3. Registers -> registers translation.
-               // The fourth cases can't happen, because a method value
-               // call uses strictly fewer registers than a method call.
+               // 2. Stack -> registers translation.
+               // 3. Registers -> stack translation.
+               // 4. Registers -> registers translation.
+               // TODO(mknyszek): Cases 2 and 3 below only work on little endian
+               // architectures. This is OK for now, but this needs to be fixed
+               // before supporting the register ABI on big endian architectures.
 
                // If the value ABI passes the value on the stack,
                // then the method ABI does too, because it has strictly
                // fewer arguments. Simply copy between the two.
                if vStep := valueSteps[0]; vStep.kind == abiStepStack {
                        mStep := methodSteps[0]
-                       if mStep.kind != abiStepStack || vStep.size != mStep.size {
-                               panic("method ABI and value ABI do not align")
+                       // Handle stack -> stack translation.
+                       if mStep.kind == abiStepStack {
+                               if vStep.size != mStep.size {
+                                       panic("method ABI and value ABI do not align")
+                               }
+                               typedmemmove(t,
+                                       add(methodFrame, mStep.stkOff, "precomputed stack offset"),
+                                       add(valueFrame, vStep.stkOff, "precomputed stack offset"))
+                               continue
+                       }
+                       // Handle stack -> register translation.
+                       for _, mStep := range methodSteps {
+                               from := add(valueFrame, vStep.stkOff+mStep.offset, "precomputed stack offset")
+                               switch mStep.kind {
+                               case abiStepPointer:
+                                       // Do the pointer copy directly so we get a write barrier.
+                                       methodRegs.Ptrs[mStep.ireg] = *(*unsafe.Pointer)(from)
+                                       fallthrough // We need to make sure this ends up in Ints, too.
+                               case abiStepIntReg:
+                                       memmove(unsafe.Pointer(&methodRegs.Ints[mStep.ireg]), from, mStep.size)
+                               case abiStepFloatReg:
+                                       memmove(unsafe.Pointer(&methodRegs.Floats[mStep.freg]), from, mStep.size)
+                               default:
+                                       panic("unexpected method step")
+                               }
                        }
-                       typedmemmove(t,
-                               add(methodFrame, mStep.stkOff, "precomputed stack offset"),
-                               add(valueFrame, vStep.stkOff, "precomputed stack offset"))
                        continue
                }
                // Handle register -> stack translation.