Array.prototype.splice() should not be using JSArray::tryCreateForInitializationPriva...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 Mar 2017 20:18:05 +0000 (20:18 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 Mar 2017 20:18:05 +0000 (20:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170303
<rdar://problem/31358281>

Reviewed by Filip Pizlo.

This is because it needs to call getProperty() later to get the values for
initializing the array.  getProperty() can execute arbitrary code and potentially
trigger the GC.  This is not allowed for clients of JSArray::tryCreateForInitializationPrivate().

* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncSplice):
(JSC::copySplicedArrayElements): Deleted.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ArrayPrototype.cpp

index a4230c3..4d32647 100644 (file)
@@ -1,3 +1,19 @@
+2017-03-31  Mark Lam  <mark.lam@apple.com>
+
+        Array.prototype.splice() should not be using JSArray::tryCreateForInitializationPrivate().
+        https://bugs.webkit.org/show_bug.cgi?id=170303
+        <rdar://problem/31358281>
+
+        Reviewed by Filip Pizlo.
+
+        This is because it needs to call getProperty() later to get the values for
+        initializing the array.  getProperty() can execute arbitrary code and potentially
+        trigger the GC.  This is not allowed for clients of JSArray::tryCreateForInitializationPrivate().
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncSplice):
+        (JSC::copySplicedArrayElements): Deleted.
+
 2017-03-31  Oleksandr Skachkov  <gskachkov@gmail.com>
 
         String.prototype.replace incorrectly applies "special replacement parameters" when passed a function
index 78db545..8bae0a6 100644 (file)
@@ -967,20 +967,6 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncSlice(ExecState* exec)
     return JSValue::encode(result);
 }
 
-template<bool needToFillHolesManually>
-inline bool copySplicedArrayElements(ExecState* exec, ThrowScope& scope, JSObject* result, JSObject* thisObj, unsigned actualStart, unsigned actualDeleteCount)
-{
-    VM& vm = scope.vm();
-    for (unsigned k = 0; k < actualDeleteCount; ++k) {
-        JSValue v = getProperty(exec, thisObj, k + actualStart);
-        RETURN_IF_EXCEPTION(scope, false);
-        if (UNLIKELY(!v && !needToFillHolesManually))
-            continue;
-        result->initializeIndex(vm, k, v);
-    }
-    return true;
-}
-
 EncodedJSValue JSC_HOST_CALL arrayProtoFuncSplice(ExecState* exec)
 {
     // 15.4.4.12
@@ -1055,26 +1041,19 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncSplice(ExecState* exec)
                 RETURN_IF_EXCEPTION(scope, encodedJSValue());
             }
         } else {
-            result = JSArray::tryCreateForInitializationPrivate(vm, exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), actualDeleteCount);
+            result = JSArray::tryCreate(vm, exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), actualDeleteCount);
             if (UNLIKELY(!result)) {
                 throwOutOfMemoryError(exec, scope);
                 return encodedJSValue();
             }
 
-            // The result can have an ArrayStorage indexing type if we're having a bad time.
-            bool isArrayStorage = hasAnyArrayStorage(result->indexingType());
-            bool success = false;
-            if (UNLIKELY(isArrayStorage)) {
-                static const bool needToFillHolesManually = true;
-                success = copySplicedArrayElements<needToFillHolesManually>(exec, scope, result, thisObj, actualStart, actualDeleteCount);
-            } else {
-                ASSERT(hasUndecided(result->indexingType()));
-                static const bool needToFillHolesManually = false;
-                success = copySplicedArrayElements<needToFillHolesManually>(exec, scope, result, thisObj, actualStart, actualDeleteCount);
-            }
-            if (UNLIKELY(!success)) {
-                ASSERT(scope.exception());
-                return encodedJSValue();
+            for (unsigned k = 0; k < actualDeleteCount; ++k) {
+                JSValue v = getProperty(exec, thisObj, k + actualStart);
+                RETURN_IF_EXCEPTION(scope, encodedJSValue());
+                if (UNLIKELY(!v))
+                    continue;
+                result->putDirectIndex(exec, k, v);
+                RETURN_IF_EXCEPTION(scope, encodedJSValue());
             }
         }
     }