Changeset 203952 in webkit

Timestamp:

Jul 30, 2016 6:08:37 PM (8 years ago)

Author:

mark.lam@apple.com

Message:

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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/fast/dom/wrapper-classes-objc-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/fast/dom/wrapper-classes-objc.html (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSArray.cpp (modified) (1 diff)
• Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-large-length.js (added)
• Source/JavaScriptCore/tests/stress/array-setLength-on-ArrayClass-with-small-length.js (added)

trunk/LayoutTests/ChangeLog

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

trunk/LayoutTests/platform/mac/fast/dom/wrapper-classes-objc-expected.txt

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

trunk/LayoutTests/platform/mac/fast/dom/wrapper-classes-objc.html

@@ -291 +291 @@
     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:
 

trunk/Source/JavaScriptCore/ChangeLog

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

trunk/Source/JavaScriptCore/runtime/JSArray.cpp

@@ -442 +442 @@
             return setLengthWithArrayStorage(
                 exec, newLength, throwException,
-                convertContiguousToArrayStorage(exec->vm()));
+                ensureArrayStorage(exec->vm()));
         }
         createInitialUndecided(exec->vm(), newLength);