Clients of JSArray::tryCreateForInitializationPrivate() should do their own null...

https://bugs.webkit.org/show_bug.cgi?id=169783

Reviewed by Saam Barati.

JSTests:

* stress/regress-169783.js: Added.

Source/JavaScriptCore:

Fixed clients of tryCreateForInitializationPrivate() to do a null check and throw
an OutOfMemoryError if allocation fails, or RELEASE_ASSERT that the allocation
succeeds.

* dfg/DFGOperations.cpp:
* ftl/FTLOperations.cpp:
(JSC::FTL::operationMaterializeObjectInOSR):
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncSplice):
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/JSArray.cpp:
(JSC::JSArray::tryCreateForInitializationPrivate):
(JSC::JSArray::fastSlice):
* runtime/JSArray.h:
(JSC::constructArray):
(JSC::constructArrayNegativeIndexed):
* runtime/RegExpMatchesArray.cpp:
(JSC::createEmptyRegExpMatchesArray):
* runtime/RegExpMatchesArray.h:
(JSC::createRegExpMatchesArray):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214313 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index d73acdc..2407af8 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,12 @@
+2017-03-23  Mark Lam  <mark.lam@apple.com>
+
+        Clients of JSArray::tryCreateForInitializationPrivate() should do their own null checks.
+        https://bugs.webkit.org/show_bug.cgi?id=169783
+
+        Reviewed by Saam Barati.
+
+        * stress/regress-169783.js: Added.
+
 2017-03-22  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC][DFG] Propagate AnyIntAsDouble information carefully to utilize it in fixup

diff --git a/JSTests/stress/regress-169783.js b/JSTests/stress/regress-169783.js
new file mode 100644 (file)
index 0000000..6fd8563
--- /dev/null
+++ b/JSTests/stress/regress-169783.js
@@ -0,0 +1,20 @@
+//@ if $buildType == "debug" then runFTLNoCJIT("--maxSingleAllocationSize=10000000") else skip end
+
+function test(a) {
+    var x = [1337, ...a, ...a, ...a, ...a, ...a];
+}
+noInline(test);
+
+function doTest(a, shouldThrow) {
+    var exception;
+    try {
+        test(a);
+    } catch (e) {
+        exception = e;
+    }
+    if (shouldThrow && exception != "Error: Out of memory")
+        throw("FAILED");
+}
+
+var a = new Array(0x40000);
+doTest(a, true);

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 9839c7c..09d6598 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,32 @@
+2017-03-23  Mark Lam  <mark.lam@apple.com>
+
+        Clients of JSArray::tryCreateForInitializationPrivate() should do their own null checks.
+        https://bugs.webkit.org/show_bug.cgi?id=169783
+
+        Reviewed by Saam Barati.
+
+        Fixed clients of tryCreateForInitializationPrivate() to do a null check and throw
+        an OutOfMemoryError if allocation fails, or RELEASE_ASSERT that the allocation
+        succeeds.
+
+        * dfg/DFGOperations.cpp:
+        * ftl/FTLOperations.cpp:
+        (JSC::FTL::operationMaterializeObjectInOSR):
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncSplice):
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::tryCreateForInitializationPrivate):
+        (JSC::JSArray::fastSlice):
+        * runtime/JSArray.h:
+        (JSC::constructArray):
+        (JSC::constructArrayNegativeIndexed):
+        * runtime/RegExpMatchesArray.cpp:
+        (JSC::createEmptyRegExpMatchesArray):
+        * runtime/RegExpMatchesArray.h:
+        (JSC::createRegExpMatchesArray):
+
 2017-03-23  Guillaume Emont  <guijemont@igalia.com>
 
         [jsc] Add MacroAssemblerMIPS::storeFence()

diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
index 5c6aa43..f04aca5 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -2086,6 +2086,10 @@ JSCell* JIT_OPERATION operationNewArrayWithSpreadSlow(ExecState* exec, void* buf
     Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
 
     JSArray* result = JSArray::tryCreateForInitializationPrivate(vm, structure, length);
+    if (UNLIKELY(!result)) {
+        throwOutOfMemoryError(exec, scope);
+        return nullptr;
+    }
     RETURN_IF_EXCEPTION(scope, nullptr);
 
     unsigned index = 0;

diff --git a/Source/JavaScriptCore/ftl/FTLOperations.cpp b/Source/JavaScriptCore/ftl/FTLOperations.cpp
index 50f4717..1d06148 100644 (file)
--- a/Source/JavaScriptCore/ftl/FTLOperations.cpp
+++ b/Source/JavaScriptCore/ftl/FTLOperations.cpp
@@ -363,6 +363,9 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
             Structure* structure = globalObject->restParameterStructure();
             ASSERT(argumentCount > 0);
             unsigned arraySize = (argumentCount - 1) > numberOfArgumentsToSkip ? argumentCount - 1 - numberOfArgumentsToSkip : 0;
+
+            // FIXME: we should throw an out of memory error here if tryCreateForInitializationPrivate() fails.
+            // https://bugs.webkit.org/show_bug.cgi?id=169784
             JSArray* array = JSArray::tryCreateForInitializationPrivate(vm, structure, arraySize);
             RELEASE_ASSERT(array);
 
@@ -452,6 +455,8 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
             }
         }
 
+        // FIXME: we should throw an out of memory error here if checkedArraySize has hasOverflowed() or tryCreateForInitializationPrivate() fails.
+        // https://bugs.webkit.org/show_bug.cgi?id=169784
         unsigned arraySize = checkedArraySize.unsafeGet(); // Crashes if overflowed.
         JSArray* result = JSArray::tryCreateForInitializationPrivate(vm, structure, arraySize);
         RELEASE_ASSERT(result);

diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
index 85ff654..ac0cbd4 100644 (file)
--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
@@ -1042,8 +1042,10 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncSplice(ExecState* exec)
             }
         } else {
             result = JSArray::tryCreateForInitializationPrivate(vm, exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), actualDeleteCount);
