Commit b88276da authored by Austin Clements's avatar Austin Clements

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: 's avatarRick Hudson <rlh@golang.org>
parent 5095e5d8
......@@ -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))
......
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