Changeset 203033 in webkit

Timestamp:

Jul 9, 2016, 1:50:51 PM (9 years ago)

Author:

keith_miller@apple.com

Message:

appendMemcpy might fail in concatAppendOne
​https://bugs.webkit.org/show_bug.cgi?id=159601
Source/JavaScriptCore:

<rdar://problem/27211300>

Reviewed by Mark Lam.

There are multiple reasons why we might fail appendMemcpy. One
reason, which I suspect was the source of the crashes, is that one
of the Array prototypes has an indexed property. This patch
consolidates the two old cases by just creating an array then
attempting to memcpy append. If that fails, we fall back to
moveElements.

• runtime/ArrayPrototype.cpp:

(JSC::concatAppendOne):

• tests/stress/concat-with-holesMustForwardToPrototype.js: Added.

(arrayEq):

LayoutTests:

Reviewed by Mark Lam.

Add new microbenchmark testing the performance of concat
when appending one new element. This patch appears to be
about a 13% progression on this test.

• js/regress/concat-append-one-expected.txt: Added.
• js/regress/concat-append-one.html: Added.
• js/regress/script-tests/concat-append-one.js: Added.

(test):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/regress/concat-append-one-expected.txt (added)
• LayoutTests/js/regress/concat-append-one.html (added)
• LayoutTests/js/regress/script-tests/concat-append-one.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/ArrayPrototype.cpp (modified) (1 diff)
• Source/JavaScriptCore/tests/stress/concat-with-holesMustForwardToPrototype.js (added)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-07-09  Keith Miller  <keith_miller@apple.com>
+
+        appendMemcpy might fail in concatAppendOne
+        https://bugs.webkit.org/show_bug.cgi?id=159601
+
+        Reviewed by Mark Lam.
+
+        Add new microbenchmark testing the performance of concat
+        when appending one new element. This patch appears to be
+        about a 13% progression on this test.
+
+        * js/regress/concat-append-one-expected.txt: Added.
+        * js/regress/concat-append-one.html: Added.
+        * js/regress/script-tests/concat-append-one.js: Added.
+        (test):
+
 2016-07-09  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-07-09  Keith Miller  <keith_miller@apple.com>
+
+        appendMemcpy might fail in concatAppendOne
+        https://bugs.webkit.org/show_bug.cgi?id=159601
+        <rdar://problem/27211300>
+
+        Reviewed by Mark Lam.
+
+        There are multiple reasons why we might fail appendMemcpy. One
+        reason, which I suspect was the source of the crashes, is that one
+        of the Array prototypes has an indexed property. This patch
+        consolidates the two old cases by just creating an array then
+        attempting to memcpy append. If that fails, we fall back to
+        moveElements.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::concatAppendOne):
+        * tests/stress/concat-with-holesMustForwardToPrototype.js: Added.
+        (arrayEq):
+
 2016-07-09  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -1048 +1048 @@
 
     IndexingType type = first->mergeIndexingTypeForCopying(indexingTypeForValue(second) | IsArray);
-    JSArray* result;
-    if (type == NonArray) {
-        result = constructEmptyArray(exec, nullptr, firstArraySize + 1);
-        if (vm.exception())
-            return JSValue::encode(JSValue());
-
+    if (type == NonArray)
+        type = ArrayWithUndecided;
+
+    Structure* resultStructure = exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(type);
+    JSArray* result = JSArray::create(vm, resultStructure, firstArraySize + 1);
+    if (!result)
+        return JSValue::encode(throwOutOfMemoryError(exec));
+
+    if (!result->appendMemcpy(exec, vm, 0, first)) {
         if (!moveElements(exec, vm, result, 0, first, firstArraySize)) {
             ASSERT(vm.exception());
             return JSValue::encode(JSValue());
         }
-
-    } else {
-        Structure* resultStructure = exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(type);
-        result = JSArray::tryCreateUninitialized(vm, resultStructure, firstArraySize + 1);
-        if (!result)
-            return JSValue::encode(throwOutOfMemoryError(exec));
-
-        bool memcpyResult = result->appendMemcpy(exec, vm, 0, first);
-        RELEASE_ASSERT(memcpyResult);
     }