Title: [203147] trunk
Revision
203147
Author
[email protected]
Date
2016-07-12 18:25:25 -0700 (Tue, 12 Jul 2016)

Log Message

[JSC] Array.prototype.join() fails some conformance tests
https://bugs.webkit.org/show_bug.cgi?id=159657

Patch by Benjamin Poulain <[email protected]> on 2016-07-12
Reviewed by Saam Barati.

Source/_javascript_Core:

There were a couple of failures:
-separator.toString() was called *before* we get the length
 and process ToLength() on it.
-We were using toUInt32() on length instead of ToLength(),
 failing on big integers and various negative numbers.

Additionally, I replaced the "fast" ArrayStorage path
by a fully generic implementation that does not depends on StringJoiner.

The reason is StringJoiner was doing poorly on sparse objects
in certain cases.
If you have a sparse object with a length > INT_MAX but very few
indices defined, and you join on the empty string, it should be possible
to join the array (albeit very slowly). With StringJoiner, we fail
because we try to allocate > INT_MAX empty strings in a contiguous vector.

* runtime/ArrayPrototype.cpp:
(JSC::slowJoin):
(JSC::canUseFastJoin):
(JSC::fastJoin):
(JSC::arrayProtoFuncJoin):
(JSC::join): Deleted.
* runtime/JSArray.h:
(JSC::toLength):

Source/WTF:

* wtf/text/AtomicString.cpp:
(WTF::AtomicString::number):
* wtf/text/AtomicString.h:

LayoutTests:

I removed 3 sputnik tests that are incorrect in the latest spec.
In ES5, Array.prototype.join() was using ToUint32 on the argument:
    https://es5.github.io/#x15.4.4.5
In ES6, the function uses ToLength:
    https://tc39.github.io/ecma262/#sec-array.prototype.join

The test use Infinity and very large integer as the length.
They are guaranteed to time out or run out of memory.
Even if we waited the hours it takes to run this, the results would be different
from what the tests expect.

* js/array-join-expected.txt: Added.
* js/array-join.html: Added.
* js/script-tests/array-join.js: Added.

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (203146 => 203147)


--- trunk/LayoutTests/ChangeLog	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/ChangeLog	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,3 +1,25 @@
+2016-07-12  Benjamin Poulain  <[email protected]>
+
+        [JSC] Array.prototype.join() fails some conformance tests
+        https://bugs.webkit.org/show_bug.cgi?id=159657
+
+        Reviewed by Saam Barati.
+
+        I removed 3 sputnik tests that are incorrect in the latest spec.
+        In ES5, Array.prototype.join() was using ToUint32 on the argument:
+            https://es5.github.io/#x15.4.4.5
+        In ES6, the function uses ToLength:
+            https://tc39.github.io/ecma262/#sec-array.prototype.join
+
+        The test use Infinity and very large integer as the length.
+        They are guaranteed to time out or run out of memory.
+        Even if we waited the hours it takes to run this, the results would be different
+        from what the tests expect.
+
+        * js/array-join-expected.txt: Added.
+        * js/array-join.html: Added.
+        * js/script-tests/array-join.js: Added.
+
 2016-07-12  Commit Queue  <[email protected]>
 
         Unreviewed, rolling out r203131.

Modified: trunk/LayoutTests/fast/history/replacestate-nocrash.html (203146 => 203147)


--- trunk/LayoutTests/fast/history/replacestate-nocrash.html	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/fast/history/replacestate-nocrash.html	2016-07-13 01:25:25 UTC (rev 203147)
@@ -6,7 +6,7 @@
     testRunner.dumpAsText();
 
 Object.prototype.__defineSetter__("foo",function(){history.replaceState("")});
-history.replaceState({foo:1,zzz:Array(1<<22).join("a")});
+history.replaceState({foo:1,zzz:"a".repeat(1<<22)});
 history.state.length;
 </script>
 </html>

Added: trunk/LayoutTests/js/array-join-expected.txt (0 => 203147)