-            if (!result)
-                return JSValue::encode(throwOutOfMemoryError(exec, scope));
+            if (UNLIKELY(!result)) {
+                throwOutOfMemoryError(exec, scope);
+                return encodedJSValue();
+            }
             
             for (unsigned k = 0; k < actualDeleteCount; ++k) {
                 JSValue v = getProperty(exec, thisObj, k + actualStart);

diff --git a/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp b/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
index bbcb03f..f0d3bef 100644 (file)
--- a/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
+++ b/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
@@ -1010,6 +1010,8 @@ SLOW_PATH_DECL(slow_path_new_array_with_spread)
     Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
 
     JSArray* result = JSArray::tryCreateForInitializationPrivate(vm, structure, arraySize);
+    if (UNLIKELY(!result))
+        THROW(createOutOfMemoryError(exec));
     CHECK_EXCEPTION();
 
     unsigned index = 0;

diff --git a/Source/JavaScriptCore/runtime/JSArray.cpp b/Source/JavaScriptCore/runtime/JSArray.cpp
index 7eebdc2..ff9e054 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSArray.cpp
+++ b/Source/JavaScriptCore/runtime/JSArray.cpp
@@ -62,7 +62,7 @@ Butterfly* createArrayButterflyInDictionaryIndexingMode(
 
 JSArray* JSArray::tryCreateForInitializationPrivate(VM& vm, GCDeferralContext* deferralContext, Structure* structure, unsigned initialLength)
 {
-    if (initialLength > MAX_STORAGE_VECTOR_LENGTH)
+    if (UNLIKELY(initialLength > MAX_STORAGE_VECTOR_LENGTH))
         return 0;
 
     unsigned outOfLineStorage = structure->outOfLineCapacity();
@@ -78,7 +78,7 @@ JSArray* JSArray::tryCreateForInitializationPrivate(VM& vm, GCDeferralContext* d
 
         unsigned vectorLength = Butterfly::optimalContiguousVectorLength(structure, initialLength);
         void* temp = vm.auxiliarySpace.tryAllocate(deferralContext, Butterfly::totalSize(0, outOfLineStorage, true, vectorLength * sizeof(EncodedJSValue)));
-        if (!temp)
+        if (UNLIKELY(!temp))
             return nullptr;
         butterfly = Butterfly::fromBase(temp, 0, outOfLineStorage);
         butterfly->setVectorLength(vectorLength);
@@ -93,7 +93,7 @@ JSArray* JSArray::tryCreateForInitializationPrivate(VM& vm, GCDeferralContext* d
     } else {
         unsigned vectorLength = ArrayStorage::optimalVectorLength(0, structure, initialLength);
         void* temp = vm.auxiliarySpace.tryAllocate(deferralContext, Butterfly::totalSize(0, outOfLineStorage, true, ArrayStorage::sizeFor(vectorLength)));
-        if (!temp)
+        if (UNLIKELY(!temp))
             return nullptr;
         butterfly = Butterfly::fromBase(temp, 0, outOfLineStorage);
         *butterfly->indexingHeader() = indexingHeaderForArrayStorage(initialLength, vectorLength);
@@ -857,7 +857,7 @@ JSArray* JSArray::fastSlice(ExecState& exec, unsigned startIndex, unsigned count
 
         Structure* resultStructure = exec.lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(arrayType);
         JSArray* resultArray = JSArray::tryCreateForInitializationPrivate(vm, resultStructure, count);
-        if (!resultArray)
+        if (UNLIKELY(!resultArray))
             return nullptr;
 
         auto& resultButterfly = *resultArray->butterfly();

diff --git a/Source/JavaScriptCore/runtime/JSArray.h b/Source/JavaScriptCore/runtime/JSArray.h
index 56c07f3..bdbd715 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSArray.h
+++ b/Source/JavaScriptCore/runtime/JSArray.h
@@ -300,6 +300,7 @@ inline JSArray* constructArray(ExecState* exec, Structure* arrayStructure, const
     // FIXME: we should probably throw an out of memory error here, but
     // 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);
 
     for (unsigned i = 0; i < length; ++i)
@@ -315,6 +316,7 @@ inline JSArray* constructArray(ExecState* exec, Structure* arrayStructure, const
     // FIXME: we should probably throw an out of memory error here, but
     // 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);
 
     for (unsigned i = 0; i < length; ++i)
@@ -330,6 +332,7 @@ inline JSArray* constructArrayNegativeIndexed(ExecState* exec, Structure* arrayS
     // FIXME: we should probably throw an out of memory error here, but
     // 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);
 
     for (int i = 0; i < static_cast<int>(length); ++i)

diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
index 0dba069..0dc05c6 100644 (file)
--- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
+++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
@@ -40,7 +40,12 @@ JSArray* createEmptyRegExpMatchesArray(JSGlobalObject* globalObject, JSString* i
     
     if (UNLIKELY(globalObject->isHavingABadTime())) {
         array = JSArray::tryCreateForInitializationPrivate(vm, &deferralContext, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1);
-        
+        // FIXME: we should probably throw an out of memory error here, but
+        // 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);
+
         array->initializeIndexWithoutBarrier(0, jsEmptyString(&vm));
         
         if (unsigned numSubpatterns = regExp->numSubpatterns()) {

diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
index 9770ed3..64ceccb 100644 (file)
--- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
+++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
@@ -81,7 +81,12 @@ ALWAYS_INLINE JSArray* createRegExpMatchesArray(
     
     if (UNLIKELY(globalObject->isHavingABadTime())) {
         array = JSArray::tryCreateForInitializationPrivate(vm, &deferralContext, globalObject->regExpMatchesArrayStructure(), numSubpatterns + 1);
-        
+        // FIXME: we should probably throw an out of memory error here, but
+        // 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);
+
         setProperties();
         
         array->initializeIndexWithoutBarrier(0, jsSubstringOfResolved(vm, &deferralContext, input, result.start, result.end - result.start));