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 <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>

Author: mknyszek

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

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.