Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (256086 => 256087)
--- trunk/Source/_javascript_Core/ChangeLog 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-02-08 06:08:33 UTC (rev 256087)
@@ -1,3 +1,29 @@
+2020-02-07 Robin Morisset <[email protected]>
+
+ Throw OutOfMemory exception instead of crashing if DirectArguments/ScopedArguments can't be created
+ https://bugs.webkit.org/show_bug.cgi?id=207423
+
+ Reviewed by Mark Lam.
+
+ AllocationFailureMode::Assert is problematic because fuzzers keep producing spurious error reports when they generate code that tries allocating infinite amount of memory.
+ The right approach is to use AllocationFailureMode::ReturnNull, and throw a JS exception upon receiving null.
+
+ In this patch I fixed two functions that were using AllocationFailureMode::Assert:
+ DirectArguments::DirectArguments::overrideThings
+ GenericArguments<Type>::initModifiedArgumentsDescriptor
+
+ No test added, because the only test we have is highly non-deterministic/flaky (only triggers about 10 to 20% of the time even before the fix).
+
+ * runtime/DirectArguments.h:
+ * runtime/GenericArguments.h:
+ * runtime/GenericArgumentsInlines.h:
+ (JSC::GenericArguments<Type>::deletePropertyByIndex):
+ (JSC::GenericArguments<Type>::defineOwnProperty):
+ (JSC::GenericArguments<Type>::initModifiedArgumentsDescriptor):
+ (JSC::GenericArguments<Type>::initModifiedArgumentsDescriptorIfNecessary):
+ (JSC::GenericArguments<Type>::setModifiedArgumentDescriptor):
+ * runtime/ScopedArguments.h:
+
2020-02-07 Ryan Haddad <[email protected]>
Unreviewed, rolling out r256051.
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2020-02-08 06:08:33 UTC (rev 256087)
@@ -987,6 +987,7 @@
CHECK_EXCEPTION();
couldDelete = baseObject->methodTable(vm)->deleteProperty(baseObject, globalObject, property);
}
+ CHECK_EXCEPTION();
if (!couldDelete && codeBlock->isStrictMode())
THROW(createTypeError(globalObject, UnableToDeletePropertyError));
Modified: trunk/Source/_javascript_Core/runtime/DirectArguments.cpp (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/DirectArguments.cpp 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/DirectArguments.cpp 2020-02-08 06:08:33 UTC (rev 256087)
@@ -110,15 +110,22 @@
return Structure::create(vm, globalObject, prototype, TypeInfo(DirectArgumentsType, StructureFlags), info());
}
-void DirectArguments::overrideThings(VM& vm)
+void DirectArguments::overrideThings(JSGlobalObject* globalObject)
{
+ VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
RELEASE_ASSERT(!m_mappedArguments);
putDirect(vm, vm.propertyNames->length, jsNumber(m_length), static_cast<unsigned>(PropertyAttribute::DontEnum));
putDirect(vm, vm.propertyNames->callee, m_callee.get(), static_cast<unsigned>(PropertyAttribute::DontEnum));
- putDirect(vm, vm.propertyNames->iteratorSymbol, globalObject(vm)->arrayProtoValuesFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
+ putDirect(vm, vm.propertyNames->iteratorSymbol, globalObject->arrayProtoValuesFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
- void* backingStore = vm.gigacageAuxiliarySpace(m_mappedArguments.kind).allocateNonVirtual(vm, mappedArgumentsSize(), nullptr, AllocationFailureMode::Assert);
+ void* backingStore = vm.gigacageAuxiliarySpace(m_mappedArguments.kind).allocateNonVirtual(vm, mappedArgumentsSize(), nullptr, AllocationFailureMode::ReturnNull);
+ if (UNLIKELY(!backingStore)) {
+ throwOutOfMemoryError(globalObject, scope);
+ return;
+ }
bool* overrides = static_cast<bool*>(backingStore);
m_mappedArguments.set(vm, this, overrides, internalLength());
for (unsigned i = internalLength(); i--;)
@@ -125,15 +132,20 @@
overrides[i] = false;
}
-void DirectArguments::overrideThingsIfNecessary(VM& vm)
+void DirectArguments::overrideThingsIfNecessary(JSGlobalObject* globalObject)
{
if (!m_mappedArguments)
- overrideThings(vm);
+ overrideThings(globalObject);
}
-void DirectArguments::unmapArgument(VM& vm, unsigned index)
+void DirectArguments::unmapArgument(JSGlobalObject* globalObject, unsigned index)
{
- overrideThingsIfNecessary(vm);
+ VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ overrideThingsIfNecessary(globalObject);
+ RETURN_IF_EXCEPTION(scope, void());
+
m_mappedArguments.at(index, internalLength()) = true;
}
Modified: trunk/Source/_javascript_Core/runtime/DirectArguments.h (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/DirectArguments.h 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/DirectArguments.h 2020-02-08 06:08:33 UTC (rev 256087)
@@ -125,18 +125,18 @@
// Methods intended for use by the GenericArguments mixin.
bool overrodeThings() const { return !!m_mappedArguments; }
- void overrideThings(VM&);
- void overrideThingsIfNecessary(VM&);
- void unmapArgument(VM&, unsigned index);
+ void overrideThings(JSGlobalObject*);
+ void overrideThingsIfNecessary(JSGlobalObject*);
+ void unmapArgument(JSGlobalObject*, unsigned index);
- void initModifiedArgumentsDescriptorIfNecessary(VM& vm)
+ void initModifiedArgumentsDescriptorIfNecessary(JSGlobalObject* globalObject)
{
- GenericArguments<DirectArguments>::initModifiedArgumentsDescriptorIfNecessary(vm, m_length);
+ GenericArguments<DirectArguments>::initModifiedArgumentsDescriptorIfNecessary(globalObject, m_length);
}
- void setModifiedArgumentDescriptor(VM& vm, unsigned index)
+ void setModifiedArgumentDescriptor(JSGlobalObject* globalObject, unsigned index)
{
- GenericArguments<DirectArguments>::setModifiedArgumentDescriptor(vm, index, m_length);
+ GenericArguments<DirectArguments>::setModifiedArgumentDescriptor(globalObject, index, m_length);
}
bool isModifiedArgumentDescriptor(unsigned index)
Modified: trunk/Source/_javascript_Core/runtime/GenericArguments.h (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/GenericArguments.h 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/GenericArguments.h 2020-02-08 06:08:33 UTC (rev 256087)
@@ -54,9 +54,9 @@
static bool deletePropertyByIndex(JSCell*, JSGlobalObject*, unsigned propertyName);
static bool defineOwnProperty(JSObject*, JSGlobalObject*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
- void initModifiedArgumentsDescriptor(VM&, unsigned length);
- void initModifiedArgumentsDescriptorIfNecessary(VM&, unsigned length);
- void setModifiedArgumentDescriptor(VM&, unsigned index, unsigned length);
+ void initModifiedArgumentsDescriptor(JSGlobalObject*, unsigned length);
+ void initModifiedArgumentsDescriptorIfNecessary(JSGlobalObject*, unsigned length);
+ void setModifiedArgumentDescriptor(JSGlobalObject*, unsigned index, unsigned length);
bool isModifiedArgumentDescriptor(unsigned index, unsigned length);
void copyToArguments(JSGlobalObject*, JSValue* firstElementDest, unsigned offset, unsigned length);
Modified: trunk/Source/_javascript_Core/runtime/GenericArgumentsInlines.h (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/GenericArgumentsInlines.h 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/GenericArgumentsInlines.h 2020-02-08 06:08:33 UTC (rev 256087)
@@ -117,14 +117,16 @@
{
Type* thisObject = jsCast<Type*>(cell);
VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
if (!thisObject->overrodeThings()
&& (ident == vm.propertyNames->length
|| ident == vm.propertyNames->callee
|| ident == vm.propertyNames->iteratorSymbol)) {
- thisObject->overrideThings(vm);
+ thisObject->overrideThings(globalObject);
+ RETURN_IF_EXCEPTION(scope, false);
PutPropertySlot dummy = slot; // This put is not cacheable, so we shadow the slot that was given to us.
- return Base::put(thisObject, globalObject, ident, value, dummy);
+ RELEASE_AND_RETURN(scope, Base::put(thisObject, globalObject, ident, value, dummy));
}
// https://tc39.github.io/ecma262/#sec-arguments-exotic-objects-set-p-v-receiver
@@ -138,7 +140,7 @@
return true;
}
- return Base::put(thisObject, globalObject, ident, value, slot);
+ RELEASE_AND_RETURN(scope, Base::put(thisObject, globalObject, ident, value, slot));
}
template<typename Type>
@@ -160,25 +162,30 @@
{
Type* thisObject = jsCast<Type*>(cell);
VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
if (!thisObject->overrodeThings()
&& (ident == vm.propertyNames->length
|| ident == vm.propertyNames->callee
- || ident == vm.propertyNames->iteratorSymbol))
- thisObject->overrideThings(vm);
-
+ || ident == vm.propertyNames->iteratorSymbol)) {
+ thisObject->overrideThings(globalObject);
+ RETURN_IF_EXCEPTION(scope, false);
+ }
+
if (Optional<uint32_t> index = parseIndex(ident))
- return GenericArguments<Type>::deletePropertyByIndex(thisObject, globalObject, *index);
+ RELEASE_AND_RETURN(scope, GenericArguments<Type>::deletePropertyByIndex(thisObject, globalObject, *index));
- return Base::deleteProperty(thisObject, globalObject, ident);
+ RELEASE_AND_RETURN(scope, Base::deleteProperty(thisObject, globalObject, ident));
}
template<typename Type>
bool GenericArguments<Type>::deletePropertyByIndex(JSCell* cell, JSGlobalObject* globalObject, unsigned index)
{
- Type* thisObject = jsCast<Type*>(cell);
VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+ Type* thisObject = jsCast<Type*>(cell);
+
bool propertyMightBeInJSObjectStorage = thisObject->isModifiedArgumentDescriptor(index) || !thisObject->isMappedArgument(index);
bool deletedProperty = true;
if (propertyMightBeInJSObjectStorage)
@@ -188,9 +195,11 @@
// Deleting an indexed property unconditionally unmaps it.
if (thisObject->isMappedArgument(index)) {
// We need to check that the property was mapped so we don't write to random memory.
- thisObject->unmapArgument(vm, index);
+ thisObject->unmapArgument(globalObject, index);
+ RETURN_IF_EXCEPTION(scope, true);
}
- thisObject->setModifiedArgumentDescriptor(vm, index);
+ thisObject->setModifiedArgumentDescriptor(globalObject, index);
+ RETURN_IF_EXCEPTION(scope, true);
}
return deletedProperty;
@@ -205,9 +214,10 @@
if (ident == vm.propertyNames->length
|| ident == vm.propertyNames->callee
- || ident == vm.propertyNames->iteratorSymbol)
- thisObject->overrideThingsIfNecessary(vm);
- else {
+ || ident == vm.propertyNames->iteratorSymbol) {
+ thisObject->overrideThingsIfNecessary(globalObject);
+ RETURN_IF_EXCEPTION(scope, false);
+ } else {
Optional<uint32_t> optionalIndex = parseIndex(ident);
if (optionalIndex) {
uint32_t index = optionalIndex.value();
@@ -230,7 +240,8 @@
object->putDirectMayBeIndex(globalObject, ident, value);
scope.assertNoException();
- thisObject->setModifiedArgumentDescriptor(vm, index);
+ thisObject->setModifiedArgumentDescriptor(globalObject, index);
+ RETURN_IF_EXCEPTION(scope, false);
}
}
@@ -246,8 +257,10 @@
object->putDirectMayBeIndex(globalObject, ident, value);
scope.assertNoException();
}
- thisObject->unmapArgument(vm, index);
- thisObject->setModifiedArgumentDescriptor(vm, index);
+ thisObject->unmapArgument(globalObject, index);
+ RETURN_IF_EXCEPTION(scope, false);
+ thisObject->setModifiedArgumentDescriptor(globalObject, index);
+ RETURN_IF_EXCEPTION(scope, false);
}
}
}
@@ -258,12 +271,19 @@
}
template<typename Type>
-void GenericArguments<Type>::initModifiedArgumentsDescriptor(VM& vm, unsigned argsLength)
+void GenericArguments<Type>::initModifiedArgumentsDescriptor(JSGlobalObject* globalObject, unsigned argsLength)
{
+ VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
RELEASE_ASSERT(!m_modifiedArgumentsDescriptor);
if (argsLength) {
- void* backingStore = vm.gigacageAuxiliarySpace(m_modifiedArgumentsDescriptor.kind).allocateNonVirtual(vm, WTF::roundUpToMultipleOf<8>(argsLength), nullptr, AllocationFailureMode::Assert);
+ void* backingStore = vm.gigacageAuxiliarySpace(m_modifiedArgumentsDescriptor.kind).allocateNonVirtual(vm, WTF::roundUpToMultipleOf<8>(argsLength), nullptr, AllocationFailureMode::ReturnNull);
+ if (UNLIKELY(!backingStore)) {
+ throwOutOfMemoryError(globalObject, scope);
+ return;
+ }
bool* modifiedArguments = static_cast<bool*>(backingStore);
m_modifiedArgumentsDescriptor.set(vm, this, modifiedArguments, argsLength);
for (unsigned i = argsLength; i--;)
@@ -272,16 +292,20 @@
}
template<typename Type>
-void GenericArguments<Type>::initModifiedArgumentsDescriptorIfNecessary(VM& vm, unsigned argsLength)
+void GenericArguments<Type>::initModifiedArgumentsDescriptorIfNecessary(JSGlobalObject* globalObject, unsigned argsLength)
{
if (!m_modifiedArgumentsDescriptor)
- initModifiedArgumentsDescriptor(vm, argsLength);
+ initModifiedArgumentsDescriptor(globalObject, argsLength);
}
template<typename Type>
-void GenericArguments<Type>::setModifiedArgumentDescriptor(VM& vm, unsigned index, unsigned length)
+void GenericArguments<Type>::setModifiedArgumentDescriptor(JSGlobalObject* globalObject, unsigned index, unsigned length)
{
- initModifiedArgumentsDescriptorIfNecessary(vm, length);
+ VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ initModifiedArgumentsDescriptorIfNecessary(globalObject, length);
+ RETURN_IF_EXCEPTION(scope, void());
if (index < length)
m_modifiedArgumentsDescriptor.at(index, length) = true;
}
Modified: trunk/Source/_javascript_Core/runtime/ScopedArguments.cpp (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/ScopedArguments.cpp 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/ScopedArguments.cpp 2020-02-08 06:08:33 UTC (rev 256087)
@@ -121,25 +121,28 @@
return Structure::create(vm, globalObject, prototype, TypeInfo(ScopedArgumentsType, StructureFlags), info());
}
-void ScopedArguments::overrideThings(VM& vm)
+void ScopedArguments::overrideThings(JSGlobalObject* globalObject)
{
+ VM& vm = globalObject->vm();
+
RELEASE_ASSERT(!m_overrodeThings);
putDirect(vm, vm.propertyNames->length, jsNumber(m_table->length()), static_cast<unsigned>(PropertyAttribute::DontEnum));
putDirect(vm, vm.propertyNames->callee, m_callee.get(), static_cast<unsigned>(PropertyAttribute::DontEnum));
- putDirect(vm, vm.propertyNames->iteratorSymbol, globalObject(vm)->arrayProtoValuesFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
+ putDirect(vm, vm.propertyNames->iteratorSymbol, globalObject->arrayProtoValuesFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
m_overrodeThings = true;
}
-void ScopedArguments::overrideThingsIfNecessary(VM& vm)
+void ScopedArguments::overrideThingsIfNecessary(JSGlobalObject* globalObject)
{
if (!m_overrodeThings)
- overrideThings(vm);
+ overrideThings(globalObject);
}
-void ScopedArguments::unmapArgument(VM& vm, uint32_t i)
+void ScopedArguments::unmapArgument(JSGlobalObject* globalObject, uint32_t i)
{
+ VM& vm = globalObject->vm();
ASSERT_WITH_SECURITY_IMPLICATION(i < m_totalLength);
unsigned namedLength = m_table->length();
if (i < namedLength)
Modified: trunk/Source/_javascript_Core/runtime/ScopedArguments.h (256086 => 256087)
--- trunk/Source/_javascript_Core/runtime/ScopedArguments.h 2020-02-08 04:21:21 UTC (rev 256086)
+++ trunk/Source/_javascript_Core/runtime/ScopedArguments.h 2020-02-08 06:08:33 UTC (rev 256087)
@@ -123,18 +123,18 @@
}
bool overrodeThings() const { return m_overrodeThings; }
- void overrideThings(VM&);
- void overrideThingsIfNecessary(VM&);
- void unmapArgument(VM&, uint32_t index);
+ void overrideThings(JSGlobalObject*);
+ void overrideThingsIfNecessary(JSGlobalObject*);
+ void unmapArgument(JSGlobalObject*, uint32_t index);
- void initModifiedArgumentsDescriptorIfNecessary(VM& vm)
+ void initModifiedArgumentsDescriptorIfNecessary(JSGlobalObject* globalObject)
{
- GenericArguments<ScopedArguments>::initModifiedArgumentsDescriptorIfNecessary(vm, m_table->length());
+ GenericArguments<ScopedArguments>::initModifiedArgumentsDescriptorIfNecessary(globalObject, m_table->length());
}
- void setModifiedArgumentDescriptor(VM& vm, unsigned index)
+ void setModifiedArgumentDescriptor(JSGlobalObject* globalObject, unsigned index)
{
- GenericArguments<ScopedArguments>::setModifiedArgumentDescriptor(vm, index, m_table->length());
+ GenericArguments<ScopedArguments>::setModifiedArgumentDescriptor(globalObject, index, m_table->length());
}
bool isModifiedArgumentDescriptor(unsigned index)
