From a8ae93fd26063c62645d9d75b78ebf0f7dc8f391 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Tue, 23 Jun 2015 11:35:21 -0400 Subject: [PATCH] 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 --- src/runtime/mbitmap.go | 12 ++++++++++-- test/fixedbugs/issue11286.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue11286.go diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go index 4d39c321d8..efdcb8fca4 100644 --- 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 index 0000000000..560b7d4794 --- /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") + } +}