Skip to content

Commit

Permalink
[JSC] Throw OOM error if constructArrayNegativeIndexed() fails to all…
Browse files Browse the repository at this point in the history
…ocate

https://bugs.webkit.org/show_bug.cgi?id=260559
<rdar://114202373>

Reviewed by Mark Lam.

This change leverages AllocationFailureMode to throw an OOM error if constructArrayNegativeIndexed()
fails to allocate an array, which does happen in the wild (iOS apps).

All clients of constructArrayNegativeIndexed() were updated to correctly handle thrown exception.

* Source/JavaScriptCore/runtime/JSArray.cpp:
(JSC::constructArray):
(JSC::constructArrayNegativeIndexed):
* Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h:
(JSC::constructArrayNegativeIndexed):

Canonical link: https://commits.webkit.org/267300@main
  • Loading branch information
Alexey Shvayka committed Aug 25, 2023
1 parent 2fab0f5 commit ddd9cbc
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
18 changes: 14 additions & 4 deletions Source/JavaScriptCore/runtime/JSArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,7 @@ bool JSArray::isIteratorProtocolFastAndNonObservable()
return true;
}

template<AllocationFailureMode failureMode>
inline JSArray* constructArray(ObjectInitializationScope& scope, Structure* arrayStructure, unsigned length)
{
JSArray* array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
Expand All @@ -1357,7 +1358,10 @@ inline JSArray* constructArray(ObjectInitializationScope& scope, Structure* arra
// when making this change we should check that all clients of this
// function will correctly handle an exception being thrown from here.
// https://bugs.webkit.org/show_bug.cgi?id=169786
RELEASE_ASSERT(array);
if constexpr (failureMode == AllocationFailureMode::Assert)
RELEASE_ASSERT(array);
else if (!array)
return nullptr;

// FIXME: We only need this for subclasses of Array because we might need to allocate a new structure to change
// indexing types while initializing. If this triggered a GC then we might scan our currently uninitialized
Expand All @@ -1374,7 +1378,7 @@ JSArray* constructArray(JSGlobalObject* globalObject, Structure* arrayStructure,
unsigned length = values.size();
ObjectInitializationScope scope(vm);

JSArray* array = constructArray(scope, arrayStructure, length);
JSArray* array = constructArray<AllocationFailureMode::Assert>(scope, arrayStructure, length);
for (unsigned i = 0; i < length; ++i)
array->initializeIndex(scope, i, values.at(i));
return array;
Expand All @@ -1385,7 +1389,7 @@ JSArray* constructArray(JSGlobalObject* globalObject, Structure* arrayStructure,
VM& vm = globalObject->vm();
ObjectInitializationScope scope(vm);

JSArray* array = constructArray(scope, arrayStructure, length);
JSArray* array = constructArray<AllocationFailureMode::Assert>(scope, arrayStructure, length);
for (unsigned i = 0; i < length; ++i)
array->initializeIndex(scope, i, values[i]);
return array;
Expand All @@ -1394,9 +1398,15 @@ JSArray* constructArray(JSGlobalObject* globalObject, Structure* arrayStructure,
JSArray* constructArrayNegativeIndexed(JSGlobalObject* globalObject, Structure* arrayStructure, const JSValue* values, unsigned length)
{
VM& vm = globalObject->vm();
auto throwScope = DECLARE_THROW_SCOPE(vm);
ObjectInitializationScope scope(vm);

JSArray* array = constructArray(scope, arrayStructure, length);
JSArray* array = constructArray<AllocationFailureMode::ReturnNull>(scope, arrayStructure, length);
if (UNLIKELY(!array)) {
throwOutOfMemoryError(globalObject, throwScope);
return nullptr;
}

for (int i = 0; i < static_cast<int>(length); ++i)
array->initializeIndex(scope, i, values[-i]);
return array;
Expand Down
6 changes: 5 additions & 1 deletion Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,11 @@ inline JSArray* constructArrayNegativeIndexed(JSGlobalObject* globalObject, Arra
auto scope = DECLARE_THROW_SCOPE(vm);
Structure* structure = globalObject->arrayStructureForProfileDuringAllocation(globalObject, profile, newTarget);
RETURN_IF_EXCEPTION(scope, nullptr);
return ArrayAllocationProfile::updateLastAllocationFor(profile, constructArrayNegativeIndexed(globalObject, structure, values, length));
scope.release();
JSArray* array = constructArrayNegativeIndexed(globalObject, structure, values, length);
if (UNLIKELY(!array))
return nullptr;
return ArrayAllocationProfile::updateLastAllocationFor(profile, array);
}

inline OptionSet<CodeGenerationMode> JSGlobalObject::defaultCodeGenerationMode() const
Expand Down

0 comments on commit ddd9cbc

Please sign in to comment.