--- trunk/LayoutTests/js/array-join-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/array-join-expected.txt	2016-07-13 01:25:25 UTC (rev 203147)
@@ -0,0 +1,43 @@
+Verify Array.prototype.join() properties
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Function properties
+PASS typeof Array.prototype.join is "function"
+PASS Array.prototype.join.name is "join"
+PASS Array.prototype.join.length is 1
+PASS Object.getOwnPropertyDescriptor(Array.prototype, 'join').configurable is true
+PASS Object.getOwnPropertyDescriptor(Array.prototype, 'join').enumerable is false
+PASS Object.getOwnPropertyDescriptor(Array.prototype, 'join').writable is true
+Int32 Array
+PASS [1, 2, 3].join() is "1,2,3"
+PASS [1, 2, 3].join('') is "123"
+PASS [1, 2, 3].join('柰') is "1柰2柰3"
+Double Array
+PASS [Math.PI, Math.E, 6.626].join() is "3.141592653589793,2.718281828459045,6.626"
+PASS [Math.PI, Math.E, 6.626].join('') is "3.1415926535897932.7182818284590456.626"
+PASS [Math.PI, Math.E, 6.626].join('柰') is "3.141592653589793柰2.718281828459045柰6.626"
+Contiguous Array
+PASS [1, 'WebKit', { toString: () => { return 'IsIncredible'} }].join() is "1,WebKit,IsIncredible"
+PASS [1, 'WebKit', { toString: () => { return 'IsIncredible'} }].join('') is "1WebKitIsIncredible"
+PASS [1, 'WebKit', { toString: () => { return 'IsIncredible'} }].join('柰') is "1柰WebKit柰IsIncredible"
+Sparse Array
+PASS smallSparseArray.join() is "WebKit,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,15"
+PASS smallSparseArray.join('') is "WebKit15"
+PASS smallSparseArray.join('柰') is "WebKit柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰15"
+PASS largeSparseArray1.join('') is "WebKit42"
+PASS largeSparseArray2.join('') is "WebKit42"
+Out of memory
+PASS oversizedArray.join('') threw exception Error: Out of memory.
+ToLength is called first on "this", followed by ToString on the separator. Followed by ToString on each element.
+PASS Array.prototype.join.call(calleeObject, separatorObject) is "WebKit0柰WebKit1"
+PASS callSequence.join(', ') is "calle.length, length.valueOf, separator.toString, calle.get 0, index0.toString, calle.get 1, index0.toString"
+We use ToLength on the object's length, not ToInt32 or ToUInt32.
+PASS Array.prototype.join.call(lengthyObject) is ""
+PASS Array.prototype.join.call(lengthyObject) is ""
+PASS Array.prototype.join.call(lengthyObject) is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/array-join.html (0 => 203147)


--- trunk/LayoutTests/js/array-join.html	                        (rev 0)
+++ trunk/LayoutTests/js/array-join.html	2016-07-13 01:25:25 UTC (rev 203147)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/js/script-tests/array-join.js (0 => 203147)


