Commit 5f45a333 authored by Cherry Zhang's avatar Cherry Zhang

reflect: align first argument in callMethod

When calling a function obtained from reflect.Value.Method (or
MethodByName), we copy the arguments from the caller frame, which
does not include the receiver, to a new frame to call the actual
method, which does include the receiver. Here we need to align
the first (non-receiver) argument. As the receiver is pointer
sized, it is generally naturally aligned, except on amd64p32,
where the argument can have larger alignment, and this aligning
becomes necessary.

Fixes #33628.

Change-Id: I5bea0e20173f06d1602c5666d4f334e3d0de5c1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190297
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarKeith Randall <khr@golang.org>
parent ad4ed87f
...@@ -2057,6 +2057,16 @@ func (p Point) TotalDist(points ...Point) int { ...@@ -2057,6 +2057,16 @@ func (p Point) TotalDist(points ...Point) int {
return tot return tot
} }
// This will be index 5.
func (p *Point) Int64Method(x int64) int64 {
return x
}
// This will be index 6.
func (p *Point) Int32Method(x int32) int32 {
return x
}
func TestMethod(t *testing.T) { func TestMethod(t *testing.T) {
// Non-curried method of type. // Non-curried method of type.
p := Point{3, 4} p := Point{3, 4}
...@@ -2265,6 +2275,17 @@ func TestMethodValue(t *testing.T) { ...@@ -2265,6 +2275,17 @@ func TestMethodValue(t *testing.T) {
if i != 425 { if i != 425 {
t.Errorf("Interface MethodByName returned %d; want 425", i) t.Errorf("Interface MethodByName returned %d; want 425", i)
} }
// For issue #33628: method args are not stored at the right offset
// on amd64p32.
m64 := ValueOf(&p).MethodByName("Int64Method").Interface().(func(int64) int64)
if x := m64(123); x != 123 {
t.Errorf("Int64Method returned %d; want 123", x)
}
m32 := ValueOf(&p).MethodByName("Int32Method").Interface().(func(int32) int32)
if x := m32(456); x != 456 {
t.Errorf("Int32Method returned %d; want 456", x)
}
} }
func TestVariadicMethodValue(t *testing.T) { func TestVariadicMethodValue(t *testing.T) {
......
...@@ -696,10 +696,16 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer, retValid *bool) { ...@@ -696,10 +696,16 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer, retValid *bool) {
scratch := framePool.Get().(unsafe.Pointer) scratch := framePool.Get().(unsafe.Pointer)
// Copy in receiver and rest of args. // Copy in receiver and rest of args.
// Avoid constructing out-of-bounds pointers if there are no args.
storeRcvr(rcvr, scratch) storeRcvr(rcvr, scratch)
if argSize-ptrSize > 0 { // Align the first arg. Only on amd64p32 the alignment can be
typedmemmovepartial(frametype, add(scratch, ptrSize, "argSize > ptrSize"), frame, ptrSize, argSize-ptrSize) // larger than ptrSize.
argOffset := uintptr(ptrSize)
if len(t.in()) > 0 {
argOffset = align(argOffset, uintptr(t.in()[0].align))
}
// Avoid constructing out-of-bounds pointers if there are no args.
if argSize-argOffset > 0 {
typedmemmovepartial(frametype, add(scratch, argOffset, "argSize > argOffset"), frame, argOffset, argSize-argOffset)
} }
// Call. // Call.
...@@ -714,9 +720,9 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer, retValid *bool) { ...@@ -714,9 +720,9 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer, retValid *bool) {
// Ignore any changes to args and just copy return values. // Ignore any changes to args and just copy return values.
// Avoid constructing out-of-bounds pointers if there are no return values. // Avoid constructing out-of-bounds pointers if there are no return values.
if frametype.size-retOffset > 0 { if frametype.size-retOffset > 0 {
callerRetOffset := retOffset - ptrSize callerRetOffset := retOffset - argOffset
if runtime.GOARCH == "amd64p32" { if runtime.GOARCH == "amd64p32" {
callerRetOffset = align(argSize-ptrSize, 8) callerRetOffset = align(argSize-argOffset, 8)
} }
// This copies to the stack. Write barriers are not needed. // This copies to the stack. Write barriers are not needed.
memmove(add(frame, callerRetOffset, "frametype.size > retOffset"), memmove(add(frame, callerRetOffset, "frametype.size > retOffset"),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment