runtime: fix heap bitmap repeating with large scalar tails

When heapBitsSetType repeats a source bitmap with a scalar tail
(typ.ptrdata < typ.size), it lays out the tail upon reaching the end
of the source bitmap by simply increasing the number of bits claimed
to be in the incoming bit buffer. This causes later iterations to read
the appropriate number of zeros out of the bit buffer before starting
on the next repeat of the source bitmap.

Currently, however, later iterations of the loop continue to read bits
from the source bitmap *regardless of the number of bits currently in
the bit buffer*. The bit buffer can only hold 32 or 64 bits, so if the
scalar tail is large and the padding bits exceed the size of the bit
buffer, the read from the source bitmap on the next iteration will
shift the incoming bits into oblivion when it attempts to put them in
the bit buffer. When the buffer does eventually shift down to where
these bits were supposed to be, it will contain zeros. As a result,
words that should be marked as pointers on later repetitions are
marked as scalars, so the garbage collector does not trace them. If
this is the only reference to an object, it will be incorrectly freed.

Fix this by adding logic to drain the bit buffer down if it is large
instead of reading more bits from the source bitmap.

Fixes #11286.

Change-Id: I964432c4b9f1cec334fc8c3da0ff16460203feb6
Reviewed-on: https://go-review.googlesource.com/11360
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index 4d39c321d8256b9b1fc54edaa92fe0528d262a68..efdcb8fca41fa8538549511f669a960c258fa8f8 100644 (file)
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -978,8 +978,16 @@ func heapBitsSetType(x, size, dataSize uintptr, typ *_type) {
                        // nb unmodified: we just loaded 8 bits,
                        // and the next iteration will consume 8 bits,
                        // leaving us with the same nb the next time we're here.
-                       b |= uintptr(*p) << nb
-                       p = add1(p)
+                       if nb < 8 {
+                               b |= uintptr(*p) << nb
+                               p = add1(p)
+                       } else {
+                               // Reduce the number of bits in b.
+                               // This is important if we skipped
+                               // over a scalar tail, since nb could
+                               // be larger than the bit width of b.
+                               nb -= 8
+                       }
                } else if p == nil {
                        // Almost as fast path: track bit count and refill from pbits.
                        // For short repetitions.

diff --git a/test/fixedbugs/issue11286.go b/test/fixedbugs/issue11286.go
new file mode 100644 (file)
index 0000000..560b7d4
--- /dev/null
+++ b/test/fixedbugs/issue11286.go
@@ -0,0 +1,34 @@
+// run
+
+// Copyright 2015 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test that pointer bitmaps of types with large scalar tails are
+// correctly repeated when unrolled into the heap bitmap.
+
+package main
+
+import "runtime"
+
+const D = 57
+
+type T struct {
+       a [D]float64
+       b map[string]int
+       c [D]float64
+}
+
+var ts []T
+
+func main() {
+       ts = make([]T, 4)
+       for i := range ts {
+               ts[i].b = make(map[string]int)
+       }
+       ts[3].b["abc"] = 42
+       runtime.GC()
+       if ts[3].b["abc"] != 42 {
+               panic("bad field value")
+       }
+}