runtime: test and fix heap bitmap for 1-pointer allocation on 32-bit system

Change-Id: Ic064fe7c6bd3304dcc8c3f7b3b5393870b5387c2
Reviewed-on: https://go-review.googlesource.com/10119
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 2f8df78e13257ac93456cfb72aa04c4c265330d5..3fddcc868f2a0ba9c5548a7eab10c6a6cd53c2b4 100644 (file)
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -150,3 +150,5 @@ func BenchSetType(n int, x interface{}) {
                }
        })
 }
+
+const PtrSize = ptrSize

diff --git a/src/runtime/gcinfo_test.go b/src/runtime/gcinfo_test.go
index 7618d86a45607685a2b25b34be6fa4a65484fdb1..f330bf2430ef6cf1ab7e41c7b9dad34153371d2c 100644 (file)
--- a/src/runtime/gcinfo_test.go
+++ b/src/runtime/gcinfo_test.go
@@ -13,11 +13,11 @@ import (
 const (
        typeScalar  = 0
        typePointer = 1
-       typeDead    = 255
 )
 
 // TestGCInfo tests that various objects in heap, data and bss receive correct GC pointer type info.
 func TestGCInfo(t *testing.T) {
+       verifyGCInfo(t, "bss Ptr", &bssPtr, infoPtr)
        verifyGCInfo(t, "bss ScalarPtr", &bssScalarPtr, infoScalarPtr)
        verifyGCInfo(t, "bss PtrScalar", &bssPtrScalar, infoPtrScalar)
        verifyGCInfo(t, "bss BigStruct", &bssBigStruct, infoBigStruct())
@@ -26,6 +26,7 @@ func TestGCInfo(t *testing.T) {
        verifyGCInfo(t, "bss eface", &bssEface, infoEface)
        verifyGCInfo(t, "bss iface", &bssIface, infoIface)
 
+       verifyGCInfo(t, "data Ptr", &dataPtr, infoPtr)
        verifyGCInfo(t, "data ScalarPtr", &dataScalarPtr, infoScalarPtr)
        verifyGCInfo(t, "data PtrScalar", &dataPtrScalar, infoPtrScalar)
        verifyGCInfo(t, "data BigStruct", &dataBigStruct, infoBigStruct())
@@ -34,6 +35,7 @@ func TestGCInfo(t *testing.T) {
        verifyGCInfo(t, "data eface", &dataEface, infoEface)
        verifyGCInfo(t, "data iface", &dataIface, infoIface)
 
+       verifyGCInfo(t, "stack Ptr", new(Ptr), infoPtr)
        verifyGCInfo(t, "stack ScalarPtr", new(ScalarPtr), infoScalarPtr)
        verifyGCInfo(t, "stack PtrScalar", new(PtrScalar), infoPtrScalar)
        verifyGCInfo(t, "stack BigStruct", new(BigStruct), infoBigStruct())
@@ -43,6 +45,7 @@ func TestGCInfo(t *testing.T) {
        verifyGCInfo(t, "stack iface", new(Iface), infoIface)
 
        for i := 0; i < 10; i++ {
+               verifyGCInfo(t, "heap Ptr", escape(new(Ptr)), trimDead(padDead(infoPtr)))
                verifyGCInfo(t, "heap PtrSlice", escape(&make([]*byte, 10)[0]), trimDead(infoPtr10))
                verifyGCInfo(t, "heap ScalarPtr", escape(new(ScalarPtr)), trimDead(infoScalarPtr))
                verifyGCInfo(t, "heap ScalarPtrSlice", escape(&make([]ScalarPtr, 4)[0]), trimDead(infoScalarPtr4))
@@ -52,21 +55,28 @@ func TestGCInfo(t *testing.T) {
                verifyGCInfo(t, "heap eface", escape(new(interface{})), trimDead(infoEface))
                verifyGCInfo(t, "heap iface", escape(new(Iface)), trimDead(infoIface))
        }
-
 }
 
 func verifyGCInfo(t *testing.T, name string, p interface{}, mask0 []byte) {
        mask := runtime.GCMask(p)
-       if len(mask) > len(mask0) {
-               mask0 = append(mask0, typeDead)
-               mask = mask[:len(mask0)]
-       }
        if bytes.Compare(mask, mask0) != 0 {
                t.Errorf("bad GC program for %v:\nwant %+v\ngot  %+v", name, mask0, mask)
                return
        }
 }
 
+func padDead(mask []byte) []byte {
+       // Because the dead bit isn't encoded until the third word,
+       // and because on 32-bit systems a one-word allocation
+       // uses a two-word block, the pointer info for a one-word
+       // object needs to be expanded to include an extra scalar
+       // on 32-bit systems to match the heap bitmap.
+       if runtime.PtrSize == 4 && len(mask) == 1 {
+               return []byte{mask[0], 0}
+       }
+       return mask
+}
+
 func trimDead(mask []byte) []byte {
        for len(mask) > 2 && mask[len(mask)-1] == typeScalar {
                mask = mask[:len(mask)-1]
@@ -81,6 +91,12 @@ func escape(p interface{}) interface{} {
        return p
 }
 
+var infoPtr = []byte{typePointer}
+
+type Ptr struct {
+       *byte
+}
+
 var infoPtr10 = []byte{typePointer, typePointer, typePointer, typePointer, typePointer, typePointer, typePointer, typePointer, typePointer, typePointer}
 
 type ScalarPtr struct {
@@ -160,6 +176,7 @@ func (IfaceImpl) f() {
 
 var (
        // BSS
+       bssPtr       Ptr
        bssScalarPtr ScalarPtr
        bssPtrScalar PtrScalar
        bssBigStruct BigStruct
@@ -169,6 +186,7 @@ var (
        bssIface     Iface
 
        // DATA
+       dataPtr                   = Ptr{new(byte)}
        dataScalarPtr             = ScalarPtr{q: 1}
        dataPtrScalar             = PtrScalar{w: 1}
        dataBigStruct             = BigStruct{w: 1}

diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index fcfcc7261cc724b832b476cc76eb657f0812f9b1..546c331614ac0fec7837312b6f6eecd6cbd2756f 100644 (file)
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -583,7 +583,7 @@ func heapBitsSetType(x, size, dataSize uintptr, typ *_type) {
        // The checks for size == ptrSize and size == 2*ptrSize can therefore
        // assume that dataSize == size without checking it explicitly.
 
-       if size == ptrSize {
+       if ptrSize == 8 && size == ptrSize {
                // It's one word and it has pointers, it must be a pointer.
                // In general we'd need an atomic update here if the
                // concurrent GC were marking objects in this span,
@@ -635,11 +635,28 @@ func heapBitsSetType(x, size, dataSize uintptr, typ *_type) {
        // are 4-word aligned (because they're all 16-byte aligned).
        if size == 2*ptrSize {
                if typ.size == ptrSize {
-                       // 2-element slice of pointer.
-                       if gcphase == _GCoff {
-                               *h.bitp |= (bitPointer | bitPointer<<heapBitsShift) << h.shift
+                       // We're allocating a block big enough to hold two pointers.
+                       // On 64-bit, that means the actual object must be two pointers,
+                       // or else we'd have used the one-pointer-sized block.
+                       // On 32-bit, however, this is the 8-byte block, the smallest one.
+                       // So it could be that we're allocating one pointer and this was
+                       // just the smallest block available. Distinguish by checking dataSize.
+                       // (In general the number of instances of typ being allocated is
+                       // dataSize/typ.size.)
+                       if ptrSize == 4 && dataSize == ptrSize {
+                               // 1 pointer.
+                               if gcphase == _GCoff {
+                                       *h.bitp |= bitPointer << h.shift
+                               } else {
+                                       atomicor8(h.bitp, bitPointer<<h.shift)
+                               }
                        } else {
-                               atomicor8(h.bitp, (bitPointer|bitPointer<<heapBitsShift)<<h.shift)
+                               // 2-element slice of pointer.
+                               if gcphase == _GCoff {
+                                       *h.bitp |= (bitPointer | bitPointer<<heapBitsShift) << h.shift
+                               } else {
+                                       atomicor8(h.bitp, (bitPointer|bitPointer<<heapBitsShift)<<h.shift)
+                               }
                        }
                        return
                }