Title: [229850] trunk
- Revision
- 229850
- Author
- msab...@apple.com
- Date
- 2018-03-22 08:12:44 -0700 (Thu, 22 Mar 2018)
Log Message
Race Condition in arrayProtoFuncReverse() causes wrong 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/_javascript_Core:
Added write barriers to ensure the reversed contents are properly marked.
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncReverse):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (229849 => 229850)
--- trunk/JSTests/ChangeLog 2018-03-22 14:54:12 UTC (rev 229849)
+++ trunk/JSTests/ChangeLog 2018-03-22 15:12:44 UTC (rev 229850)
@@ -1,3 +1,17 @@
+2018-03-22 Michael Saboff <msab...@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-21 Filip Pizlo <fpi...@apple.com>
ScopedArguments should do poisoning and index masking
Added: trunk/JSTests/stress/array-reverse-doesnt-clobber.js (0 => 229850)
--- trunk/JSTests/stress/array-reverse-doesnt-clobber.js (rev 0)
+++ trunk/JSTests/stress/array-reverse-doesnt-clobber.js 2018-03-22 15:12:44 UTC (rev 229850)
@@ -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);
Modified: trunk/Source/_javascript_Core/ChangeLog (229849 => 229850)
--- trunk/Source/_javascript_Core/ChangeLog 2018-03-22 14:54:12 UTC (rev 229849)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-03-22 15:12:44 UTC (rev 229850)
@@ -1,3 +1,15 @@
+2018-03-22 Michael Saboff <msab...@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-21 Filip Pizlo <fpi...@apple.com>
ScopedArguments should do poisoning and index masking
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (229849 => 229850)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2018-03-22 14:54:12 UTC (rev 229849)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2018-03-22 15:12:44 UTC (rev 229850)
@@ -839,6 +839,8 @@
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: {
@@ -859,6 +861,7 @@
break;
auto data = ""
std::reverse(data, data + length);
+ vm.heap.writeBarrier(thisObject);
return JSValue::encode(thisObject);
}
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes