[webkit-changes] [207322] trunk

Thu, 13 Oct 2016 22:29:16 -0700

Title: [207322] trunk
Revision
207322
Author
mark....@apple.com
Date
2016-10-13 22:29:02 -0700 (Thu, 13 Oct 2016)

Log Message

Fix Array.prototype.splice ES6 compliance.
https://bugs.webkit.org/show_bug.cgi?id=163372

Reviewed by Geoffrey Garen and Yusuke Suzuki.

JSTests:

* stress/array-splice-on-frozen-object.js: Added.

Source/_javascript_Core:

Our Array.prototype.splice implementation neglected to set length on the result
array (step 12 of https://tc39.github.io/ecma262/#sec-array.prototype.splice) in
a certain code path.  This is now fixed.

I'm deferring the implementation of step 8 till later because it requires more
careful consideration and the fix is of a lesser value (and therefore, of less
urgency).  See https://bugs.webkit.org/show_bug.cgi?id=163417

Also added some needed exception checks and assertions.

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

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (207321 => 207322)


--- trunk/JSTests/ChangeLog	2016-10-14 04:09:33 UTC (rev 207321)
+++ trunk/JSTests/ChangeLog	2016-10-14 05:29:02 UTC (rev 207322)
@@ -1,3 +1,12 @@
+2016-10-13  Mark Lam  <mark....@apple.com>
+
+        Fix Array.prototype.splice ES6 compliance.
+        https://bugs.webkit.org/show_bug.cgi?id=163372
+
+        Reviewed by Geoffrey Garen and Yusuke Suzuki.
+
+        * stress/array-splice-on-frozen-object.js: Added.
+
 2016-10-12  Keith Miller  <keith_mil...@apple.com>
 
         Handle non-function, non-undefined comparator in Array.prototype.sort

Added: trunk/JSTests/stress/array-splice-on-frozen-object.js (0 => 207322)


--- trunk/JSTests/stress/array-splice-on-frozen-object.js	                        (rev 0)
+++ trunk/JSTests/stress/array-splice-on-frozen-object.js	2016-10-14 05:29:02 UTC (rev 207322)
@@ -0,0 +1,72 @@
+//@ runFTLNoCJIT
+
+let totalFailed = 0;
+
+function shouldEqual(testId, actual, expected) {
+    if (actual != expected) {
+        throw testId + ": ERROR: expect " + expected + ", actual " + actual;
+    }
+}
+
+function makeArray() {
+    return ['unmodifiable'];
+}
+
+function makeArrayLikeObject() {
+    var obj = {};
+    obj[0] = 'unmodifiable';
+    obj.length = 1; 
+    return obj;
+}
+
+function emptyArraySourceMaker() {
+    return [];
+}
+
+function singleElementArraySourceMaker() {
+    return ['modified_1'];
+}
+
+// Make test functions with unique codeblocks.
+function makeSpliceWithNoArgsTest(testId) {
+    return new Function("arr", "arr.splice(); return " + testId + ";");
+}
+function makeSpliceTest(testId) {
+    return new Function("arr", "arr.splice(0); return " + testId + ";");
+}
+
+let numIterations = 10000;
+
+function runTest(testId, testMaker, targetMaker, sourceMaker, expectedValue, expectedException) {
+    var test = testMaker(testId);
+    noInline(test);
+
+    for (var i = 0; i < numIterations; i++) {
+        var exception = undefined;
+
+        var obj = targetMaker();
+        obj = Object.freeze(obj);
+
+        var arr = sourceMaker();
+        arr.constructor = { [Symbol.species]: function() { return obj; } };
+
+        try {
+            test(arr);
+        } catch (e) {
+            exception = "" + e;
+            exception = exception.substr(0, 10); // Search for "TypeError:".
+        }
+        shouldEqual(testId, exception, expectedException);
+        shouldEqual(testId, obj[0], expectedValue);
+    }
+}
+
+runTest(10010, makeSpliceWithNoArgsTest, makeArray,           emptyArraySourceMaker,         "unmodifiable", "TypeError:");
+runTest(10011, makeSpliceWithNoArgsTest, makeArray,           singleElementArraySourceMaker, "unmodifiable", "TypeError:");
+runTest(10020, makeSpliceWithNoArgsTest, makeArrayLikeObject, emptyArraySourceMaker,         "unmodifiable", "TypeError:");
+runTest(10021, makeSpliceWithNoArgsTest, makeArrayLikeObject, singleElementArraySourceMaker, "unmodifiable", "TypeError:");
+
+runTest(10110, makeSpliceTest, makeArray,           emptyArraySourceMaker,         "unmodifiable", "TypeError:");
+runTest(10111, makeSpliceTest, makeArray,           singleElementArraySourceMaker, "unmodifiable", "TypeError:");
+runTest(10120, makeSpliceTest, makeArrayLikeObject, emptyArraySourceMaker,         "unmodifiable", "TypeError:");
+runTest(10121, makeSpliceTest, makeArrayLikeObject, singleElementArraySourceMaker, "unmodifiable", "TypeError:");

Modified: trunk/Source/_javascript_Core/ChangeLog (207321 => 207322)


--- trunk/Source/_javascript_Core/ChangeLog	2016-10-14 04:09:33 UTC (rev 207321)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-10-14 05:29:02 UTC (rev 207322)
@@ -1,3 +1,23 @@
+2016-10-13  Mark Lam  <mark....@apple.com>
+
+        Fix Array.prototype.splice ES6 compliance.
+        https://bugs.webkit.org/show_bug.cgi?id=163372
+
+        Reviewed by Geoffrey Garen and Yusuke Suzuki.
+
+        Our Array.prototype.splice implementation neglected to set length on the result
+        array (step 12 of https://tc39.github.io/ecma262/#sec-array.prototype.splice) in
+        a certain code path.  This is now fixed.
+
+        I'm deferring the implementation of step 8 till later because it requires more
+        careful consideration and the fix is of a lesser value (and therefore, of less
+        urgency).  See https://bugs.webkit.org/show_bug.cgi?id=163417
+
+        Also added some needed exception checks and assertions.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncSplice):
+
 2016-10-13  Joseph Pecoraro  <pecor...@apple.com>
 
         Web Inspector: Stepping highlight for dot/bracket expressions in if statements highlights subset of the _expression_

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (207321 => 207322)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2016-10-14 04:09:33 UTC (rev 207321)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2016-10-14 05:29:02 UTC (rev 207322)
@@ -922,9 +922,12 @@
     }
 
     unsigned actualStart = argumentClampedIndexFromStartOrEnd(exec, 0, length);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
     unsigned actualDeleteCount = length - actualStart;
