From 2cdfede3cb4d73a7287d87b93e96c7287b27091b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:46:13 +0000 Subject: [PATCH 1/3] [JSC] Throw instead of crashing when a Vector that grows with script input cannot grow Eight builtins keep one entry per script-controlled item in a WTF::Vector and grow it with the infallible append(): String.prototype.replaceAll(string, string) the offset of every match String.prototype.replace(regexp, string) one part per "$" reference String.prototype.split(string) the end of every piece JSON.parse(text, reviver) the source range of every array element FinalizationRegistry.prototype.register every registration Intl.ListFormat format() and formatToParts() every string of the iterable new WebAssembly.Tag({ parameters }) every parameter type WebAssembly compile options { builtins } every name of the list append() crashes in two cases. A Vector holds at most 2^31 bytes (isValidCapacityForVector), and a growth step past that is a CRASH() in VectorBufferBase::allocateBuffer. And the allocation itself can fail. Either way script alone ends the process, inside try/catch too: ','.repeat(372712672).split(',') 'a'.repeat(2 ** 28).replaceAll('a', 'c') 'x'.replace(/(x)/g, '$1'.repeat(180000000)) JSON.parse('[' + '1,'.repeat(51821029) + '1]', (key, value) => value) Each site now appends with tryAppend() and throws the out-of-memory RangeError when that fails. The Vectors next to them in StringPrototypeInlines.h already work this way (tryConstructAndAppend() and throwOutOfMemoryError()), and it is what 'a'.repeat(2 ** 28).replace(/a/g, 'c') throws today. The other callers of forEachInIterable() that collect what they iterate use a MarkedArgumentBuffer and check hasOverflowed(). splitStringByOneCharacterImpl() and parseReplacementTemplate() have no ThrowScope, so they report the failure to their caller. JSFinalizationRegistry::registerTarget() now reports whether it registered the target. When the Vector of a new unregister token cannot take its first registration, the empty bucket is removed again, because reconcileWeakReferencesAtGCEnd() expects every bucket to hold a registration. Not changed: the appends inside reconcileWeakReferencesAtGCEnd(), which run at the end of a collection where nothing can throw, and JSONRanges::record(), which is a MarkedArgumentBuffer::appendWithCrashOnOverflow(). The new test runs in debug builds, where --maxSingleAllocationSize makes the allocation fail after a few hundred thousand items. The replaceAll test reaches the 2^31 byte limit for real, so it is memoryHog and slow. * JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js: Added. * JSTests/stress/string-replaceAll-string-string-too-many-matches.js: Added. * Source/JavaScriptCore/runtime/FinalizationRegistryPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/IntlListFormat.cpp: (JSC::stringListFromIterable): * Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp: (JSC::JSFinalizationRegistry::registerTarget): * Source/JavaScriptCore/runtime/JSFinalizationRegistry.h: * Source/JavaScriptCore/runtime/LiteralParser.cpp: (JSC::reviverMode>::parse): * Source/JavaScriptCore/runtime/StringPrototype.cpp: (JSC::splitStringByOneCharacterImpl): (JSC::stringSplitFast): * Source/JavaScriptCore/runtime/StringPrototypeInlines.h: (JSC::stringReplaceAllStringString): (JSC::parseReplacementTemplate): (JSC::replaceAllWithStringUsingRegExpSearch): * Source/JavaScriptCore/wasm/js/WebAssemblyCompileOptions.cpp: (JSC::WebAssemblyCompileOptions::tryCreate): * Source/JavaScriptCore/wasm/js/WebAssemblyTagConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): --- ...ry-when-script-sized-vector-cannot-grow.js | 98 +++++++++++++++++++ ...placeAll-string-string-too-many-matches.js | 52 ++++++++++ .../runtime/FinalizationRegistryPrototype.cpp | 5 +- .../JavaScriptCore/runtime/IntlListFormat.cpp | 3 +- .../runtime/JSFinalizationRegistry.cpp | 17 +++- .../runtime/JSFinalizationRegistry.h | 3 +- .../JavaScriptCore/runtime/LiteralParser.cpp | 6 +- .../runtime/StringPrototype.cpp | 40 +++++--- .../runtime/StringPrototypeInlines.h | 27 +++-- .../wasm/js/WebAssemblyCompileOptions.cpp | 3 +- .../wasm/js/WebAssemblyTagConstructor.cpp | 3 +- 11 files changed, 222 insertions(+), 35 deletions(-) create mode 100644 JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js create mode 100644 JSTests/stress/string-replaceAll-string-string-too-many-matches.js diff --git a/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js b/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js new file mode 100644 index 0000000000000..d624837146baa --- /dev/null +++ b/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js @@ -0,0 +1,98 @@ +//@ if $buildType == "debug" then runDefault("--maxSingleAllocationSize=1048576") else skip end + +// Each builtin below keeps one entry per script-controlled item in a WTF::Vector. When that Vector +// cannot grow, because the allocation fails or because a Vector holds at most 2^31 bytes, the +// builtin has to throw the out-of-memory RangeError. It used to crash. +// --maxSingleAllocationSize makes every allocation of more than 1 MiB fail, so a few hundred thousand +// items are enough here. No string below is longer than 1 MiB, because creating one would crash too. + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`bad value: expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); +} + +function shouldThrowOutOfMemory(func) { + let error = null; + try { + func(); + } catch (e) { + error = e; + } + if (String(error) !== "RangeError: Out of memory") + throw new Error(`bad error: ${String(error)} from ${func}`); +} + +// Never ends on its own: the only way out of the consumer is the throw, which has to close it. +function endless(value) { + const iterable = { + closed: false, + [Symbol.iterator]() { + return { + next() { return { done: false, value }; }, + return() { iterable.closed = true; return { }; }, + }; + }, + }; + return iterable; +} + +// String.prototype.replaceAll(string, string): the offset of every match. +shouldThrowOutOfMemory(() => "a".repeat(200000).replaceAll("a", "c")); +shouldThrowOutOfMemory(() => "a".repeat(200000).replaceAll("", "c")); +shouldBe("banana".replaceAll("a", "o"), "bonono"); + +// String.prototype.replace(regexp, string): one part per "$" reference of the replacement. +shouldThrowOutOfMemory(() => "x".replace(/(x)/g, "$1".repeat(100000))); +shouldThrowOutOfMemory(() => "x".replace(/(x)/g, "$$".repeat(100000))); +shouldThrowOutOfMemory(() => "x".replace(/(?x)/g, "-$".repeat(50000))); +shouldBe("x".replace(/(x)/g, "[$1$$$&]"), "[x$x]"); + +// String.prototype.split(string): the end of every piece. +shouldThrowOutOfMemory(() => ",".repeat(400000).split(",")); +shouldThrowOutOfMemory(() => "\u3042".repeat(400000).split("\u3042")); +shouldThrowOutOfMemory(() => ",;".repeat(400000).split(",;")); +shouldBe(JSON.stringify("a,b,,c".split(",")), `["a","b","","c"]`); + +// JSON.parse with a reviver: the source range of every array element. +shouldThrowOutOfMemory(() => JSON.parse("[" + "1,".repeat(50000) + "1]", (key, value) => value)); +shouldBe(JSON.stringify(JSON.parse("[1,[2,3]]", (key, value) => value)), "[1,[2,3]]"); + +// FinalizationRegistry.prototype.register: every registration, in one list per unregister token. +for (const token of [undefined, { }]) { + const registry = new FinalizationRegistry(() => { }); + const target = { }; + shouldThrowOutOfMemory(() => { + for (;;) + registry.register(target, 1, token); + }); + if (token) { + shouldBe(registry.unregister(token), true); + shouldBe(registry.unregister(token), false); + registry.register(target, 1, token); + shouldBe(registry.unregister(token), true); + } +} + +// Intl.ListFormat: every string of the iterable. +{ + const listFormat = new Intl.ListFormat("en"); + for (const format of [items => listFormat.format(items), items => listFormat.formatToParts(items)]) { + const items = endless("a"); + shouldThrowOutOfMemory(() => format(items)); + shouldBe(items.closed, true); + } + shouldBe(listFormat.format(["a", "b", "c"]), "a, b, and c"); +} + +// WebAssembly.Tag: every parameter type. The compile options: every name of the builtins list. +if (typeof WebAssembly === "object") { + const parameters = endless("i32"); + shouldThrowOutOfMemory(() => new WebAssembly.Tag({ parameters })); + shouldBe(parameters.closed, true); + + const emptyModule = new Uint8Array([0, 97, 115, 109, 1, 0, 0, 0]); + const builtins = endless("js-string"); + shouldThrowOutOfMemory(() => WebAssembly.validate(emptyModule, { builtins })); + shouldBe(builtins.closed, true); + shouldBe(WebAssembly.validate(emptyModule, { builtins: ["js-string"] }), true); +} diff --git a/JSTests/stress/string-replaceAll-string-string-too-many-matches.js b/JSTests/stress/string-replaceAll-string-string-too-many-matches.js new file mode 100644 index 0000000000000..196e239eac462 --- /dev/null +++ b/JSTests/stress/string-replaceAll-string-string-too-many-matches.js @@ -0,0 +1,52 @@ +//@ memoryHog! +//@ slow! +//@ skip if $buildType == "debug" +//@ skip if $addressBits <= 32 +//@ runDefault + +// replaceAll(string, string) records the offset of every match in a Vector before it builds +// the result. A Vector holds at most 2^31 bytes, which is 2^28 - 1 offsets. replaceAll has to throw +// when that Vector cannot grow, the way replaceAll(/regexp/g, string) does. It used to crash. +// Every block below finds more than 100 million matches, so each takes seconds. +// out-of-memory-when-script-sized-vector-cannot-grow.js has the small version of this test, where +// the allocation fails, and the other builtins that had the same bug. + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`bad value: expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); +} + +function shouldThrowOutOfMemory(func) { + let error = null; + try { + func(); + } catch (e) { + error = e; + } + if (String(error) !== "RangeError: Out of memory") + throw new Error(`bad error: ${String(error)}`); +} + +function replaceAll(string, search, replacement) { return string.replaceAll(search, replacement); } +noInline(replaceAll); + +// Warm this up so that the call below also goes through the DFG's StringReplaceAll node. +for (let i = 0; i < testLoopCount; ++i) + shouldBe(replaceAll("banana" + i, "a", "o"), "bonono" + i); + +const string = "a".repeat(2 ** 28); + +shouldThrowOutOfMemory(() => string.replaceAll("a", "c")); +shouldThrowOutOfMemory(() => replaceAll(string, "a", "c")); + +// An empty search string matches before every character and at the end. +shouldThrowOutOfMemory(() => string.replaceAll("", "c")); + +// 2^27 matches fit. +{ + const result = string.substring(2 ** 27).replaceAll("a", "c"); + shouldBe(result.length, 2 ** 27); + shouldBe(result[0], "c"); + shouldBe(result[2 ** 27 - 1], "c"); + shouldBe(result.indexOf("a"), -1); +} diff --git a/Source/JavaScriptCore/runtime/FinalizationRegistryPrototype.cpp b/Source/JavaScriptCore/runtime/FinalizationRegistryPrototype.cpp index f125ba1610409..ff771201ab857 100644 --- a/Source/JavaScriptCore/runtime/FinalizationRegistryPrototype.cpp +++ b/Source/JavaScriptCore/runtime/FinalizationRegistryPrototype.cpp @@ -86,7 +86,10 @@ JSC_DEFINE_HOST_FUNCTION(protoFuncFinalizationRegistryRegister, (JSGlobalObject* if (!unregisterToken.isUndefined() && !canBeHeldWeakly(unregisterToken)) [[unlikely]] return throwVMTypeError(globalObject, scope, "register requires an object or a non-registered symbol as the unregistration token"_s); - group->registerTarget(vm, target.asCell(), holdings, unregisterToken); + if (!group->registerTarget(vm, target.asCell(), holdings, unregisterToken)) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return { }; + } return encodedJSUndefined(); } diff --git a/Source/JavaScriptCore/runtime/IntlListFormat.cpp b/Source/JavaScriptCore/runtime/IntlListFormat.cpp index 4f8221b84cfad..1596d3044e3f5 100644 --- a/Source/JavaScriptCore/runtime/IntlListFormat.cpp +++ b/Source/JavaScriptCore/runtime/IntlListFormat.cpp @@ -156,7 +156,8 @@ static Vector stringListFromIterable(JSGlobalObject* globalObject, JS } String item = value.toWTFString(globalObject); RETURN_IF_EXCEPTION(scope, void()); - result.append(item); + if (!result.tryAppend(WTF::move(item))) [[unlikely]] + throwOutOfMemoryError(globalObject, scope); }); return result; } diff --git a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp index 625e263704ef9..1aeac0c213e8a 100644 --- a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp +++ b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp @@ -204,20 +204,27 @@ JSValue JSFinalizationRegistry::takeDeadHoldingsValue() return result; } -void JSFinalizationRegistry::registerTarget(VM& vm, JSCell* target, JSValue holdings, JSValue token) +bool JSFinalizationRegistry::registerTarget(VM& vm, JSCell* target, JSValue holdings, JSValue token) { Locker locker { cellLock() }; Registration registration; registration.target = target; registration.holdings.setWithoutWriteBarrier(holdings); - if (token.isUndefined()) - m_noUnregistrationLive.append(WTF::move(registration)); - else { + if (token.isUndefined()) { + if (!m_noUnregistrationLive.tryAppend(WTF::move(registration))) [[unlikely]] + return false; + } else { RELEASE_ASSERT(token.isCell()); auto result = m_liveRegistrations.add(token.asCell(), LiveRegistrations()); - result.iterator->value.append(WTF::move(registration)); + if (!result.iterator->value.tryAppend(WTF::move(registration))) [[unlikely]] { + // reconcileWeakReferencesAtGCEnd() expects every bucket to hold a registration. + if (result.isNewEntry) + m_liveRegistrations.remove(result.iterator); + return false; + } } vm.writeBarrier(this); + return true; } bool JSFinalizationRegistry::unregister(VM&, JSCell* token) diff --git a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.h b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.h index 1013d5d379b4a..5f3d8b2df3054 100644 --- a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.h +++ b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.h @@ -78,7 +78,8 @@ class JSFinalizationRegistry final : public JSInternalFieldObjectImpl<1> { bool unregister(VM&, JSCell* token); // token should be a JSObject, Symbol, or undefined. - void registerTarget(VM&, JSCell* target, JSValue holdings, JSValue token); + // Returns false, and registers nothing, when the list of registrations cannot grow. + [[nodiscard]] bool registerTarget(VM&, JSCell* target, JSValue holdings, JSValue token); struct LiveRegistration { JSCell* target; diff --git a/Source/JavaScriptCore/runtime/LiteralParser.cpp b/Source/JavaScriptCore/runtime/LiteralParser.cpp index b984db6c5ece6..2f01b217c309e 100644 --- a/Source/JavaScriptCore/runtime/LiteralParser.cpp +++ b/Source/JavaScriptCore/runtime/LiteralParser.cpp @@ -1705,8 +1705,10 @@ JSValue LiteralParser::parse(VM& vm, ParserState initialS array->putDirectIndex(m_globalObject, array->length(), lastValue); RETURN_IF_EXCEPTION(scope, { }); if constexpr (reviverMode == JSONReviverMode::Enabled) { - if (sourceRanges) - std::get(m_rangesStack.last().properties).append(WTF::move(lastValueRange)); + if (sourceRanges && !std::get(m_rangesStack.last().properties).tryAppend(WTF::move(lastValueRange))) [[unlikely]] { + throwOutOfMemoryError(m_globalObject, scope); + return { }; + } } if (m_lexer.currentToken()->type == TokComma) diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp index 5044b99111478..dd9262133d781 100644 --- a/Source/JavaScriptCore/runtime/StringPrototype.cpp +++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp @@ -1019,9 +1019,14 @@ JSC_DEFINE_HOST_FUNCTION(stringProtoFuncSlice, (JSGlobalObject* globalObject, Ca RELEASE_AND_RETURN(scope, JSValue::encode(stringSlice(globalObject, vm, string, length, start, end))); } -// Return true in case of early return (resultLength got to limitLength). +enum class SplitStatus : uint8_t { + ReachedEnd, + ReachedLimit, // resultLength got to limitLength. + OutOfMemory, // result cannot grow. +}; + template -static ALWAYS_INLINE bool splitStringByOneCharacterImpl(Indice& result, StringImpl* string, char16_t separatorCharacter, unsigned limitLength) +static ALWAYS_INLINE SplitStatus splitStringByOneCharacterImpl(Indice& result, StringImpl* string, char16_t separatorCharacter, unsigned limitLength) { // 12. Let q = p. size_t matchPosition; @@ -1035,17 +1040,18 @@ static ALWAYS_INLINE bool splitStringByOneCharacterImpl(Indice& result, StringIm // through q (exclusive). // 2. Call the [[DefineOwnProperty]] internal method of A with arguments ToString(lengthA), // Property Descriptor {[[Value]]: T, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false. - result.append(matchPosition); + if (!result.tryAppend(matchPosition)) [[unlikely]] + return SplitStatus::OutOfMemory; // 3. Increment lengthA by 1. // 4. If lengthA == lim, return A. if (result.size() == limitLength) - return true; + return SplitStatus::ReachedLimit; // 5. Let p = e. // 8. Let q = p. position = matchPosition + 1; } - return false; + return SplitStatus::ReachedEnd; } JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSString* separatorString, unsigned limit) @@ -1207,13 +1213,15 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt if (separatorLength == 1) { char16_t separatorCharacter = separatorImpl->at(0); - if (stringImpl->is8Bit()) { - if (splitStringByOneCharacterImpl(result, stringImpl, separatorCharacter, limit)) - RELEASE_AND_RETURN(scope, cacheAndCreateArray()); - } else { - if (splitStringByOneCharacterImpl(result, stringImpl, separatorCharacter, limit)) - RELEASE_AND_RETURN(scope, cacheAndCreateArray()); + SplitStatus status = stringImpl->is8Bit() + ? splitStringByOneCharacterImpl(result, stringImpl, separatorCharacter, limit) + : splitStringByOneCharacterImpl(result, stringImpl, separatorCharacter, limit); + if (status == SplitStatus::OutOfMemory) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return { }; } + if (status == SplitStatus::ReachedLimit) + RELEASE_AND_RETURN(scope, cacheAndCreateArray()); } else { // 13. Let q = p. size_t matchPosition; @@ -1226,7 +1234,10 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt // 1. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive) // through q (exclusive). // 2. Call CreateDataProperty(A, ToString(lengthA), T). - result.append(matchPosition); + if (!result.tryAppend(matchPosition)) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return { }; + } // 3. Increment lengthA by 1. // 4. If lengthA == lim, return A. if (result.size() == limit) @@ -1241,7 +1252,10 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt // 15. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive) // through s (exclusive). // 16. Call CreateDataProperty(A, ToString(lengthA), T). - result.append(input->length()); + if (!result.tryAppend(input->length())) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return { }; + } RELEASE_AND_RETURN(scope, cacheAndCreateArray()); } diff --git a/Source/JavaScriptCore/runtime/StringPrototypeInlines.h b/Source/JavaScriptCore/runtime/StringPrototypeInlines.h index 3b496b59d4565..b3139e5c4ab4f 100644 --- a/Source/JavaScriptCore/runtime/StringPrototypeInlines.h +++ b/Source/JavaScriptCore/runtime/StringPrototypeInlines.h @@ -457,7 +457,10 @@ ALWAYS_INLINE JSString* stringReplaceAllStringString(JSGlobalObject* globalObjec if (matchStart == notFound) break; - matchStarts.append(matchStart); + if (!matchStarts.tryAppend(matchStart)) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return nullptr; + } matchStart += searchLength; if (search.isEmpty()) ++matchStart; @@ -1328,7 +1331,8 @@ struct StringReplaceTemplatePart { using StringReplaceTemplateParts = Vector; -ALWAYS_INLINE void parseReplacementTemplate(StringReplaceTemplateParts& parts, StringView replacement, RegExp* regExp, size_t dollarPos) +// Returns false when parts cannot grow. +[[nodiscard]] ALWAYS_INLINE bool parseReplacementTemplate(StringReplaceTemplateParts& parts, StringView replacement, RegExp* regExp, size_t dollarPos) { bool hasNamedCaptures = regExp->hasNamedCaptures(); size_t i = dollarPos; @@ -1341,8 +1345,8 @@ ALWAYS_INLINE void parseReplacementTemplate(StringReplaceTemplateParts& parts, S if (ref == '$') { // "$$" -> "$" ++i; - if (i - offset) - parts.append(StringReplaceTemplatePart::literal(offset, i - offset)); + if (i - offset && !parts.tryAppend(StringReplaceTemplatePart::literal(offset, i - offset))) [[unlikely]] + return false; offset = i + 1; continue; } @@ -1388,15 +1392,17 @@ ALWAYS_INLINE void parseReplacementTemplate(StringReplaceTemplateParts& parts, S } else continue; - if (i - offset) - parts.append(StringReplaceTemplatePart::literal(offset, i - offset)); + if (i - offset && !parts.tryAppend(StringReplaceTemplatePart::literal(offset, i - offset))) [[unlikely]] + return false; i += 1 + advance; offset = i + 1; - parts.append(part); + if (!parts.tryAppend(part)) [[unlikely]] + return false; } while ((i = replacement.find('$', i + 1)) != notFound); - if (replacement.length() - offset) - parts.append(StringReplaceTemplatePart::literal(offset, replacement.length() - offset)); + if (replacement.length() - offset && !parts.tryAppend(StringReplaceTemplatePart::literal(offset, replacement.length() - offset))) [[unlikely]] + return false; + return true; } ALWAYS_INLINE void appendReplacementUsingTemplate(StringBuilder& result, std::span parts, StringView replacement, StringView source, const int* ovector, RegExp* regExp) @@ -1463,7 +1469,8 @@ ALWAYS_INLINE JSString* replaceAllWithStringUsingRegExpSearch(VM& vm, JSGlobalOb if (!anyMatch) { anyMatch = true; - parseReplacementTemplate(templateParts, replacementString, regExp, dollarPos); + if (!parseReplacementTemplate(templateParts, replacementString, regExp, dollarPos)) [[unlikely]] + OUT_OF_MEMORY(globalObject, scope); firstMatch = result; firstOvector.append(std::span { ovector, static_cast(regExp->offsetVectorSize()) }); } else { diff --git a/Source/JavaScriptCore/wasm/js/WebAssemblyCompileOptions.cpp b/Source/JavaScriptCore/wasm/js/WebAssemblyCompileOptions.cpp index a37e440b503ee..7232986d69898 100644 --- a/Source/JavaScriptCore/wasm/js/WebAssemblyCompileOptions.cpp +++ b/Source/JavaScriptCore/wasm/js/WebAssemblyCompileOptions.cpp @@ -66,7 +66,8 @@ std::optional WebAssemblyCompileOptions::tryCreate(JS auto contents = asString(nextValue)->value(globalObject); RETURN_IF_EXCEPTION(scope, void()); String qualifiedName = makeString("wasm:"_s, StringView(contents)); - options.m_qualifiedBuiltinSetNames.append(qualifiedName); + if (!options.m_qualifiedBuiltinSetNames.tryAppend(WTF::move(qualifiedName))) [[unlikely]] + throwOutOfMemoryError(globalObject, scope); } else sawBadEntries = true; }); diff --git a/Source/JavaScriptCore/wasm/js/WebAssemblyTagConstructor.cpp b/Source/JavaScriptCore/wasm/js/WebAssemblyTagConstructor.cpp index 6a19ea2b90712..29029666b7612 100644 --- a/Source/JavaScriptCore/wasm/js/WebAssemblyTagConstructor.cpp +++ b/Source/JavaScriptCore/wasm/js/WebAssemblyTagConstructor.cpp @@ -83,7 +83,8 @@ JSC_DEFINE_HOST_FUNCTION(constructJSWebAssemblyTag, (JSGlobalObject* globalObjec return; } - parameters.append(type); + if (!parameters.tryAppend(type)) [[unlikely]] + throwOutOfMemoryError(globalObject, scope); }); RETURN_IF_EXCEPTION(scope, { }); From 515597e6b6841417a741afda155f0ea5e8e0a68d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:11:11 +0000 Subject: [PATCH 2/3] [JSC] split() releases its scratch Vector before the out-of-memory throw; a registration that does not fit at the end of a collection is dropped split() collects into vm.stringSplitIndice, a scratch Vector that keeps its capacity between calls. When it cannot grow, its buffer (up to 1.4 GiB) is released before the throw. A throw from FinalizationRegistry.prototype.register leaves the list of the registrations without a token full. reconcileWeakReferencesAtGCEnd() appends to that list when a token dies before its target, and nothing can throw at the end of a collection. A registration that does not fit there is now dropped, so its cleanup callback never runs, which the specification allows. The appends to the lists of dead registrations are unchanged. The test is slow!, and the real-size replaceAll test is gone: it needed 6 GB in a plain run of the stress tests, and Bun's test suite has a real-size test that its CI runs. * JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js: * JSTests/stress/string-replaceAll-string-string-too-many-matches.js: Removed. * Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp: (JSC::JSFinalizationRegistry::reconcileWeakReferencesAtGCEnd): * Source/JavaScriptCore/runtime/StringPrototype.cpp: (JSC::stringSplitFast): --- ...ry-when-script-sized-vector-cannot-grow.js | 26 +++++++--- ...placeAll-string-string-too-many-matches.js | 52 ------------------- .../runtime/JSFinalizationRegistry.cpp | 6 ++- .../runtime/StringPrototype.cpp | 24 ++++----- 4 files changed, 36 insertions(+), 72 deletions(-) delete mode 100644 JSTests/stress/string-replaceAll-string-string-too-many-matches.js diff --git a/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js b/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js index d624837146baa..1c80b1d554e81 100644 --- a/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js +++ b/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js @@ -1,3 +1,4 @@ +//@ slow! //@ if $buildType == "debug" then runDefault("--maxSingleAllocationSize=1048576") else skip end // Each builtin below keeps one entry per script-controlled item in a WTF::Vector. When that Vector @@ -58,19 +59,30 @@ shouldThrowOutOfMemory(() => JSON.parse("[" + "1,".repeat(50000) + "1]", (key, v shouldBe(JSON.stringify(JSON.parse("[1,[2,3]]", (key, value) => value)), "[1,[2,3]]"); // FinalizationRegistry.prototype.register: every registration, in one list per unregister token. -for (const token of [undefined, { }]) { +{ const registry = new FinalizationRegistry(() => { }); const target = { }; + const token = { }; shouldThrowOutOfMemory(() => { for (;;) registry.register(target, 1, token); }); - if (token) { - shouldBe(registry.unregister(token), true); - shouldBe(registry.unregister(token), false); - registry.register(target, 1, token); - shouldBe(registry.unregister(token), true); - } + shouldBe(registry.unregister(token), true); + shouldBe(registry.unregister(token), false); + + shouldThrowOutOfMemory(() => { + for (;;) + registry.register(target, 1); + }); + // The list of the registrations without a token is full now. When a token dies before its target, the + // end of the collection moves the registration to that list. Nothing can throw there, so it is dropped. + (function () { + for (let i = 0; i < 100; ++i) + registry.register(target, 2, { }); + })(); + fullGC(); + registry.register(target, 3, token); + shouldBe(registry.unregister(token), true); } // Intl.ListFormat: every string of the iterable. diff --git a/JSTests/stress/string-replaceAll-string-string-too-many-matches.js b/JSTests/stress/string-replaceAll-string-string-too-many-matches.js deleted file mode 100644 index 196e239eac462..0000000000000 --- a/JSTests/stress/string-replaceAll-string-string-too-many-matches.js +++ /dev/null @@ -1,52 +0,0 @@ -//@ memoryHog! -//@ slow! -//@ skip if $buildType == "debug" -//@ skip if $addressBits <= 32 -//@ runDefault - -// replaceAll(string, string) records the offset of every match in a Vector before it builds -// the result. A Vector holds at most 2^31 bytes, which is 2^28 - 1 offsets. replaceAll has to throw -// when that Vector cannot grow, the way replaceAll(/regexp/g, string) does. It used to crash. -// Every block below finds more than 100 million matches, so each takes seconds. -// out-of-memory-when-script-sized-vector-cannot-grow.js has the small version of this test, where -// the allocation fails, and the other builtins that had the same bug. - -function shouldBe(actual, expected) { - if (actual !== expected) - throw new Error(`bad value: expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); -} - -function shouldThrowOutOfMemory(func) { - let error = null; - try { - func(); - } catch (e) { - error = e; - } - if (String(error) !== "RangeError: Out of memory") - throw new Error(`bad error: ${String(error)}`); -} - -function replaceAll(string, search, replacement) { return string.replaceAll(search, replacement); } -noInline(replaceAll); - -// Warm this up so that the call below also goes through the DFG's StringReplaceAll node. -for (let i = 0; i < testLoopCount; ++i) - shouldBe(replaceAll("banana" + i, "a", "o"), "bonono" + i); - -const string = "a".repeat(2 ** 28); - -shouldThrowOutOfMemory(() => string.replaceAll("a", "c")); -shouldThrowOutOfMemory(() => replaceAll(string, "a", "c")); - -// An empty search string matches before every character and at the end. -shouldThrowOutOfMemory(() => string.replaceAll("", "c")); - -// 2^27 matches fit. -{ - const result = string.substring(2 ** 27).replaceAll("a", "c"); - shouldBe(result.length, 2 ** 27); - shouldBe(result[0], "c"); - shouldBe(result[2 ** 27 - 1], "c"); - shouldBe(result.indexOf("a"), -1); -} diff --git a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp index 1aeac0c213e8a..d178a1cff5380 100644 --- a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp +++ b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp @@ -140,7 +140,11 @@ void JSFinalizationRegistry::reconcileWeakReferencesAtGCEnd(VM& vm, CollectionSc } if (keyIsDead) { - m_noUnregistrationLive.append(reg); + // registerTarget() can leave this list full, and nothing can throw at the end of a collection. + // A registration that does not fit is dropped: its cleanup callback never runs, which the + // specification allows. + bool appended = m_noUnregistrationLive.tryAppend(reg); + UNUSED_VARIABLE(appended); return true; } diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp index dd9262133d781..68ca102f69662 100644 --- a/Source/JavaScriptCore/runtime/StringPrototype.cpp +++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp @@ -1081,6 +1081,12 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt auto& result = vm.stringSplitIndice; result.shrink(0); + // result keeps its capacity for the next call. A buffer that could not grow is not worth keeping. + auto throwOutOfMemory = [&]() -> JSCell* { + result.clear(); + throwOutOfMemoryError(globalObject, scope); + return nullptr; + }; constexpr unsigned atomStringsArrayLimit = 100; const bool subjectIsAtom = input->impl()->isAtom(); @@ -1216,10 +1222,8 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt SplitStatus status = stringImpl->is8Bit() ? splitStringByOneCharacterImpl(result, stringImpl, separatorCharacter, limit) : splitStringByOneCharacterImpl(result, stringImpl, separatorCharacter, limit); - if (status == SplitStatus::OutOfMemory) [[unlikely]] { - throwOutOfMemoryError(globalObject, scope); - return { }; - } + if (status == SplitStatus::OutOfMemory) [[unlikely]] + return throwOutOfMemory(); if (status == SplitStatus::ReachedLimit) RELEASE_AND_RETURN(scope, cacheAndCreateArray()); } else { @@ -1234,10 +1238,8 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt // 1. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive) // through q (exclusive). // 2. Call CreateDataProperty(A, ToString(lengthA), T). - if (!result.tryAppend(matchPosition)) [[unlikely]] { - throwOutOfMemoryError(globalObject, scope); - return { }; - } + if (!result.tryAppend(matchPosition)) [[unlikely]] + return throwOutOfMemory(); // 3. Increment lengthA by 1. // 4. If lengthA == lim, return A. if (result.size() == limit) @@ -1252,10 +1254,8 @@ JSCell* stringSplitFast(JSGlobalObject* globalObject, JSString* thisString, JSSt // 15. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive) // through s (exclusive). // 16. Call CreateDataProperty(A, ToString(lengthA), T). - if (!result.tryAppend(input->length())) [[unlikely]] { - throwOutOfMemoryError(globalObject, scope); - return { }; - } + if (!result.tryAppend(input->length())) [[unlikely]] + return throwOutOfMemory(); RELEASE_AND_RETURN(scope, cacheAndCreateArray()); } From 5074c2a1648193cbff2417d7206a9a3c1675e30c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:47:18 +0000 Subject: [PATCH 3/3] [JSC] FinalizationRegistry: every list that grows at the end of a collection drops what does not fit reconcileWeakReferencesAtGCEnd() appends to four Vectors: the registrations whose token died, and the held values of the registrations whose target died (one list for the registrations without a live token, one per live token). Nothing can throw there. The first already dropped a registration that does not fit. The lists of held values now do the same, so none of them ends the process when it cannot grow. A held value that is dropped means a cleanup callback that never runs, which the specification allows. A list of held values that could not take its first value is removed again, because takeDeadHoldingsValue() expects every list to hold a value. m_deadRegistrations.add() and m_liveRegistrations.add() stay as they are: a HashMap has no fallible add(). * JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js: * Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp: (JSC::JSFinalizationRegistry::reconcileWeakReferencesAtGCEnd): --- ...ry-when-script-sized-vector-cannot-grow.js | 40 +++++++++++++++++++ .../runtime/JSFinalizationRegistry.cpp | 25 ++++++------ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js b/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js index 1c80b1d554e81..6269c0218f5ea 100644 --- a/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js +++ b/JSTests/stress/out-of-memory-when-script-sized-vector-cannot-grow.js @@ -85,6 +85,46 @@ shouldBe(JSON.stringify(JSON.parse("[1,[2,3]]", (key, value) => value)), "[1,[2, shouldBe(registry.unregister(token), true); } +// The end of a collection also moves the held values of the registrations whose target died to a second +// list. What does not fit there is dropped too. One list of registrations stops below the size that this +// second list stops at, so the registrations are spread over many tokens, each with its own target. The +// second list overflows even when the last few of them are still reachable at the collection. +{ + const registry = new FinalizationRegistry(() => { }); + (function () { + for (let t = 0; t < 30; ++t) { + const target = { }; + const token = { }; + for (let i = 0; i < 5000; ++i) + registry.register(target, 1, token); + } + })(); + fullGC(); + const target = { }; + const token = { }; + registry.register(target, 1, token); + shouldBe(registry.unregister(token), true); +} + +// While a token is alive, the held values move to a list of that token, so that unregister() still finds +// them. No cleanup callback runs before this script ends, so that list fills up over several collections. +{ + const registry = new FinalizationRegistry(() => { }); + const token = { }; + for (let round = 0; round < 7; ++round) { + (function () { + for (let t = 0; t < 6; ++t) { + const target = { }; + for (let i = 0; i < 5000; ++i) + registry.register(target, 1, token); + } + })(); + fullGC(); + } + shouldBe(registry.unregister(token), true); + shouldBe(registry.unregister(token), false); +} + // Intl.ListFormat: every string of the iterable. { const listFormat = new Intl.ListFormat("en"); diff --git a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp index d178a1cff5380..6ed166f345c72 100644 --- a/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp +++ b/Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp @@ -106,12 +106,13 @@ void JSFinalizationRegistry::reconcileWeakReferencesAtGCEnd(VM& vm, CollectionSc RELEASE_ASSERT(iter.value.size()); #endif + // Nothing can throw at the end of a collection, and registerTarget() can leave a list full. What does not fit + // in its list is dropped: that cleanup callback never runs, which the specification allows. bool readiedCell = false; m_noUnregistrationLive.removeAllMatching([&] (const Registration& reg) { ASSERT(!reg.holdings.get().isCell() || vm.heap.isMarked(reg.holdings.get().asCell())); if (!vm.heap.isMarked(reg.target)) { - m_noUnregistrationDead.append(reg.holdings); - readiedCell = true; + readiedCell |= m_noUnregistrationDead.tryAppend(reg.holdings); return true; } return false; @@ -122,27 +123,27 @@ void JSFinalizationRegistry::reconcileWeakReferencesAtGCEnd(VM& vm, CollectionSc bool keyIsDead = !vm.heap.isMarked(bucket.key); DeadRegistrations* deadList = nullptr; - auto getDeadList = [&] () -> DeadRegistrations& { + auto tryAppendToDeadList = [&] (const WriteBarrier& holdings) -> bool { if (!deadList) [[unlikely]] deadList = &m_deadRegistrations.add(bucket.key, DeadRegistrations()).iterator->value; - return *deadList; + if (deadList->tryAppend(holdings)) [[likely]] + return true; + // takeDeadHoldingsValue() expects every bucket to hold a value. + if (deadList->isEmpty()) { + m_deadRegistrations.remove(bucket.key); + deadList = nullptr; + } + return false; }; bucket.value.removeAllMatching([&] (const Registration& reg) { ASSERT(!reg.holdings.get().isCell() || vm.heap.isMarked(reg.holdings.get().asCell())); if (!vm.heap.isMarked(reg.target)) { - if (keyIsDead) - m_noUnregistrationDead.append(reg.holdings); - else - getDeadList().append(reg.holdings); - readiedCell = true; + readiedCell |= keyIsDead ? m_noUnregistrationDead.tryAppend(reg.holdings) : tryAppendToDeadList(reg.holdings); return true; } if (keyIsDead) { - // registerTarget() can leave this list full, and nothing can throw at the end of a collection. - // A registration that does not fit is dropped: its cleanup callback never runs, which the - // specification allows. bool appended = m_noUnregistrationLive.tryAppend(reg); UNUSED_VARIABLE(appended); return true;