commit 6502ca57a26a7e0694f4b413453c595de54c03f5
parent 3f4b804de8bf477fafe37f115c041978dfa16f7b
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Sat, 1 Nov 2025 11:58:10 +0000
Bug 1994871 - Part 1: Simplify error handling for SparseBitmap by handling OOM in the caller r=sfink
This removes the infallible versions of SparseBitmap methods.
Differential Revision: https://phabricator.services.mozilla.com/D269954
Diffstat:
4 files changed, 34 insertions(+), 49 deletions(-)
diff --git a/js/src/ds/Bitmap.cpp b/js/src/ds/Bitmap.cpp
@@ -37,15 +37,6 @@ SparseBitmap::BitBlock* SparseBitmap::createBlock(Data::AddPtr p,
return block.release();
}
-SparseBitmap::BitBlock& SparseBitmap::createBlock(
- Data::AddPtr p, size_t blockId, AutoEnterOOMUnsafeRegion& oomUnsafe) {
- BitBlock* block = createBlock(p, blockId);
- if (!block) {
- oomUnsafe.crash("Bitmap OOM");
- }
- return *block;
-}
-
bool SparseBitmap::getBit(size_t bit) const {
size_t word = bit / JS_BITS_PER_WORD;
size_t blockWord = blockStartWord(word);
@@ -85,14 +76,19 @@ void SparseBitmap::bitwiseAndWith(const DenseBitmap& other) {
}
}
-void SparseBitmap::bitwiseOrWith(const SparseBitmap& other) {
+bool SparseBitmap::bitwiseOrWith(const SparseBitmap& other) {
for (Data::Range r(other.data.all()); !r.empty(); r.popFront()) {
const BitBlock& otherBlock = *r.front().value();
- BitBlock& block = getOrCreateBlock(r.front().key());
+ BitBlock* block = getOrCreateBlock(r.front().key());
+ if (!block) {
+ return false;
+ }
for (size_t i = 0; i < WordsInBlock; i++) {
- block[i] |= otherBlock[i];
+ (*block)[i] |= otherBlock[i];
}
}
+
+ return true;
}
void SparseBitmap::bitwiseOrInto(DenseBitmap& other) const {
diff --git a/js/src/ds/Bitmap.h b/js/src/ds/Bitmap.h
@@ -110,9 +110,6 @@ class SparseBitmap {
return static_cast<size_t>(std::clamp(count, 0l, (long)WordsInBlock));
}
- BitBlock& createBlock(Data::AddPtr p, size_t blockId,
- AutoEnterOOMUnsafeRegion& oomUnsafe);
-
BitBlock* createBlock(Data::AddPtr p, size_t blockId);
MOZ_ALWAYS_INLINE BitBlock* getBlock(size_t blockId) const {
@@ -126,18 +123,7 @@ class SparseBitmap {
return p ? p->value() : nullptr;
}
- MOZ_ALWAYS_INLINE BitBlock& getOrCreateBlock(size_t blockId) {
- // The lookupForAdd() needs protection against injected OOMs, as does
- // the add() within createBlock().
- AutoEnterOOMUnsafeRegion oomUnsafe;
- Data::AddPtr p = data.lookupForAdd(blockId);
- if (p) {
- return *p->value();
- }
- return createBlock(p, blockId, oomUnsafe);
- }
-
- MOZ_ALWAYS_INLINE BitBlock* getOrCreateBlockFallible(size_t blockId) {
+ MOZ_ALWAYS_INLINE BitBlock* getOrCreateBlock(size_t blockId) {
Data::AddPtr p = data.lookupForAdd(blockId);
if (p) {
return p->value();
@@ -150,17 +136,10 @@ class SparseBitmap {
size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
- MOZ_ALWAYS_INLINE void setBit(size_t bit) {
- size_t word = bit / JS_BITS_PER_WORD;
- size_t blockWord = blockStartWord(word);
- BitBlock& block = getOrCreateBlock(blockWord / WordsInBlock);
- block[word - blockWord] |= bitMask(bit);
- }
-
- MOZ_ALWAYS_INLINE bool setBitFallible(size_t bit) {
+ [[nodiscard]] MOZ_ALWAYS_INLINE bool setBit(size_t bit) {
size_t word = bit / JS_BITS_PER_WORD;
size_t blockWord = blockStartWord(word);
- BitBlock* block = getOrCreateBlockFallible(blockWord / WordsInBlock);
+ BitBlock* block = getOrCreateBlock(blockWord / WordsInBlock);
if (!block) {
return false;
}
@@ -172,7 +151,7 @@ class SparseBitmap {
bool readonlyThreadsafeGetBit(size_t bit) const;
void bitwiseAndWith(const DenseBitmap& other);
- void bitwiseOrWith(const SparseBitmap& other);
+ [[nodiscard]] bool bitwiseOrWith(const SparseBitmap& other);
void bitwiseOrInto(DenseBitmap& other) const;
// Currently, the following APIs only supports a range of words that is in a
diff --git a/js/src/gc/AtomMarking-inl.h b/js/src/gc/AtomMarking-inl.h
@@ -10,6 +10,7 @@
#include "gc/AtomMarking.h"
#include "mozilla/Assertions.h"
+#include "mozilla/Maybe.h"
#include <type_traits>
@@ -56,12 +57,21 @@ MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomInternal(
size_t bit = GetAtomBit(cell);
MOZ_ASSERT(bit / JS_BITS_PER_WORD < allocatedWords);
- if (Fallible) {
- if (!cx->zone()->markedAtoms().setBitFallible(bit)) {
- return false;
+ {
+ mozilla::Maybe<AutoEnterOOMUnsafeRegion> oomUnsafe;
+ if constexpr (!Fallible) {
+ oomUnsafe.emplace();
+ }
+
+ bool ok = cx->zone()->markedAtoms().setBit(bit);
+
+ if (!ok) {
+ if constexpr (!Fallible) {
+ oomUnsafe->crash("AtomMarkingRuntime::inlinedMarkAtomInternal");
+ } else {
+ return false;
+ }
}
- } else {
- cx->zone()->markedAtoms().setBit(bit);
}
// Trigger a read barrier on the atom, in case there is an incremental
diff --git a/js/src/jsapi-tests/testSparseBitmap.cpp b/js/src/jsapi-tests/testSparseBitmap.cpp
@@ -28,7 +28,7 @@ BEGIN_TEST(testSparseBitmapBasics) {
// Set some bits in the first block and check they are set.
for (size_t i = 0; i < 100; i += 2) {
- bitmap.setBit(i);
+ CHECK(bitmap.setBit(i));
}
for (size_t i = 0; i < 100; i++) {
CHECK(bitmap.getBit(i) == ((i % 2) == 0));
@@ -36,7 +36,7 @@ BEGIN_TEST(testSparseBitmapBasics) {
// Set some bits in different blocks and check they are set.
for (size_t i = 0; i < 100; i += 2) {
- bitmap.setBit(i * 1000);
+ CHECK(bitmap.setBit(i * 1000));
}
for (size_t i = 0; i < 100; i++) {
CHECK(bitmap.getBit(i * 1000) == ((i % 2) == 0));
@@ -45,14 +45,14 @@ BEGIN_TEST(testSparseBitmapBasics) {
// Create another bitmap with different bits set.
SparseBitmap other;
for (size_t i = 1; i < 100; i += 2) {
- other.setBit(i * 1000);
+ CHECK(other.setBit(i * 1000));
}
for (size_t i = 0; i < 100; i++) {
CHECK(other.getBit(i * 1000) == ((i % 2) != 0));
}
// OR some bits into this bitmap and check the result.
- bitmap.bitwiseOrWith(other);
+ CHECK(bitmap.bitwiseOrWith(other));
for (size_t i = 0; i < 100; i++) {
CHECK(bitmap.getBit(i * 1000));
}
@@ -79,7 +79,7 @@ BEGIN_TEST(testSparseBitmapExternalOR) {
// Create a bitmap with one bit set per word so we can tell them apart.
SparseBitmap bitmap;
for (size_t i = 0; i < wordCount; i++) {
- bitmap.setBit(i * JS_BITS_PER_WORD + i);
+ CHECK(bitmap.setBit(i * JS_BITS_PER_WORD + i));
}
// Copy a single word.
@@ -121,8 +121,8 @@ BEGIN_TEST(testSparseBitmapExternalAND) {
// Create a bitmap with two bits set per word based on the word index.
SparseBitmap bitmap;
for (size_t i = 0; i < wordCount; i++) {
- bitmap.setBit(i * JS_BITS_PER_WORD + i);
- bitmap.setBit(i * JS_BITS_PER_WORD + i + 1);
+ CHECK(bitmap.setBit(i * JS_BITS_PER_WORD + i));
+ CHECK(bitmap.setBit(i * JS_BITS_PER_WORD + i + 1));
}
// Update a single word, clearing one of the bits.