+    unsigned insertCount = 0;
     if (exec->argumentCount() > 1) {
+        insertCount = exec->argumentCount() - 2;
         double deleteCount = exec->uncheckedArgument(1).toInteger(exec);
         if (deleteCount < 0)
             actualDeleteCount = 0;
@@ -934,9 +937,13 @@
             actualDeleteCount = static_cast<unsigned>(deleteCount);
     }
 
+    // FIXME: Need to implement step 8 of the spec https://tc39.github.io/ecma262/#sec-array.prototype.splice here.
+    // https://bugs.webkit.org/show_bug.cgi?id=163417
+
     std::pair<SpeciesConstructResult, JSObject*> speciesResult = speciesConstructArray(exec, thisObj, actualDeleteCount);
+    ASSERT(!scope.exception() || speciesResult.first == SpeciesConstructResult::Exception);
     if (speciesResult.first == SpeciesConstructResult::Exception)
-        return JSValue::encode(jsUndefined());
+        return encodedJSValue();
 
     JSObject* result = nullptr;
     if (speciesResult.first == SpeciesConstructResult::FastPath && isJSArray(thisObj) && length == getLength(exec, thisObj))
@@ -966,10 +973,14 @@
                     continue;
                 result->initializeIndex(vm, k, v);
             }
+            ASSERT(!scope.exception());
         }
+        setLength(exec, vm, result, actualDeleteCount);
+        RETURN_IF_EXCEPTION(scope, encodedJSValue());
     }
 
-    unsigned itemCount = std::max<int>(exec->argumentCount() - 2, 0);
+    unsigned itemCount = insertCount;
+    ASSERT(itemCount == static_cast<unsigned>(std::max<int>(exec->argumentCount() - 2, 0)));
     if (itemCount < actualDeleteCount) {
         shift<JSArray::ShiftCountForSplice>(exec, thisObj, actualStart, actualDeleteCount, itemCount, length);
         RETURN_IF_EXCEPTION(scope, encodedJSValue());

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to