Changeset 214684 in webkit

Timestamp:

Mar 31, 2017 1:18:05 PM (7 years ago)

Author:

mark.lam@apple.com

Message:

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.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ArrayPrototype.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -968 +968 @@
 }
 
-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)
 {
@@ -1056 +1042 @@
             }
         } 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);
@@ -1062 +1048 @@
             }
 
-            // 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());
             }
         }