--- trunk/LayoutTests/js/script-tests/array-join.js	                        (rev 0)
+++ trunk/LayoutTests/js/script-tests/array-join.js	2016-07-13 01:25:25 UTC (rev 203147)
@@ -0,0 +1,96 @@
+"use strict";
+
+description("Verify Array.prototype.join() properties");
+
+debug("Function properties");
+shouldBeEqualToString("typeof Array.prototype.join", "function");
+shouldBeEqualToString("Array.prototype.join.name", "join");
+shouldBe("Array.prototype.join.length", "1");
+shouldBeTrue("Object.getOwnPropertyDescriptor(Array.prototype, 'join').configurable");
+shouldBeFalse("Object.getOwnPropertyDescriptor(Array.prototype, 'join').enumerable");
+shouldBeTrue("Object.getOwnPropertyDescriptor(Array.prototype, 'join').writable");
+
+debug("Int32 Array");
+shouldBeEqualToString("[1, 2, 3].join()", "1,2,3");
+shouldBeEqualToString("[1, 2, 3].join('')", "123");
+shouldBeEqualToString("[1, 2, 3].join('柰')", "1柰2柰3");
+
+debug("Double Array");
+shouldBeEqualToString("[Math.PI, Math.E, 6.626].join()", "3.141592653589793,2.718281828459045,6.626");
+shouldBeEqualToString("[Math.PI, Math.E, 6.626].join('')", "3.1415926535897932.7182818284590456.626");
+shouldBeEqualToString("[Math.PI, Math.E, 6.626].join('柰')", "3.141592653589793柰2.718281828459045柰6.626");
+
+debug("Contiguous Array");
+shouldBeEqualToString("[1, 'WebKit', { toString: () => { return 'IsIncredible'} }].join()", "1,WebKit,IsIncredible");
+shouldBeEqualToString("[1, 'WebKit', { toString: () => { return 'IsIncredible'} }].join('')", "1WebKitIsIncredible");
+shouldBeEqualToString("[1, 'WebKit', { toString: () => { return 'IsIncredible'} }].join('柰')", "1柰WebKit柰IsIncredible");
+
+debug("Sparse Array");
+var smallSparseArray = new Array;
+smallSparseArray[-1] = "Oops";
+smallSparseArray[0] = "WebKit";
+smallSparseArray[42] = 15;
+shouldBeEqualToString("smallSparseArray.join()", "WebKit,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,15");
+shouldBeEqualToString("smallSparseArray.join('')", "WebKit15");
+shouldBeEqualToString("smallSparseArray.join('柰')", "WebKit柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰柰15");
+
+var largeSparseArray1 = new Array;
+largeSparseArray1[100001] = 42;
+largeSparseArray1[0] = "WebKit";
+largeSparseArray1[Number.MAX_SAFE_INTEGER] = { valueOf: () => { return 'IsCool'} };
+shouldBeEqualToString("largeSparseArray1.join('')", "WebKit42");
+
+var largeSparseArray2 = new Array;
+largeSparseArray2[100001] = 42;
+largeSparseArray2[42] = "WebKit";
+largeSparseArray2[1024] = "";
+shouldBeEqualToString("largeSparseArray2.join('')", "WebKit42");
+
+debug("Out of memory");
+// 4194303 * 4096 > Max String Length.
+let oversizedArray = new Array(4096);
+let sharedString = "A".repeat(1048576);
+oversizedArray.fill(sharedString);
+shouldThrow("oversizedArray.join('')", "'Error: Out of memory'");
+
+debug("ToLength is called first on \"this\", followed by ToString on the separator. Followed by ToString on each element.");
+var callSequence = [];
+var lengthObject = {
+    toString: () => { callSequence.push("length.toString"); return "FAIL!"; },
+    valueOf: () => { callSequence.push("length.valueOf"); return 2; }
+};
+var index0Object = {
+    toString: () => { callSequence.push("index0.toString"); return "WebKit0"; },
+    valueOf: () => { callSequence.push("index0.valueOf"); return "FAIL!"; }
+};
+var index1Object = {
+    toString: () => { callSequence.push("index0.toString"); return "WebKit1"; },
+    valueOf: () => { callSequence.push("index0.valueOf"); return "FAIL!"; }
+};
+var calleeObject = {
+    toString: () => { callSequence.push("callee.toString"); return "FAIL!"; },
+    valueOf: () => { callSequence.push("calle.valueOf"); return "FAIL!"; },
+    get length () { callSequence.push("calle.length"); return lengthObject; },
+    get 0 () { callSequence.push("calle.get 0"); return index0Object; },
+    get 1 () { callSequence.push("calle.get 1"); return index1Object; }
+};
+var separatorObject = {
+    toString: () => { callSequence.push("separator.toString"); return "柰"; },
+    valueOf: () => { callSequence.push("separator.valueOf"); return "FAIL!"; }
+};
+
+shouldBeEqualToString("Array.prototype.join.call(calleeObject, separatorObject)", "WebKit0柰WebKit1");
+shouldBeEqualToString("callSequence.join(', ')", "calle.length, length.valueOf, separator.toString, calle.get 0, index0.toString, calle.get 1, index0.toString");
+
+
+debug("We use ToLength on the object's length, not ToInt32 or ToUInt32.");
+var lengthyObject = {
+    get 0 () { throw "Fail! Accessed 0."; },
+    get 1 () { throw "Fail! Accessed 1."; }
+}
+lengthyObject.length = -1;
+shouldBeEqualToString("Array.prototype.join.call(lengthyObject)", "");
+lengthyObject.length = -4294967294;
+shouldBeEqualToString("Array.prototype.join.call(lengthyObject)", "");
+lengthyObject.length = -4294967295;
+shouldBeEqualToString("Array.prototype.join.call(lengthyObject)", "");

