commit 9c37b4ea73a7f757aad3cd27b48c0517d5ad7f4b
parent e1a772de0e69e8661d5c1294c0994f91aa069176
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Sat, 1 Nov 2025 11:58:10 +0000
Bug 1994871 - Part 3: Make the atom marking bitmaps handle gray state becoming invalid r=sfink
When we OOM during gray unmarking or abort incremental marking, mark all
references to symbols in the atom marking bitmaps as being black. Any of these
references could have been marked black by later gray unmarking or incremental
marking.
The state becoming invalid is generally rare in practice (it does happen during
tests though). This loses some information but the CC will trigger a full GC if
the gray bits are invalid which will help rebuild it.
Differential Revision: https://phabricator.services.mozilla.com/D270687
Diffstat:
8 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/js/src/ds/Bitmap.h b/js/src/ds/Bitmap.h
@@ -207,6 +207,22 @@ class SparseBitmap {
}
}
}
+
+ template <typename F>
+ void forEachWord(size_t wordStart, size_t numWords, F&& func) {
+ size_t blockWord = blockStartWord(wordStart);
+
+ // We only support using a single bit block in this API.
+ MOZ_ASSERT(numWords &&
+ (blockWord == blockStartWord(wordStart + numWords - 1)));
+
+ BitBlock* block = getBlock(blockWord / WordsInBlock);
+ if (block) {
+ for (size_t i = 0; i < numWords; i++) {
+ func((*block)[wordStart - blockWord + i]);
+ }
+ }
+ }
};
} // namespace js
diff --git a/js/src/gc/AtomMarking-inl.h b/js/src/gc/AtomMarking-inl.h
@@ -99,11 +99,13 @@ inline void AtomMarkingRuntime::maybeUnmarkGrayAtomically(Zone* zone,
return;
}
+ // The atom is currently marked black or gray.
MOZ_ASSERT(atomIsMarked(zone, symbol));
+ // Set the black bit. This has the effect of making the mark black if it was
+ // previously gray.
size_t blackBit = getAtomBit(symbol) + size_t(ColorBit::BlackBit);
MOZ_ASSERT(blackBit / JS_BITS_PER_WORD < allocatedWords);
-
zone->markedAtoms().atomicSetExistingBit(blackBit);
MOZ_ASSERT(getAtomMarkColor(zone, symbol) == CellColor::Black);
diff --git a/js/src/gc/AtomMarking.cpp b/js/src/gc/AtomMarking.cpp
@@ -171,19 +171,19 @@ void AtomMarkingRuntime::refineZoneBitmapsForCollectedZones(GCRuntime* gc) {
// marked black this works on its own. For kinds that can be marked gray we must
// preprocess the mark bitmap so that both mark bits are set for black cells.
-// Mask of bit positions in a mark bitmap word that can be black bits, which is
-// every other bit.
+// Masks of bit positions in a mark bitmap word that can be ColorBit::BlackBit
+// or GrayOrBlackBit, which alternative throughout the word.
#if JS_BITS_PER_WORD == 32
static constexpr uintptr_t BlackBitMask = 0x55555555;
#else
static constexpr uintptr_t BlackBitMask = 0x5555555555555555;
#endif
+static constexpr uintptr_t GrayOrBlackBitMask = ~BlackBitMask;
static void PropagateBlackBitsToGrayOrBlackBits(DenseBitmap& bitmap,
Arena* arena) {
// This only works if the gray bit and black bits are in the same word,
// which is true for symbols.
-
MOZ_ASSERT(
TraceKindCanBeMarkedGray(MapAllocToTraceKind(arena->getAllocKind())));
MOZ_ASSERT((arena->getThingSize() / CellBytesPerMarkBit) % 2 == 0);
@@ -200,6 +200,19 @@ static void PropagateBlackBitsToGrayOrBlackBits(
}
}
+static void PropagateGrayOrBlackBitsToBlackBits(SparseBitmap& bitmap,
+ Arena* arena) {
+ // This only works if the gray bit and black bits are in the same word,
+ // which is true for symbols.
+ MOZ_ASSERT(
+ TraceKindCanBeMarkedGray(MapAllocToTraceKind(arena->getAllocKind())));
+ MOZ_ASSERT((arena->getThingSize() / CellBytesPerMarkBit) % 2 == 0);
+
+ bitmap.forEachWord(
+ arena->atomBitmapStart(), ArenaBitmapWords,
+ [](uintptr_t& word) { word |= (word & GrayOrBlackBitMask) >> 1; });
+}
+
#ifdef DEBUG
static bool ArenaContainsGrayCells(Arena* arena) {
for (ArenaCellIter cell(arena); !cell.done(); cell.next()) {
@@ -327,6 +340,24 @@ void AtomMarkingRuntime::markAtomsUsedByUncollectedZones(
BitwiseOrIntoChunkMarkBits(gc->atomsZone(), *markedUnion);
}
+void AtomMarkingRuntime::unmarkAllGrayReferences(GCRuntime* gc) {
+ for (ZonesIter sourceZone(gc, SkipAtoms); !sourceZone.done();
+ sourceZone.next()) {
+ MOZ_ASSERT(!sourceZone->isAtomsZone());
+ auto& bitmap = sourceZone->markedAtoms();
+ for (ArenaIter arena(gc->atomsZone(), AllocKind::SYMBOL); !arena.done();
+ arena.next()) {
+ PropagateGrayOrBlackBitsToBlackBits(bitmap, arena);
+ }
+#ifdef DEBUG
+ for (auto cell = gc->atomsZone()->cellIter<JS::Symbol>(); !cell.done();
+ cell.next()) {
+ MOZ_ASSERT(getAtomMarkColor(sourceZone, cell.get()) != CellColor::Gray);
+ }
+#endif
+ }
+}
+
template <typename T>
void AtomMarkingRuntime::markAtom(JSContext* cx, T* thing) {
// Trigger a read barrier on the atom, in case there is an incremental
diff --git a/js/src/gc/AtomMarking.h b/js/src/gc/AtomMarking.h
@@ -65,6 +65,13 @@ class AtomMarkingRuntime {
void markAtomsUsedByUncollectedZones(GCRuntime* gc,
UniquePtr<DenseBitmap> markedUnion);
+ // If gray unmarking fails or GC marking is aborted then the gray bits may end
+ // up in an invalid state. This updates references to all things that could be
+ // gray in the atom marking bitmaps, marking them as black if they were
+ // previously (and perhaps incorrectly) considered gray. This is always safe
+ // but loses information.
+ void unmarkAllGrayReferences(GCRuntime* gc);
+
// Get the index into the atom marking bitmaps for the first bit associated
// with an atom.
// This is public for testing access.
diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp
@@ -3882,6 +3882,11 @@ GCRuntime::IncrementalResult GCRuntime::resetIncrementalGC(
return IncrementalResult::Reset;
}
+void GCRuntime::setGrayBitsInvalid() {
+ grayBitsValid = false;
+ atomMarking.unmarkAllGrayReferences(this);
+}
+
void GCRuntime::disableIncrementalBarriers() {
// Clear needsIncrementalBarrier so we don't do any write barriers during
// foreground finalization. This would otherwise happen when destroying
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
@@ -547,7 +547,7 @@ class GCRuntime {
bool didCompactZones() const { return isCompacting && zonesCompacted; }
bool areGrayBitsValid() const { return grayBitsValid; }
- void setGrayBitsInvalid() { grayBitsValid = false; }
+ void setGrayBitsInvalid();
mozilla::TimeStamp lastGCStartTime() const { return lastGCStartTime_; }
mozilla::TimeStamp lastGCEndTime() const { return lastGCEndTime_; }
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
@@ -2904,7 +2904,7 @@ void UnmarkGrayTracer::onChild(JS::GCCellPtr thing, const char* name) {
Zone* zone = tenured.zoneFromAnyThread();
// If the cell is in a zone whose mark bits are being cleared, then it will
- // end up white.
+ // end up being marked black by GC marking.
if (zone->isGCPreparing()) {
return;
}
@@ -2915,24 +2915,28 @@ void UnmarkGrayTracer::onChild(JS::GCCellPtr thing, const char* name) {
}
if (zone->isGCMarking()) {
- // If the cell is in a zone that we're currently marking, then it's possible
- // that it is currently white but will end up gray. To handle this case,
- // trigger the barrier for any cells in zones that are currently being
- // marked. This will ensure they will eventually get marked black.
+ // If the cell is in a zone that we're currently marking, then it's
+ // possible that it is currently white but will end up gray. To handle
+ // this case, trigger the barrier for any cells in zones that are
+ // currently being marked. This will ensure they will eventually get
+ // marked black.
TraceEdgeForBarrier(marker, &tenured, thing.kind());
} else if (tenured.isMarkedGray()) {
- // TODO: It may be a small improvement to only use the atomic version during
- // parallel marking.
+ // TODO: It may be a small improvement to only use the atomic version
+ // during parallel marking.
tenured.markBlackAtomic();
if (!stack.append(thing)) {
oom = true;
}
}
+ // As well as updating the mark bits, we may need to update the color in the
+ // atom marking bitmap to record that |zone| now has a black edge to |thing|.
if (zone->isAtomsZone() && sourceZone) {
MOZ_ASSERT(tenured.is<JS::Symbol>());
- runtime()->gc.atomMarking.maybeUnmarkGrayAtomically(
- sourceZone, tenured.as<JS::Symbol>());
+ GCRuntime* gc = &runtime()->gc;
+ JS::Symbol* symbol = tenured.as<JS::Symbol>();
+ gc->atomMarking.maybeUnmarkGrayAtomically(sourceZone, symbol);
}
unmarkedAny = true;
diff --git a/js/src/vm/JSContext-inl.h b/js/src/vm/JSContext-inl.h
@@ -104,9 +104,8 @@ class ContextChecks {
// marking is taking place.
gc::GCRuntime* gc = &cx->runtime()->gc;
bool isGCMarking =
- gc->state() >= gc::State::Mark && gc->state() <= gc::State::Sweep;
+ gc->state() >= gc::State::Prepare && gc->state() <= gc::State::Sweep;
if (zone() && !isGCMarking) {
- gc::GCRuntime* gc = &cx->runtime()->gc;
gc::CellColor color = gc->atomMarking.getAtomMarkColor(zone(), thing);
if (color != gc::CellColor::Black) {
MOZ_CRASH_UNSAFE_PRINTF(
