runtime: fix bitmap copying corner-cases

When an object spans heap arenas, its bitmap is discontiguous, so
heapBitsSetType unrolls the bitmap into the object itself and then
copies it out to the real heap bitmap. Unfortunately, since this code
path is rare, it had two unnoticed bugs related to the head and tail
of the bitmap:

1. At the head of the object, we were using hbitp as the destination
bitmap pointer rather than h.bitp, but hbitp points into the
*temporary* bitmap space (that is, the object itself), so we were
failing to copy the partial bitmap byte at the head of an object.

2. The core copying loop copied all of the full bitmap bytes, but
always drove the remaining word count down to 0, even if there was a
partial bitmap byte for the tail of the object. As a result, we never
wrote partial bitmap bytes at the tail of an object.

I found these by enabling out-of-place unrolling all the time. To
improve our chances of detecting these sorts of bugs in the future,
this CL mimics this by enabling out-of-place mode 50% of the time when
doubleCheck is enabled so that we test both in-place and out-of-place
mode.

Change-Id: I69e5d829fb3444be4cf11f4c6d8462c26dc467e8
Reviewed-on: https://go-review.googlesource.com/110995
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index 513e6b9eed6bdc731a6d291e6b2978dcfad4eb27..75f23a16b413d256debe5e7e8d5bd71771399486 100644 (file)
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -992,10 +992,13 @@ func heapBitsSetType(x, size, dataSize uintptr, typ *_type) {
        // machine instructions.
 
        outOfPlace := false
-       if arenaIndex(x+size-1) != arenaIdx(h.arena) {
+       if arenaIndex(x+size-1) != arenaIdx(h.arena) || (doubleCheck && fastrand()%2 == 0) {
                // This object spans heap arenas, so the bitmap may be
                // discontiguous. Unroll it into the object instead
                // and then copy it out.
+               //
+               // In doubleCheck mode, we randomly do this anyway to
+               // stress test the bitmap copying path.
                outOfPlace = true
                h.bitp = (*uint8)(unsafe.Pointer(x))
                h.last = nil
@@ -1352,7 +1355,7 @@ Phase4:
                        }
                }
                if sys.PtrSize == 8 && h.shift == 2 {
-                       *hbitp = *hbitp&^((bitPointer|bitScan|(bitPointer|bitScan)<<heapBitsShift)<<(2*heapBitsShift)) | *src
+                       *h.bitp = *h.bitp&^((bitPointer|bitScan|(bitPointer|bitScan)<<heapBitsShift)<<(2*heapBitsShift)) | *src
                        h = h.next().next()
                        cnw -= 2
                        src = addb(src, 1)
@@ -1361,7 +1364,9 @@ Phase4:
                // bitmaps until the last byte (which may again be
                // partial).
                for cnw >= 4 {
-                       hNext, words := h.forwardOrBoundary(cnw)
+                       // This loop processes four words at a time,
+                       // so round cnw down accordingly.
+                       hNext, words := h.forwardOrBoundary(cnw / 4 * 4)
 
                        // n is the number of bitmap bytes to copy.
                        n := words / 4
@@ -1370,6 +1375,10 @@ Phase4:
                        h = hNext
                        src = addb(src, n)
                }
+               if doubleCheck && h.shift != 0 {
+                       print("cnw=", cnw, " h.shift=", h.shift, "\n")
+                       throw("bad shift after block copy")
+               }
                // Handle the last byte if it's shared.
                if cnw == 2 {
                        *h.bitp = *h.bitp&^(bitPointer|bitScan|(bitPointer|bitScan)<<heapBitsShift) | *src
@@ -1451,7 +1460,7 @@ Phase4:
                                print("initial bits h0.bitp=", h0.bitp, " h0.shift=", h0.shift, "\n")
                                print("current bits h.bitp=", h.bitp, " h.shift=", h.shift, " *h.bitp=", hex(*h.bitp), "\n")
                                print("ptrmask=", ptrmask, " p=", p, " endp=", endp, " endnb=", endnb, " pbits=", hex(pbits), " b=", hex(b), " nb=", nb, "\n")
-                               println("at word", i, "offset", i*sys.PtrSize, "have", have, "want", want)
+                               println("at word", i, "offset", i*sys.PtrSize, "have", hex(have), "want", hex(want))
                                if typ.kind&kindGCProg != 0 {
                                        println("GC program:")
                                        dumpGCProg(addb(typ.gcdata, 4))