]> Cypherpunks.ru repositories - gostls13.git/commitdiff
runtime: delay incrementing freeindex in malloc
authorCherry Mui <cherryyz@google.com>
Wed, 9 Nov 2022 15:44:36 +0000 (10:44 -0500)
committerMichael Knyszek <mknyszek@google.com>
Fri, 11 Nov 2022 18:33:54 +0000 (18:33 +0000)
When the GC is scanning some memory (possibly conservatively),
finding a pointer, while concurrently another goroutine is
allocating an object at the same address as the found pointer, the
GC may see the pointer before the object and/or the heap bits are
initialized. This may cause the GC to see bad pointers and
possibly crash.

To prevent this, we make it that the scanner can only see the
object as allocated after the object and the heap bits are
initialized. As the scanner uses the freeindex to determine if an
object is allocated, we delay the increment of freeindex after the
initialization.

As currently in some code path finding the next free index and
updating the free index to a new slot past it is coupled, this
needs a small refactoring. In the new code mspan.nextFreeIndex
return the next free index but not update it (although allocCache
is updated). mallocgc will update it at a later time.

Fixes #54596.

Change-Id: I6dd5ccf743f2d2c46a1ed67c6a8237fe09a71260
Reviewed-on: https://go-review.googlesource.com/c/go/+/427619
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
src/runtime/malloc.go
src/runtime/mbitmap.go
src/runtime/mcentral.go

index 70a13d0576bd0877b40cc1910ef9a1ffd528c884..c7335c55c619115338edb57be86c1179920e13b3 100644 (file)
@@ -813,24 +813,22 @@ retry:
 // base address for all 0-byte allocations
 var zerobase uintptr
 
