[release-branch.go1.11] reflect: use correct write barrier operations for method...

Fix the code to use write barriers on heap memory, and no
write barriers on stack memory.

These errors were discovered as part of fixing #27695. They may
have something to do with that issue, but hard to be sure.
The core cause is different, so this fix is a separate CL.

Update #27867

Change-Id: Ib005f6b3308de340be83c3d07d049d5e316b1e3c
Reviewed-on: https://go-review.googlesource.com/137438
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit e35a41261b19589f40d32bd66274c23ab4b9b32e)
Reviewed-on: https://go-review.googlesource.com/138581
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/reflect/type.go b/src/reflect/type.go
index 58cfc0e884035984d3123eb42d07c002966148cf..6b0ce431a67713cbbc9c3ad633fb4dfcb7160864 100644 (file)
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -3066,6 +3066,8 @@ func funcLayout(t *rtype, rcvr *rtype) (frametype *rtype, argSize, retOffset uin
                // space no matter how big they actually are.
                if ifaceIndir(rcvr) || rcvr.pointers() {
                        ptrmap.append(1)
+               } else {
+                       ptrmap.append(0)
                }
                offset += ptrSize
        }

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 4e7b1d74db3dc729d019987d6f99b58a4051555a..6a49adef593c585f6d8a2c1f3bb6f5d7eb1508c6 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -453,15 +453,14 @@ func (v Value) call(op string, in []Value) []Value {
 
        var ret []Value
        if nout == 0 {
-               // This is untyped because the frame is really a
-               // stack, even though it's a heap object.
-               memclrNoHeapPointers(args, frametype.size)
+               typedmemclr(frametype, args)
                framePool.Put(args)
        } else {
                // Zero the now unused input area of args,
                // because the Values returned by this function contain pointers to the args object,
                // and will thus keep the args object alive indefinitely.
-               memclrNoHeapPointers(args, retOffset)
+               typedmemclrpartial(frametype, args, 0, retOffset)
+
                // Wrap Values around return values in args.
                ret = make([]Value, nout)
                off = retOffset
@@ -472,6 +471,10 @@ func (v Value) call(op string, in []Value) []Value {
                        if tv.Size() != 0 {
                                fl := flagIndir | flag(tv.Kind())
                                ret[i] = Value{tv.common(), add(args, off, "tv.Size() != 0"), fl}
+                               // Note: this does introduce false sharing between results -
+                               // if any result is live, they are all live.
+                               // (And the space for the args is live as well, but as we've
+                               // cleared that space it isn't as big a deal.)
                        } else {
                                // For zero-sized return value, args+off may point to the next object.
                                // In this case, return the zero value instead.
@@ -660,6 +663,8 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
        }
 
        // Call.
+       // Call copies the arguments from args to the stack, calls fn,
+       // and then copies the results back into args.
        call(frametype, fn, args, uint32(frametype.size), uint32(retOffset))
 
        // Copy return values. On amd64p32, the beginning of return values
@@ -673,16 +678,14 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
                if runtime.GOARCH == "amd64p32" {
                        callerRetOffset = align(argSize-ptrSize, 8)
                }
-               typedmemmovepartial(frametype,
-                       add(frame, callerRetOffset, "frametype.size > retOffset"),
+               // This copies to the stack. Write barriers are not needed.
+               memmove(add(frame, callerRetOffset, "frametype.size > retOffset"),
                        add(args, retOffset, "frametype.size > retOffset"),
-                       retOffset,
                        frametype.size-retOffset)
        }
 
-       // This is untyped because the frame is really a stack, even
-       // though it's a heap object.
-       memclrNoHeapPointers(args, frametype.size)
+       // Put the args scratch space back in the pool.
+       typedmemclr(frametype, args)
        framePool.Put(args)
 
        // See the comment in callReflect.
@@ -2569,6 +2572,10 @@ func call(argtype *rtype, fn, arg unsafe.Pointer, n uint32, retoffset uint32)
 
 func ifaceE2I(t *rtype, src interface{}, dst unsafe.Pointer)
 
+// memmove copies size bytes to dst from src. No write barriers are used.
+//go:noescape
+func memmove(dst, src unsafe.Pointer, size uintptr)
+
 // typedmemmove copies a value of type t to dst from src.
 //go:noescape
 func typedmemmove(t *rtype, dst, src unsafe.Pointer)
@@ -2578,14 +2585,20 @@ func typedmemmove(t *rtype, dst, src unsafe.Pointer)
 //go:noescape
 func typedmemmovepartial(t *rtype, dst, src unsafe.Pointer, off, size uintptr)
 
+// typedmemclr zeros the value at ptr of type t.
+//go:noescape
+func typedmemclr(t *rtype, ptr unsafe.Pointer)
+
+// typedmemclrpartial is like typedmemclr but assumes that
+// dst points off bytes into the value and only clears size bytes.
+//go:noescape
+func typedmemclrpartial(t *rtype, ptr unsafe.Pointer, off, size uintptr)
+
 // typedslicecopy copies a slice of elemType values from src to dst,
 // returning the number of elements copied.
 //go:noescape
 func typedslicecopy(elemType *rtype, dst, src sliceHeader) int
 
-//go:noescape
-func memclrNoHeapPointers(ptr unsafe.Pointer, n uintptr)
-
 // Dummy annotation marking that the value x escapes,
 // for use in cases where the reflect code is so clever that
 // the compiler cannot follow.

diff --git a/src/runtime/mbarrier.go b/src/runtime/mbarrier.go
index b6c5ee06580612927f5aa20e87ab2027013119cd..ed47c6f0ef0b502f0ea65277b9bfbd36a57f7ff1 100644 (file)
--- a/src/runtime/mbarrier.go
+++ b/src/runtime/mbarrier.go
@@ -320,6 +320,19 @@ func typedmemclr(typ *_type, ptr unsafe.Pointer) {
        memclrNoHeapPointers(ptr, typ.size)
 }
 
+//go:linkname reflect_typedmemclr reflect.typedmemclr
+func reflect_typedmemclr(typ *_type, ptr unsafe.Pointer) {
+       typedmemclr(typ, ptr)
+}
+
+//go:linkname reflect_typedmemclrpartial reflect.typedmemclrpartial
+func reflect_typedmemclrpartial(typ *_type, ptr unsafe.Pointer, off, size uintptr) {
+       if typ.kind&kindNoPointers == 0 {
+               bulkBarrierPreWrite(uintptr(ptr), 0, size)
+       }
+       memclrNoHeapPointers(ptr, size)
+}
+
 // memclrHasPointers clears n bytes of typed memory starting at ptr.
 // The caller must ensure that the type of the object at ptr has
 // pointers, usually by checking typ.kind&kindNoPointers. However, ptr