Skip to content

Commit

Permalink
runtime: initialize pointer bits of noscan spans
Browse files Browse the repository at this point in the history
Some code paths in the runtime (cgo, heapdump) request heap bits
without first checking that the span is !noscan. Instead of trying
to find and work around all those cases, just set the pointer bits
of noscan spans correctly. It's somewhat safer than ensuring we
caught all the possible cases.

Fixes #54557
Fixes #54558

Change-Id: Ibd476e6cdea77c962e4d15aad26f29df66fd94e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/425194
Reviewed-by: Michael Knyszek <[email protected]>
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
  • Loading branch information
randall77 committed Aug 23, 2022
1 parent e1114fd commit a6e6b11
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
23 changes: 16 additions & 7 deletions src/runtime/mbitmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@
// If ha.noMorePtrs[i]>>j&1 is set, the entries in ha.bitmap[8*i+j+1] and
// beyond must all be zero until the start of the next object.
//
// The bitmap for noscan spans is not maintained (can be junk). Code must
// ensure that an object is scannable before consulting its bitmap by
// checking either the noscan bit in the span or by consulting its
// type's information.
// The bitmap for noscan spans is set to all zero at span allocation time.
//
// The bitmap for unallocated objects is also not maintained.
// The bitmap for unallocated objects in scannable spans is not maintained
// (can be junk).

package runtime

Expand Down Expand Up @@ -722,6 +720,16 @@ func typeBitsBulkBarrier(typ *_type, dst, src, size uintptr) {
// If this is a span of single pointer allocations, it initializes all
// words to pointer.
func (s *mspan) initHeapBits() {
if s.spanclass.noscan() {
// Set all the pointer bits to zero. We do this once
// when the span is allocated so we don't have to do it
// for each object allocation.
base := s.base()
size := s.npages * pageSize
h := writeHeapBitsForAddr(base)
h.flush(base, size)
return
}
isPtrs := goarch.PtrSize == 8 && s.elemsize == goarch.PtrSize
if !isPtrs {
return // nothing to do
Expand Down Expand Up @@ -873,7 +881,7 @@ func (h writeHeapBits) flush(addr, size uintptr) {
// Continue on writing zeros for the rest of the object.
// For standard use of the ptr bits this is not required, as
// the bits are read from the beginning of the object. Some uses,
// like oblets, bulk write barriers, and cgocheck, might
// like noscan spans, oblets, bulk write barriers, and cgocheck, might
// start mid-object, so these writes are still required.
for {
// Write zero bits.
Expand Down Expand Up @@ -942,7 +950,8 @@ func heapBitsSetType(x, size, dataSize uintptr, typ *_type) {
// It's one word and it has pointers, it must be a pointer.
// Since all allocated one-word objects are pointers
// (non-pointers are aggregated into tinySize allocations),
// initSpan sets the pointer bits for us. Nothing to do here.
// (*mspan).initHeapBits sets the pointer bits for us.
// Nothing to do here.
if doubleCheck {
h, addr := heapBitsForAddr(x, size).next()
if addr != x {
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/mgcmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,8 @@ func scanobject(b uintptr, gcw *gcWork) {
throw("scanobject n == 0")
}
if s.spanclass.noscan() {
// Correctness-wise this is ok, but it's inefficient
// if noscan objects reach here.
throw("scanobject of a noscan object")
}

Expand Down

0 comments on commit a6e6b11

Please sign in to comment.