Assertion failure while setting the length of an ArrayClass array.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 31 Jul 2016 01:08:37 +0000 (01:08 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 31 Jul 2016 01:08:37 +0000 (01:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160381
<rdar://problem/27328703>

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

When setting large length values, we're currently treating ArrayClass as a
ContiguousIndexingType array.  This results in an assertion failure.  This is
now fixed.

There are currently only 2 places where we create arrays with indexing type
ArrayClass: ArrayPrototype and RuntimeArray.  The fix in JSArray:;setLength()
takes care of ArrayPrototype.

RuntimeArray already checks for the setting of its length property, and will
throw a RangeError.  Hence, there's no change is needed for the RuntimeArray.
Instead, I added some test cases ensure that the check and throw behavior does
not change without notice.

* runtime/JSArray.cpp:
(JSC::JSArray::setLength):
* tests/stress/array-setLength-on-ArrayClass-with-large-length.js: Added.
(toString):
(assertEqual):
* tests/stress/array-setLength-on-ArrayClass-with-small-length.js: Added.
(toString):
(assertEqual):

LayoutTests:

Test that RuntimeArrays will throw an error if we try to set its length.

* platform/mac/fast/dom/wrapper-classes-objc.html:
* platform/mac/fast/dom/wrapper-classes-objc-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/fast/dom/wrapper-classes-objc-expected.txt
LayoutTests/platform/mac/fast/dom/wrapper-classes-objc.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-large-length.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-small-length.js [new file with mode: 0644]

index aae02a7..cfb75c7 100644 (file)
@@ -1,3 +1,16 @@
+2016-07-30  Mark Lam  <mark.lam@apple.com>
+
+        Assertion failure while setting the length of an ArrayClass array.
+        https://bugs.webkit.org/show_bug.cgi?id=160381
+        <rdar://problem/27328703>
+
+        Reviewed by Filip Pizlo.
+
+        Test that RuntimeArrays will throw an error if we try to set its length.
+
+        * platform/mac/fast/dom/wrapper-classes-objc.html:
+        * platform/mac/fast/dom/wrapper-classes-objc-expected.txt:
+
 2016-07-30  Chris Dumez  <cdumez@apple.com>
 
         Enable strict type checking for Window dictionary members
index 9ce30c9..58184ba 100644 (file)
@@ -191,4 +191,9 @@ PASS typeof objCObjectOfClass('NSCFString') is 'string'
 PASS typeof objCObjectOfClass('WebScriptObject') is 'object'
 PASS objCObjectOfClass('NSArray') instanceof Array is true
 PASS concatenateArray(objCArrayOfString()) is 'onetwothree'
+PASS let arr = objCArrayOfString(); arr.length is 3
+PASS let arr = objCArrayOfString(); arr.length = 0 threw exception RangeError: Range error.
+PASS let arr = objCArrayOfString(); arr.length = 5 threw exception RangeError: Range error.
+PASS let arr = objCArrayOfString(); arr.length = 0x40000000 threw exception RangeError: Range error.
+PASS let arr = objCArrayOfString(); try { arr.length = 0 } catch(e) { } arr.length is 3
 
index 3fecba1..ddb0167 100644 (file)
@@ -290,6 +290,12 @@ function runTest()
 
     shouldBe("concatenateArray(objCArrayOfString())", "'onetwothree'");
 
+    shouldBe("let arr = objCArrayOfString(); arr.length", "3");
+    shouldThrow("let arr = objCArrayOfString(); arr.length = 0");
+    shouldThrow("let arr = objCArrayOfString(); arr.length = 5");
+    shouldThrow("let arr = objCArrayOfString(); arr.length = 0x40000000");
+    shouldBe("let arr = objCArrayOfString(); try { arr.length = 0 } catch(e) { } arr.length", "3");
+
     // Not yet tested:
 
     // CSSCharsetRule
index dbd0190..8adbebb 100644 (file)
@@ -1,3 +1,33 @@
+2016-07-30  Mark Lam  <mark.lam@apple.com>
+
+        Assertion failure while setting the length of an ArrayClass array.
+        https://bugs.webkit.org/show_bug.cgi?id=160381
+        <rdar://problem/27328703>
+
+        Reviewed by Filip Pizlo.
+
+        When setting large length values, we're currently treating ArrayClass as a
+        ContiguousIndexingType array.  This results in an assertion failure.  This is
+        now fixed.
+
+        There are currently only 2 places where we create arrays with indexing type
+        ArrayClass: ArrayPrototype and RuntimeArray.  The fix in JSArray:;setLength()
+        takes care of ArrayPrototype.
+
+        RuntimeArray already checks for the setting of its length property, and will
+        throw a RangeError.  Hence, there's no change is needed for the RuntimeArray.
+        Instead, I added some test cases ensure that the check and throw behavior does
+        not change without notice.
+
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::setLength):
+        * tests/stress/array-setLength-on-ArrayClass-with-large-length.js: Added.
+        (toString):
+        (assertEqual):
+        * tests/stress/array-setLength-on-ArrayClass-with-small-length.js: Added.
+        (toString):
+        (assertEqual):
+
 2016-07-29  Keith Miller  <keith_miller@apple.com>
 
         TypedArray super constructor has some incompatabilities
index a1747d6..cdc6a69 100644 (file)
@@ -441,7 +441,7 @@ bool JSArray::setLength(ExecState* exec, unsigned newLength, bool throwException
         if (newLength >= MIN_SPARSE_ARRAY_INDEX) {
             return setLengthWithArrayStorage(
                 exec, newLength, throwException,
-                convertContiguousToArrayStorage(exec->vm()));
+                ensureArrayStorage(exec->vm()));
         }
         createInitialUndecided(exec->vm(), newLength);
         return true;
diff --git a/Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-large-length.js b/Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-large-length.js
new file mode 100644 (file)
index 0000000..c534a3a
--- /dev/null
@@ -0,0 +1,19 @@
+//@ runDefault
+// This test should not crash
+
+function assertEqual(actual, expected) {
+    function toString(x) {
+        return '' + x;
+    }
+    if (typeof actual != typeof expected)
+        throw Error("Failed: typeof expected: '" + typeof(expected) + "', typeof actual: '" + typeof(actual) + "'");;
+    
+    if (toString(actual) != toString(expected))
+        throw Error("Failed: expected: '" + toString(expected) + "', actual: '" + toString(actual) + "'");;
+}
+
+assertEqual(Array.prototype.length, 0);
+
+Array.prototype.length = 0x40000000;
+
+assertEqual(Array.prototype.length, 0x40000000);
diff --git a/Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-small-length.js b/Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-small-length.js
new file mode 100644 (file)
index 0000000..5ac4c8c
--- /dev/null
@@ -0,0 +1,19 @@
+//@ runDefault
+// This test should not crash
+
+function assertEqual(actual, expected) {
+    function toString(x) {
+        return '' + x;
+    }
+    if (typeof actual != typeof expected)
+        throw Error("Failed: typeof expected: '" + typeof(expected) + "', typeof actual: '" + typeof(actual) + "'");;
+    
+    if (toString(actual) != toString(expected))
+        throw Error("Failed: expected: '" + toString(expected) + "', actual: '" + toString(actual) + "'");;
+}
+
+assertEqual(Array.prototype.length, 0);
+
+Array.prototype.length = 5;
+
+assertEqual(Array.prototype.length, 5);