Deleted: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A2_T2-expected.txt (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A2_T2-expected.txt	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A2_T2-expected.txt	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,6 +0,0 @@
-S15.4.4.5_A2_T2
-
-PASS 
-
-TEST COMPLETE
-

Deleted: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A2_T2.html (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A2_T2.html	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A2_T2.html	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,167 +0,0 @@
-<html>
-<head>
-<meta charset='utf-8'>
-<style>
-.pass {
-    font-weight: bold;
-    color: green;
-}
-.fail {
-    font-weight: bold;
-    color: red;
-}
-</style>
-
-<script>
-if (window.testRunner)
-    testRunner.dumpAsText();
-
-function SputnikError(message)
-{
-    this.message = message;
-}
-
-SputnikError.prototype.toString = function ()
-{
-    return 'SputnikError: ' + this.message;
-};
-
-var sputnikException;
-
-function testPrint(msg)
-{
-    var span = document.createElement("span");
-    document.getElementById("console").appendChild(span); // insert it first so XHTML knows the namespace 
-    span.innerHTML = msg + '<br />';
-}
-
-function escapeHTML(text)
-{
-    return text.toString().replace(/&/g, "&amp;").replace(/</g, "&lt;");
-}
-
-function printTestPassed(msg)
-{
-    testPrint('<span><span class="pass">PASS</span> ' + escapeHTML(msg) + '</span>');
-}
-
-function printTestFailed(msg)
-{
-    testPrint('<span><span class="fail">FAIL</span> ' + escapeHTML(msg) + '</span>');
-}
-
-function testFailed(msg)
-{
-    throw new SputnikError(msg);
-}
-
-var successfullyParsed = false;
-</script>
-
-</head>
-<body>
-<p>S15.4.4.5_A2_T2</p>
-<div id='console'></div>
-<script>
-try {
-
-/**
- * @name: S15.4.4.5_A2_T2;
- * @section: 15.4.4.5;
- * @assertion: The join function is intentionally generic. 
- * It does not require that its this value be an Array object;
- * @description: If ToUint32(length) is zero, return the empty string;
-*/
-
-var obj = {};
-obj.join = Array.prototype.join;
-
-//CHECK#1
-obj.length = NaN;
-if (obj.join() !== "") {
-  testFailed('#1: var obj = {}; obj.length = NaN; obj.join = Array.prototype.join; obj.join() === "". Actual: ' + (obj.join()));
-}
-
-//CHECK#2
-if (isNaN(obj.length) !== true) {
-  testFailed('#2: var obj = {}; obj.length = NaN; obj.join = Array.prototype.join; obj.join(); obj.length === Not-a-Number. Actual: ' + (obj.length));
-}
-
-//CHECK#3
-obj.length = Number.POSITIVE_INFINITY;
-if (obj.join() !== "") {
-  testFailed('#3: var obj = {}; obj.length = Number.POSITIVE_INFINITY; obj.join = Array.prototype.join; obj.join() === "". Actual: ' + (obj.join()));
-}
-
-//CHECK#4
-if (obj.length !== Number.POSITIVE_INFINITY) {
-  testFailed('#4: var obj = {}; obj.length = Number.POSITIVE_INFINITY; obj.join = Array.prototype.join; obj.join(); obj.length === Number.POSITIVE_INFINITY. Actual: ' + (obj.length));
-}
-
-//CHECK#5
-obj.length = Number.NEGATIVE_INFINITY;
-if (obj.join() !== "") {
-  testFailed('#5: var obj = {}; obj.length = Number.NEGATIVE_INFINITY; obj.join = Array.prototype.join; obj.join() === "". Actual: ' + (obj.join()));
-}
-
-//CHECK#6
-if (obj.length !== Number.NEGATIVE_INFINITY) {
-  testFailed('#6: var obj = {}; obj.length = Number.NEGATIVE_INFINITY; obj.join = Array.prototype.join; obj.join(); obj.length === Number.NEGATIVE_INFINITY. Actual: ' + (obj.length));
-}
-
-//CHECK#7
-obj.length = -0;
-if (obj.join() !== "") {
-  testFailed('#7: var obj = {}; obj.length = -0; obj.join = Array.prototype.join; obj.join() === "". Actual: ' + (obj.join()));
-}    
-
-//CHECK#8
-if (obj.length !== -0) {
-  testFailed('#8: var obj = {}; obj.length = -0; obj.join = Array.prototype.join; obj.join(); obj.length === 0. Actual: ' + (obj.length));
-} else {
-  if (1/obj.length !== Number.NEGATIVE_INFINITY) {
-    testFailed('#8: var obj = {}; obj.length = -0; obj.join = Array.prototype.join; obj.join(); obj.length === -0. Actual: ' + (obj.length));
-  }  
-}   
-
-//CHECK#9
-obj.length = 0.5;
-if (obj.join() !== "") {
-  testFailed('#9: var obj = {}; obj.length = 0.5; obj.join = Array.prototype.join; obj.join() === "". Actual: ' + (obj.join()));
-}
-
-//CHECK#10
-if (obj.length !== 0.5) {
-  testFailed('#10: var obj = {}; obj.length = 0.5; obj.join = Array.prototype.join; obj.join(); obj.length === 0.5. Actual: ' + (obj.length));
-} 
-
-//CHECK#11
-var x = new Number(0);
-obj.length = x;
-if (obj.join() !== "") {
-  testFailed('#11: var x = new Number(0); var obj = {}; obj.length = x; obj.join = Array.prototype.join; obj.join() === "". Actual: ' + (obj.join()));
-}
-
-//CHECK#12
-if (obj.length !== x) {
-  testFailed('#12: var x = new Number(0); var obj = {}; obj.length = x; obj.join = Array.prototype.join; obj.join(); obj.length === x. Actual: ' + (obj.length));
-}
-
-} catch (ex) {
-    sputnikException = ex;
-}
-
-var successfullyParsed = true;
-</script>
-
-<script>
-if (!successfullyParsed)
-    printTestFailed('successfullyParsed is not set');
-else if (sputnikException)
-    printTestFailed(sputnikException);
-else
-    printTestPassed("");
-testPrint('<br /><span class="pass">TEST COMPLETE</span>');
-</script>
-</body>
-</html>

Deleted: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T1-expected.txt (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T1-expected.txt	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T1-expected.txt	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,6 +0,0 @@
-S15.4.4.5_A4_T1
-
-PASS 
-
-TEST COMPLETE
-

Deleted: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T1.html (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T1.html	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T1.html	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,108 +0,0 @@
-<html>
-<head>
-<meta charset='utf-8'>
-<style>
-.pass {
-    font-weight: bold;
-    color: green;
-}
-.fail {
-    font-weight: bold;
-    color: red;
-}
-</style>
-
-<script>
-if (window.testRunner)
-    testRunner.dumpAsText();
-
-function SputnikError(message)
-{
-    this.message = message;
-}
-
-SputnikError.prototype.toString = function ()
-{
-    return 'SputnikError: ' + this.message;
-};
-
-var sputnikException;
-
-function testPrint(msg)
-{
-    var span = document.createElement("span");
-    document.getElementById("console").appendChild(span); // insert it first so XHTML knows the namespace 
-    span.innerHTML = msg + '<br />';
-}
-
-function escapeHTML(text)
-{
-    return text.toString().replace(/&/g, "&amp;").replace(/</g, "&lt;");
-}
-
-function printTestPassed(msg)
-{
-    testPrint('<span><span class="pass">PASS</span> ' + escapeHTML(msg) + '</span>');
-}
-
-function printTestFailed(msg)
-{
-    testPrint('<span><span class="fail">FAIL</span> ' + escapeHTML(msg) + '</span>');
-}
-
-function testFailed(msg)
-{
-    throw new SputnikError(msg);
-}
-
-var successfullyParsed = false;
-</script>
-
-</head>
-<body>
-<p>S15.4.4.5_A4_T1</p>
-<div id='console'></div>
-<script>
-try {
-
-/**
- * @name: S15.4.4.5_A4_T1;
- * @section: 15.4.4.5;
- * @assertion: Check ToUint32(length) for non Array objects;
- * @description: length = 4294967296; 
-*/
-
-var obj = {};
-obj.join = Array.prototype.join;
-obj[0] = "x";
-obj[4294967295] = "y";
-obj.length = 4294967296;
-
-//CHECK#1
-if (obj.join("") !== "") {
-  testFailed('#1: var obj = {}; obj.join = Array.prototype.join; obj[0] = "x"; obj[4294967295] = "y"; obj.length = 4294967296; obj.join("") === "". Actual: ' + (obj.join("")));
-}
-
-//CHECK#2
-if (obj.length !== 4294967296) {
-  testFailed('#2: var obj = {}; obj.join = Array.prototype.join; obj[0] = "x"; obj[4294967295] = "y"; obj.length = 4294967296; obj.join(""); obj.length === 4294967296. Actual: ' + (obj.length));
-}
-
-} catch (ex) {
-    sputnikException = ex;
-}
-
-var successfullyParsed = true;
-</script>
-
-<script>
-if (!successfullyParsed)
-    printTestFailed('successfullyParsed is not set');
-else if (sputnikException)
-    printTestFailed(sputnikException);
-else
-    printTestPassed("");
-testPrint('<br /><span class="pass">TEST COMPLETE</span>');
-</script>
-</body>
-</html>

Deleted: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T2-expected.txt (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T2-expected.txt	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T2-expected.txt	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,6 +0,0 @@
-S15.4.4.5_A4_T2
-
-PASS 
-
-TEST COMPLETE
-

Deleted: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T2.html (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T2.html	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T2.html	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,109 +0,0 @@
-<html>
-<head>
-<meta charset='utf-8'>
-<style>
-.pass {
-    font-weight: bold;
-    color: green;
-}
-.fail {
-    font-weight: bold;
-    color: red;
-}
-</style>
-
-<script>
-if (window.testRunner)
-    testRunner.dumpAsText();
-
-function SputnikError(message)
-{
-    this.message = message;
-}
-
-SputnikError.prototype.toString = function ()
-{
-    return 'SputnikError: ' + this.message;
-};
-
-var sputnikException;
-
-function testPrint(msg)
-{
-    var span = document.createElement("span");
-    document.getElementById("console").appendChild(span); // insert it first so XHTML knows the namespace 
-    span.innerHTML = msg + '<br />';
-}
-
-function escapeHTML(text)
-{
-    return text.toString().replace(/&/g, "&amp;").replace(/</g, "&lt;");
-}
-
-function printTestPassed(msg)
-{
-    testPrint('<span><span class="pass">PASS</span> ' + escapeHTML(msg) + '</span>');
-}
-
-function printTestFailed(msg)
-{
-    testPrint('<span><span class="fail">FAIL</span> ' + escapeHTML(msg) + '</span>');
-}
-
-function testFailed(msg)
-{
-    throw new SputnikError(msg);
-}
-
-var successfullyParsed = false;
-</script>
-
-</head>
-<body>
-<p>S15.4.4.5_A4_T2</p>
-<div id='console'></div>
-<script>
-try {
-
-/**
- * @name: S15.4.4.5_A4_T2;
- * @section: 15.4.4.5;
- * @assertion: Check ToUint32(length) for non Array objects;
- * @description: length = 4294967297; 
-*/
-
-var obj = {};
-obj.join = Array.prototype.join;
-obj[0] = "x";
-obj[1] = "y";
-obj[4294967296] = "z";
-obj.length = 4294967297;
-
-//CHECK#1
-if (obj.join("") !== "x") {
-  testFailed('#1: var obj = {}; obj.join = Array.prototype.join; obj[0] = "x"; obj[1] = "y"; obj[4294967296] = "z"; obj.length = 4294967297; obj.join("") === "x". Actual: ' + (obj.join("")));
-}
-
-//CHECK#2
-if (obj.length !== 4294967297) {
-  testFailed('#2: var obj = {}; obj.join = Array.prototype.join; obj[0] = "x"; obj[1] = "y"; obj[4294967296] = "z"; obj.length = 4294967297; obj.join(""); obj.length === 4294967297. Actual: ' + (obj.length));
-}
-
-} catch (ex) {
-    sputnikException = ex;
-}
-
-var successfullyParsed = true;
-</script>
-
-<script>
-if (!successfullyParsed)
-    printTestFailed('successfullyParsed is not set');
-else if (sputnikException)
-    printTestFailed(sputnikException);
-else
-    printTestPassed("");
-testPrint('<br /><span class="pass">TEST COMPLETE</span>');
-</script>
-</body>
-</html>

Modified: trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T3.html (203146 => 203147)


--- trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T3.html	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.5_Array_prototype_join/S15.4.4.5_A4_T3.html	2016-07-13 01:25:25 UTC (rev 203147)
@@ -80,8 +80,8 @@
 obj.length = -4294967294;
 
 //CHECK#1
-if (obj.join("") !== "xy") {
-  testFailed('#1: var obj = {}; obj.join = Array.prototype.join; obj[0] = "x"; obj[1] = "y"; obj[2] = "z"; obj.length = -4294967294; obj.join("") === "xy". Actual: ' + (obj.join("")));
+if (obj.join("") !== "") {
+  testFailed('#1: var obj = {}; obj.join = Array.prototype.join; obj[0] = "x"; obj[1] = "y"; obj[2] = "z"; obj.length = -4294967294; obj.join("") === "". Actual: ' + (obj.join("")));
 }
 
 //CHECK#2

Modified: trunk/Source/_javascript_Core/ChangeLog (203146 => 203147)


--- trunk/Source/_javascript_Core/ChangeLog	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,3 +1,35 @@
+2016-07-12  Benjamin Poulain  <[email protected]>
+
+        [JSC] Array.prototype.join() fails some conformance tests
+        https://bugs.webkit.org/show_bug.cgi?id=159657
+
+        Reviewed by Saam Barati.
+
+        There were a couple of failures:
+        -separator.toString() was called *before* we get the length
+         and process ToLength() on it.
+        -We were using toUInt32() on length instead of ToLength(),
+         failing on big integers and various negative numbers.
+
+        Additionally, I replaced the "fast" ArrayStorage path
+        by a fully generic implementation that does not depends on StringJoiner.
+
+        The reason is StringJoiner was doing poorly on sparse objects
+        in certain cases.
+        If you have a sparse object with a length > INT_MAX but very few
+        indices defined, and you join on the empty string, it should be possible
+        to join the array (albeit very slowly). With StringJoiner, we fail
+        because we try to allocate > INT_MAX empty strings in a contiguous vector.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::slowJoin):
+        (JSC::canUseFastJoin):
+        (JSC::fastJoin):
+        (JSC::arrayProtoFuncJoin):
+        (JSC::join): Deleted.
+        * runtime/JSArray.h:
+        (JSC::toLength):
+
 2016-07-12  Mark Lam  <[email protected]>
 
         Gardening: C Loop build fix after r203142.

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (203146 => 203147)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2016-07-13 01:25:25 UTC (rev 203147)
@@ -486,14 +486,73 @@
     return object->structure(vm)->holesMustForwardToPrototype(vm);
 }
 
-static inline JSValue join(ExecState& state, JSObject* thisObject, StringView separator)
+static JSValue slowJoin(ExecState& exec, JSObject* thisObject, JSString* separator, uint64_t length)
 {
-    unsigned length = getLength(&state, thisObject);
-    if (state.hadException())
-        return jsUndefined();
+    // 5. If len is zero, return the empty String.
+    if (!length)
+        return jsEmptyString(&exec);
 
+    VM& vm = exec.vm();
+
+    // 6. Let element0 be Get(O, "0").
+    JSValue element0 = thisObject->getIndex(&exec, 0);
+    if (vm.exception())
+        return JSValue();
+
+    // 7. If element0 is undefined or null, let R be the empty String; otherwise, let R be ? ToString(element0).
+    JSString* r = nullptr;
+    if (element0.isUndefinedOrNull())
+        r = jsEmptyString(&exec);
+    else
+        r = element0.toString(&exec);
+    if (vm.exception())
+        return JSValue();
+
+    // 8. Let k be 1.
+    // 9. Repeat, while k < len
+    // 9.e Increase k by 1..
+    for (uint64_t k = 1; k < length; ++k) {
+        // b. Let element be ? Get(O, ! ToString(k)).
+        JSValue element = thisObject->get(&exec, Identifier::fromString(&exec, AtomicString::number(k)));
+        if (vm.exception())
+            return JSValue();
+
+        // c. If element is undefined or null, let next be the empty String; otherwise, let next be ? ToString(element).
+        JSString* next = nullptr;
+        if (element.isUndefinedOrNull()) {
+            if (!separator->length())
+                continue;
+            next = jsEmptyString(&exec);
+        } else
+            next = element.toString(&exec);
+        if (vm.exception())
+            return JSValue();
+
+        // a. Let S be the String value produced by concatenating R and sep.
+        // d. Let R be a String value produced by concatenating S and next.
+        r = JSRopeString::create(vm, r, separator, next);
+    }
+    // 10. Return R.
+    return r;
+}
+
+static inline bool canUseFastJoin(const JSObject* thisObject)
+{
     switch (thisObject->indexingType()) {
     case ALL_CONTIGUOUS_INDEXING_TYPES:
+    case ALL_INT32_INDEXING_TYPES:
+    case ALL_DOUBLE_INDEXING_TYPES:
+        return true;
+    default:
+        break;
+    }
+    return false;
+}
+
+static inline JSValue fastJoin(ExecState& state, JSObject* thisObject, StringView separator, unsigned length)
+{
+    switch (thisObject->indexingType()) {
+    case ALL_CONTIGUOUS_INDEXING_TYPES:
     case ALL_INT32_INDEXING_TYPES: {
         auto& butterfly = *thisObject->butterfly();
         if (length > butterfly.publicLength())
@@ -542,26 +601,7 @@
         }
         return joiner.join(state);
     }
-    case ALL_ARRAY_STORAGE_INDEXING_TYPES: {
-        auto& storage = *thisObject->butterfly()->arrayStorage();
-        if (length > storage.vectorLength())
-            break;
-        if (storage.hasHoles() && thisObject->structure(state.vm())->holesMustForwardToPrototype(state.vm()))
-            break;
-        JSStringJoiner joiner(state, separator, length);
-        if (state.hadException())
-            return jsUndefined();
-        auto data = ""
-        for (unsigned i = 0; i < length; ++i) {
-            if (JSValue value = data[i].get()) {
-                if (!joiner.appendWithoutSideEffects(state, value))
-                    goto generalCase;
-            } else
-                joiner.appendEmptyString();
-        }
-        return joiner.join(state);
     }
-    }
 
 generalCase:
     JSStringJoiner joiner(state, separator, length);
@@ -580,6 +620,7 @@
 
 EncodedJSValue JSC_HOST_CALL arrayProtoFuncJoin(ExecState* exec)
 {
+    // 1. Let O be ? ToObject(this value).
     JSObject* thisObject = exec->thisValue().toThis(exec, StrictMode).toObject(exec);
     if (!thisObject)
         return JSValue::encode(JSValue());
@@ -588,16 +629,43 @@
     if (JSValue earlyReturnValue = checker.earlyReturnValue())
         return JSValue::encode(earlyReturnValue);
 
+    // 2. Let len be ? ToLength(? Get(O, "length")).
+    double length = toLength(exec, thisObject);
+    if (exec->hadException())
+        return JSValue::encode(JSValue());
+
+    // 3. If separator is undefined, let separator be the single-element String ",".
     JSValue separatorValue = exec->argument(0);
     if (separatorValue.isUndefined()) {
         const LChar comma = ',';
-        return JSValue::encode(join(*exec, thisObject, { &comma, 1 }));
+
+        if (UNLIKELY(length > std::numeric_limits<unsigned>::max() || !canUseFastJoin(thisObject))) {
+            uint64_t length64 = static_cast<uint64_t>(length);
+            ASSERT(static_cast<double>(length64) == length);
+            JSString* jsSeparator = jsSingleCharacterString(exec, comma);
+            if (exec->hadException())
+                return JSValue::encode(JSValue());
+
+            return JSValue::encode(slowJoin(*exec, thisObject, jsSeparator, length64));
+        }
+
+        unsigned unsignedLength = static_cast<unsigned>(length);
+        ASSERT(static_cast<double>(unsignedLength) == length);
+        return JSValue::encode(fastJoin(*exec, thisObject, { &comma, 1 }, unsignedLength));
     }
 
-    JSString* separator = separatorValue.toString(exec);
+    // 4. Let sep be ? ToString(separator).
+    JSString* jsSeparator = separatorValue.toString(exec);
     if (exec->hadException())
         return JSValue::encode(jsUndefined());
-    return JSValue::encode(join(*exec, thisObject, separator->view(exec).get()));
+
+    if (UNLIKELY(length > std::numeric_limits<unsigned>::max() || !canUseFastJoin(thisObject))) {
+        uint64_t length64 = static_cast<uint64_t>(length);
+        ASSERT(static_cast<double>(length64) == length);
+        return JSValue::encode(slowJoin(*exec, thisObject, jsSeparator, length64));
+    }
+
+    return JSValue::encode(fastJoin(*exec, thisObject, jsSeparator->view(exec).get(), length));
 }
 
 EncodedJSValue JSC_HOST_CALL arrayProtoFuncPop(ExecState* exec)

Modified: trunk/Source/_javascript_Core/runtime/JSArray.h (203146 => 203147)


--- trunk/Source/_javascript_Core/runtime/JSArray.h	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/Source/_javascript_Core/runtime/JSArray.h	2016-07-13 01:25:25 UTC (rev 203147)
@@ -353,6 +353,18 @@
     return lengthValue.toUInt32(exec);
 }
 
+ALWAYS_INLINE double toLength(ExecState* exec, JSObject* obj)
+{
+    if (isJSArray(obj))
+        return jsCast<JSArray*>(obj)->length();
+
+    VM& vm = exec->vm();
+    JSValue lengthValue = obj->get(exec, vm.propertyNames->length);
+    if (UNLIKELY(vm.exception()))
+        return PNaN;
+    return lengthValue.toLength(exec);
+}
+
 } // namespace JSC
 
 #endif // JSArray_h

Modified: trunk/Source/WTF/ChangeLog (203146 => 203147)


--- trunk/Source/WTF/ChangeLog	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/Source/WTF/ChangeLog	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,3 +1,14 @@
+2016-07-12  Benjamin Poulain  <[email protected]>
+
+        [JSC] Array.prototype.join() fails some conformance tests
+        https://bugs.webkit.org/show_bug.cgi?id=159657
+
+        Reviewed by Saam Barati.
+
+        * wtf/text/AtomicString.cpp:
+        (WTF::AtomicString::number):
+        * wtf/text/AtomicString.h:
+
 2016-07-12  Commit Queue  <[email protected]>
 
         Unreviewed, rolling out r203131.

Modified: trunk/Source/WTF/wtf/text/AtomicString.cpp (203146 => 203147)


--- trunk/Source/WTF/wtf/text/AtomicString.cpp	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/Source/WTF/wtf/text/AtomicString.cpp	2016-07-13 01:25:25 UTC (rev 203147)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2008, 2013-2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2008, 2013-2014, 2016 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Patrick Gansterer <[email protected]>
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
@@ -92,6 +92,16 @@
     return numberToStringUnsigned<AtomicString>(number);
 }
 
+AtomicString AtomicString::number(unsigned long number)
+{
+    return numberToStringUnsigned<AtomicString>(number);
+}
+
+AtomicString AtomicString::number(unsigned long long number)
+{
+    return numberToStringUnsigned<AtomicString>(number);
+}
+
 AtomicString AtomicString::number(double number)
 {
     NumberToStringBuffer buffer;

Modified: trunk/Source/WTF/wtf/text/AtomicString.h (203146 => 203147)


--- trunk/Source/WTF/wtf/text/AtomicString.h	2016-07-13 01:24:22 UTC (rev 203146)
+++ trunk/Source/WTF/wtf/text/AtomicString.h	2016-07-13 01:25:25 UTC (rev 203147)
@@ -105,6 +105,8 @@
 
     WTF_EXPORT_STRING_API static AtomicString number(int);
     WTF_EXPORT_STRING_API static AtomicString number(unsigned);
+    WTF_EXPORT_STRING_API static AtomicString number(unsigned long);
+    WTF_EXPORT_STRING_API static AtomicString number(unsigned long long);
     WTF_EXPORT_STRING_API static AtomicString number(double);
     // If we need more overloads of the number function, we can add all the others that String has, but these seem to do for now.
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to