Skip to content

Commit

Permalink
Throw exception when ClonedArguments allocation fails
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264511
rdar://118039984

Reviewed by Mark Lam.

Currently, if we try and allocate a ClonedArguments object and run out of
memory, we silently return nullptr. This can result in the creation of an
empty JSValue being returned. This patch ensures that we check for and
propagate the null result, in addition to throwing an OutOfMemory error.
In cases where we can't throw an OutOfMemory error, specifically in
operationMaterializeObjectInOSR, we RELEASE_ASSERT that the result is
non-null to guarantee we crash instead of allowing the empty value to
escape.

* JSTests/stress/cloned-arguments-oom.js: Added.
(Allocator):
(Allocator.prototype.size):
(Allocator.prototype.allocate):
(createClonedArguments):
(0x0.map.size.new.Allocator):
(catch):
* Source/JavaScriptCore/dfg/DFGOperations.cpp:
(JSC::DFG::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/ftl/FTLOperations.cpp:
(JSC::FTL::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/runtime/ClonedArguments.cpp:
(JSC::ClonedArguments::createEmpty):
(JSC::ClonedArguments::createWithInlineFrame):
(JSC::ClonedArguments::createWithMachineFrame):
(JSC::ClonedArguments::createByCopyingFrom):
* Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:
(JSC::JSC_DEFINE_COMMON_SLOW_PATH):
* Source/JavaScriptCore/runtime/FunctionPrototype.cpp:
(JSC::JSC_DEFINE_CUSTOM_GETTER):

Originally-landed-as: 267815.638@safari-7617-branch (dc9b30f). rdar://121478772
Canonical link: https://commits.webkit.org/273483@main
  • Loading branch information
ddegazio authored and robert-jenner committed Jan 25, 2024
1 parent 17ee3e3 commit 9c7c233
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 9 deletions.
6 changes: 5 additions & 1 deletion Source/JavaScriptCore/dfg/DFGOperations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2106,7 +2106,11 @@ JSC_DEFINE_JIT_OPERATION(operationCreateClonedArguments, JSCell*, (JSGlobalObjec
VM& vm = globalObject->vm();
CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
return ClonedArguments::createByCopyingFrom(globalObject, structure, argumentStart, length, callee, butterfly);
auto scope = DECLARE_THROW_SCOPE(vm);

JSCell* result = ClonedArguments::createByCopyingFrom(globalObject, structure, argumentStart, length, callee, butterfly);
EXCEPTION_ASSERT_UNUSED(scope, scope.exception() || result);
return result;
}

JSC_DEFINE_JIT_OPERATION(operationCreateDirectArgumentsDuringExit, JSCell*, (VM* vmPointer, InlineCallFrame* inlineCallFrame, JSFunction* callee, uint32_t argumentCount))
Expand Down
10 changes: 7 additions & 3 deletions Source/JavaScriptCore/ftl/FTLOperations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,11 @@ JSC_DEFINE_JIT_OPERATION(operationMaterializeObjectInOSR, JSCell*, (JSGlobalObje
switch (materialization->type()) {
case PhantomDirectArguments:
return DirectArguments::createByCopying(globalObject, callFrame);
case PhantomClonedArguments:
return ClonedArguments::createWithMachineFrame(globalObject, callFrame, ArgumentsMode::Cloned);
case PhantomClonedArguments: {
ClonedArguments* result = ClonedArguments::createWithMachineFrame(globalObject, callFrame, ArgumentsMode::Cloned);
RELEASE_ASSERT(result);
return result;
}
case PhantomCreateRest: {
CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(
materialization->origin(), callFrame->codeBlock()->baselineAlternative());
Expand Down Expand Up @@ -467,7 +470,8 @@ JSC_DEFINE_JIT_OPERATION(operationMaterializeObjectInOSR, JSCell*, (JSGlobalObje
case PhantomClonedArguments: {
unsigned length = argumentCount - 1;
ClonedArguments* result = ClonedArguments::createEmpty(vm, codeBlock->globalObject()->clonedArgumentsStructure(), callee, length, nullptr);

RELEASE_ASSERT(result);

for (unsigned i = materialization->properties().size(); i--;) {
const ExitPropertyValue& property = materialization->properties()[i];
if (property.location().kind() != ArgumentPLoc)
Expand Down
17 changes: 14 additions & 3 deletions Source/JavaScriptCore/runtime/ClonedArguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ ClonedArguments::ClonedArguments(VM& vm, Structure* structure, Butterfly* butter

ClonedArguments* ClonedArguments::createEmpty(VM& vm, Structure* structure, JSFunction* callee, unsigned length, Butterfly* butterfly)
{
auto scope = DECLARE_THROW_SCOPE(vm);
unsigned vectorLength = length;
if (vectorLength > MAX_STORAGE_VECTOR_LENGTH)
return nullptr;
Expand All @@ -57,8 +58,10 @@ ClonedArguments* ClonedArguments::createEmpty(VM& vm, Structure* structure, JSFu
indexingHeader.setVectorLength(vectorLength);
indexingHeader.setPublicLength(length);
butterfly = Butterfly::tryCreate(vm, nullptr, 0, structure->outOfLineCapacity(), true, indexingHeader, vectorLength * sizeof(EncodedJSValue));
if (!butterfly)
if (UNLIKELY(!butterfly)) {
throwOutOfMemoryError(structure->globalObject(), scope);
return nullptr;
}
}

for (unsigned i = length; i < vectorLength; ++i)
Expand Down Expand Up @@ -91,7 +94,7 @@ ClonedArguments* ClonedArguments::createWithInlineFrame(JSGlobalObject* globalOb
// NB. Some clients might expect that the global object of of this object is the global object
// of the callee. We don't do this for now, but maybe we should.
ClonedArguments* result = ClonedArguments::createEmpty(vm, globalObject->clonedArgumentsStructure(), callee, length, nullptr);
ASSERT(!result->needsSlowPutIndexing() || shouldUseSlowPut(result->structure()->indexingType()));
ASSERT(!result || !result->needsSlowPutIndexing() || shouldUseSlowPut(result->structure()->indexingType()));
return result;
};

Expand All @@ -105,12 +108,16 @@ ClonedArguments* ClonedArguments::createWithInlineFrame(JSGlobalObject* globalOb
length = inlineCallFrame->argumentCountIncludingThis;
length--;
result = createEmptyWithAssert(globalObject, callee, length);
if (!result)
return result;

for (unsigned i = length; i--;)
result->putDirectIndex(globalObject, i, inlineCallFrame->m_argumentsWithFixup[i + 1].recover(targetFrame));
} else {
length = targetFrame->argumentCount();
result = createEmptyWithAssert(globalObject, callee, length);
if (!result)
return result;

for (unsigned i = length; i--;)
result->putDirectIndex(globalObject, i, targetFrame->uncheckedArgument(i));
Expand All @@ -120,6 +127,8 @@ ClonedArguments* ClonedArguments::createWithInlineFrame(JSGlobalObject* globalOb

case ArgumentsMode::FakeValues: {
result = createEmptyWithAssert(globalObject, callee, 0);
if (!result)
return result;
break;
} }

Expand All @@ -131,14 +140,16 @@ ClonedArguments* ClonedArguments::createWithInlineFrame(JSGlobalObject* globalOb
ClonedArguments* ClonedArguments::createWithMachineFrame(JSGlobalObject* globalObject, CallFrame* targetFrame, ArgumentsMode mode)
{
ClonedArguments* result = createWithInlineFrame(globalObject, targetFrame, nullptr, mode);
ASSERT(!result->needsSlowPutIndexing() || shouldUseSlowPut(result->structure()->indexingType()));
ASSERT(!result || !result->needsSlowPutIndexing() || shouldUseSlowPut(result->structure()->indexingType()));
return result;
}

ClonedArguments* ClonedArguments::createByCopyingFrom(JSGlobalObject* globalObject, Structure* structure, Register* argumentStart, unsigned length, JSFunction* callee, Butterfly* butterfly)
{
VM& vm = globalObject->vm();
ClonedArguments* result = createEmpty(vm, structure, callee, length, butterfly);
if (!result)
return result;

for (unsigned i = length; i--;)
result->putDirectIndex(globalObject, i, argumentStart[i].jsValue());
Expand Down
4 changes: 3 additions & 1 deletion Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ JSC_DEFINE_COMMON_SLOW_PATH(slow_path_create_cloned_arguments)
{
BEGIN();
auto bytecode = pc->as<OpCreateClonedArguments>();
RETURN(ClonedArguments::createWithMachineFrame(globalObject, callFrame, ArgumentsMode::Cloned));
auto result = ClonedArguments::createWithMachineFrame(globalObject, callFrame, ArgumentsMode::Cloned);
EXCEPTION_ASSERT(throwScope.exception() || result);
RETURN(result);
}

JSC_DEFINE_COMMON_SLOW_PATH(slow_path_create_this)
Expand Down
4 changes: 3 additions & 1 deletion Source/JavaScriptCore/runtime/FunctionPrototype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ JSC_DEFINE_CUSTOM_GETTER(argumentsGetter, (JSGlobalObject* globalObject, Encoded
if (!thisObj || !isAllowedReceiverFunctionForCallerAndArguments(thisObj))
return throwVMTypeError(globalObject, scope, RestrictedPropertyAccessError);

return JSValue::encode(retrieveArguments(vm, vm.topCallFrame, thisObj));
JSValue result = retrieveArguments(vm, vm.topCallFrame, thisObj);
EXCEPTION_ASSERT(scope.exception() || result);
return JSValue::encode(result);
}

class RetrieveCallerFunctionFunctor {
Expand Down

0 comments on commit 9c7c233

Please sign in to comment.