Merge r229850 - Race Condition in arrayProtoFuncReverse() causes wron…

…g results or crash

https://bugs.webkit.org/show_bug.cgi?id=183901

Reviewed by Keith Miller.

JSTests:

New test.

* stress/array-reverse-doesnt-clobber.js: Added.
(testArrayReverse):
(createArrayOfArrays):
(createArrayStorage):

Source/JavaScriptCore:

Added write barriers to ensure the reversed contents are properly marked.

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

JSTests/ChangeLog

@@ -1,3 +1,17 @@
+2018-03-22  Michael Saboff  <msaboff@apple.com>
+
+        Race Condition in arrayProtoFuncReverse() causes wrong results or crash
+        https://bugs.webkit.org/show_bug.cgi?id=183901
+
+        Reviewed by Keith Miller.
+
+        New test.
+
+        * stress/array-reverse-doesnt-clobber.js: Added.
+        (testArrayReverse):
+        (createArrayOfArrays):
+        (createArrayStorage):
+
 2018-03-01  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         ASSERTION FAILED: matchContextualKeyword(m_vm->propertyNames->async)

JSTests/stress/array-reverse-doesnt-clobber.js

@@ -0,0 +1,61 @@
+// This tests that array.Prototype.reverse() doesn't inadvertently clobber indexed properties.
+// This test shouldn't throw or crash.
+
+const outerArrayLength = 10000;
+const innerArrayLength = 128;
+
+function testArrayReverse(createArray)
+{
+    const limit = 5;
+    let save = [0, 0];
+
+    for (let at = 0; at < limit; at++) {
+        let arr = createArray();
+
+        let v = [];
+        for (let i = 0; i < 273; i++) {
+            for (let j = 0; j < 8; j++)
+                arr.reverse();
+
+            v.push(new String("X").repeat(123008));
+        }
+
+        for (let i = 0; i < arr.length; i++) {
+            if (arr[i].length != innerArrayLength)
+                throw "arr[" + i + "].length has changed from " + innerArrayLength + " to " + arr[i].length;
+        }
+
+        let f = [];
+        for (let i = 0; i < 1000; i++)
+            f.push(new Array(16).fill(0x42424242));
+
+        save.push(arr);
+        save.push(v);
+        save.push(f);
+    }
+}
+
+function createArrayOfArrays()
+{
+    let result = new Array(outerArrayLength);
+
+    for (let i = 0; i < result.length; i++)
+        result[i] = new Array(innerArrayLength).fill(0x41414141);
+
+    return result;
+}
+
+var alt = 0;
+
+function createArrayStorage()
+{
+    let result = createArrayOfArrays();
+
+    if (!(typeof ensureArrayStorage === undefined) && alt++ % 0)
+        ensureArrayStorage(result);
+
+    return result;
+}
+
+testArrayReverse(createArrayOfArrays);
+testArrayReverse(createArrayStorage);

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,15 @@
+2018-03-22  Michael Saboff  <msaboff@apple.com>
+
+        Race Condition in arrayProtoFuncReverse() causes wrong results or crash
+        https://bugs.webkit.org/show_bug.cgi?id=183901
+
+        Reviewed by Keith Miller.
+
+        Added write barriers to ensure the reversed contents are properly marked.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncReverse):
+
 2018-03-05  Mark Lam  <mark.lam@apple.com>
 
         JITThunk functions should only be called when the JIT is enabled.

Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -837,6 +837,8 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncReverse(ExecState* exec)
         if (containsHole(data, length) && holesMustForwardToPrototype(vm, thisObject))
             break;
         std::reverse(data, data + length);
+        if (!hasInt32(thisObject->indexingType()))
+            vm.heap.writeBarrier(thisObject);
         return JSValue::encode(thisObject);
     }
     case ALL_DOUBLE_INDEXING_TYPES: {
@@ -857,6 +859,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncReverse(ExecState* exec)
             break;
         auto data = storage.vector().data();
         std::reverse(data, data + length);
+        vm.heap.writeBarrier(thisObject);
         return JSValue::encode(thisObject);
     }
     }