-// nextFreeFast returns the next free object if one is quickly available.
-// Otherwise it returns 0.
-func nextFreeFast(s *mspan) gclinkptr {
+// nextFreeFast returns the next free object if one is quickly available,
+// and the corresponding free index. Otherwise it returns 0, 0.
+func nextFreeFast(s *mspan) (gclinkptr, uintptr) {
        theBit := sys.TrailingZeros64(s.allocCache) // Is there a free object in the allocCache?
        if theBit < 64 {
                result := s.freeindex + uintptr(theBit)
                if result < s.nelems {
-                       freeidx := result + 1
-                       if freeidx%64 == 0 && freeidx != s.nelems {
-                               return 0
-                       }
                        s.allocCache >>= uint(theBit + 1)
-                       s.freeindex = freeidx
+                       // NOTE: s.freeindex is not updated for now (although allocCache
+                       // is updated). mallocgc will update s.freeindex later after the
+                       // memory is initialized.
                        s.allocCount++
-                       return gclinkptr(result*s.elemsize + s.base())
+                       return gclinkptr(result*s.elemsize + s.base()), result
                }
        }
-       return 0
+       return 0, 0
 }
 
 // nextFree returns the next free object from the cached span if one is available.
@@ -842,10 +840,10 @@ func nextFreeFast(s *mspan) gclinkptr {
 //
 // Must run in a non-preemptible context since otherwise the owner of
 // c could change.
-func (c *mcache) nextFree(spc spanClass) (v gclinkptr, s *mspan, shouldhelpgc bool) {
+func (c *mcache) nextFree(spc spanClass) (v gclinkptr, s *mspan, freeIndex uintptr, shouldhelpgc bool) {
        s = c.alloc[spc]
        shouldhelpgc = false
-       freeIndex := s.nextFreeIndex()
+       freeIndex = s.nextFreeIndex()
        if freeIndex == s.nelems {
                // The span is full.
                if uintptr(s.allocCount) != s.nelems {
@@ -953,6 +951,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
        // In some cases block zeroing can profitably (for latency reduction purposes)
        // be delayed till preemption is possible; delayedZeroing tracks that state.
        delayedZeroing := false
+       var freeidx uintptr
        if size <= maxSmallSize {
                if noscan && size < maxTinySize {
                        // Tiny allocator.
@@ -1012,9 +1011,10 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
                        }
                        // Allocate a new maxTinySize block.
                        span = c.alloc[tinySpanClass]
-                       v := nextFreeFast(span)
+                       var v gclinkptr
+                       v, freeidx = nextFreeFast(span)
                        if v == 0 {
-                               v, span, shouldhelpgc = c.nextFree(tinySpanClass)
+                               v, span, freeidx, shouldhelpgc = c.nextFree(tinySpanClass)
                        }
                        x = unsafe.Pointer(v)
                        (*[2]uint64)(x)[0] = 0
@@ -1037,9 +1037,10 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
                        size = uintptr(class_to_size[sizeclass])
                        spc := makeSpanClass(sizeclass, noscan)
                        span = c.alloc[spc]
-                       v := nextFreeFast(span)
+                       var v gclinkptr
+                       v, freeidx = nextFreeFast(span)
                        if v == 0 {
-                               v, span, shouldhelpgc = c.nextFree(spc)
+                               v, span, freeidx, shouldhelpgc = c.nextFree(spc)
                        }
                        x = unsafe.Pointer(v)
                        if needzero && span.needzero != 0 {
@@ -1051,7 +1052,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
                // For large allocations, keep track of zeroed state so that
                // bulk zeroing can be happen later in a preemptible context.
                span = c.allocLarge(size, noscan)
-               span.freeindex = 1
+               freeidx = 0
                span.allocCount = 1
                size = span.elemsize
                x = unsafe.Pointer(span.base())
@@ -1093,6 +1094,11 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
        // but see uninitialized memory or stale heap bits.
        publicationBarrier()
 
+       // As x and the heap bits are initialized, update
+       // freeindx now so x is seen by the GC (including
+       // convervative scan) as an allocated object.
+       span.updateFreeIndex(freeidx + 1)
+
        // Allocate black during GC.
        // All slots hold nil so no scanning is needed.
        // This may be racing with GC so do it atomically if there can be
index dc99ba768bb4c160a58640e559f66f7610e27536..8fee8262b71ebdf275466d3d63faf7cb2af81845 100644 (file)
@@ -132,7 +132,8 @@ func (s *mspan) refillAllocCache(whichByte uintptr) {
 }
 
 // nextFreeIndex returns the index of the next free object in s at
-// or after s.freeindex.
+// or after s.freeindex. s.freeindex is not updated (except the full
+// span case), but the alloc cache is updated.
 // There are hardware instructions that can be used to make this
 // faster if profiling warrants it.
 func (s *mspan) nextFreeIndex() uintptr {
@@ -170,9 +171,18 @@ func (s *mspan) nextFreeIndex() uintptr {
        }
 
        s.allocCache >>= uint(bitIndex + 1)
-       sfreeindex = result + 1
 
-       if sfreeindex%64 == 0 && sfreeindex != snelems {
+       // NOTE: s.freeindex is not updated for now (although allocCache
+       // is updated). mallocgc will update s.freeindex later after the
+       // memory is initialized.
+
+       return result
+}
+
+// updateFreeIndex updates s.freeindex to sfreeindex, refills
+// the allocCache if necessary.
+func (s *mspan) updateFreeIndex(sfreeindex uintptr) {
+       if sfreeindex%64 == 0 && sfreeindex != s.nelems {
                // We just incremented s.freeindex so it isn't 0.
                // As each 1 in s.allocCache was encountered and used for allocation
                // it was shifted away. At this point s.allocCache contains all 0s.
@@ -182,7 +192,6 @@ func (s *mspan) nextFreeIndex() uintptr {
                s.refillAllocCache(whichByte)
        }
        s.freeindex = sfreeindex
-       return result
 }
 
 // isFree reports whether the index'th object in s is unallocated.
index 3382c54e7f6269b4ce9fe1d61851d2bdb1aeb8ec..6621af5f783148b6630e0239e5ec17130d8f2ad5 100644 (file)
@@ -146,7 +146,6 @@ func (c *mcentral) cacheSpan() *mspan {
                                // Check if there's any free space.
                                freeIndex := s.nextFreeIndex()
                                if freeIndex != s.nelems {
-                                       s.freeindex = freeIndex
                                        sweep.active.end(sl)
                                        goto havespan
                                }