Skip to content

Commit 3675bff

Browse files
committed
runtime: mark heapBits.bits nosplit
heapBits.bits is used during bulkBarrierPreWrite via heapBits.isPointer, which means it must not be preempted. If it is preempted, several bad things can happen: 1. This could allow a GC phase change, and the resulting shear between the barriers and the memory writes could result in a lost pointer. 2. Since bulkBarrierPreWrite uses the P's local write barrier buffer, if it also migrates to a different P, it could try to append to the write barrier buffer concurrently with another write barrier. This can result in the buffer's next pointer skipping over its end pointer, which results in a buffer overflow that can corrupt arbitrary other fields in the Ps (or anything in the heap, really, but it'll probably crash from the corrupted P quickly). Fix this by marking heapBits.bits go:nosplit. This would be the perfect use for a recursive no-preempt annotation (#21314). This doesn't actually affect any binaries because this function was always inlined anyway. (I discovered it when I was modifying heapBits and make h.bits() no longer inline, which led to rampant crashes from problem 2 above.) Updates #22987 and #22988 (but doesn't fix because it doesn't actually change the generated code). Change-Id: I60ebb928b1233b0613361ac3d0558d7b1cb65610 Reviewed-on: https://go-review.googlesource.com/83015 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Rick Hudson <rlh@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
1 parent 0da486d commit 3675bff

File tree

1 file changed

+3
-0
lines changed

1 file changed

+3
-0
lines changed

src/runtime/mbitmap.go

+3
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,9 @@ func (h heapBits) forward(n uintptr) heapBits {
475475
// The caller can test morePointers and isPointer by &-ing with bitScan and bitPointer.
476476
// The result includes in its higher bits the bits for subsequent words
477477
// described by the same bitmap byte.
478+
//
479+
// nosplit because it is used during write barriers and must not be preempted.
480+
//go:nosplit
478481
func (h heapBits) bits() uint32 {
479482
// The (shift & 31) eliminates a test and conditional branch
480483
// from the generated code.

0 commit comments

Comments
 (0)