Title: [236437] trunk
- Revision
- 236437
- Author
- [email protected]
- Date
- 2018-09-24 16:05:54 -0700 (Mon, 24 Sep 2018)
Log Message
Array.prototype.indexOf fast path needs to ensure the length is still valid after performing effects
https://bugs.webkit.org/show_bug.cgi?id=189922
<rdar://problem/44651275>
Reviewed by Mark Lam.
JSTests:
* stress/array-indexof-fast-path-effects.js: Added.
* stress/array-indexof-cached-length.js: Added.
Source/_javascript_Core:
The implementation was first getting the length to iterate up to,
then getting the starting index. However, getting the starting
index may perform effects. e.g, it could change the length of the
array. This changes it so we verify the length is still valid.
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncIndexOf):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (236436 => 236437)
--- trunk/JSTests/ChangeLog 2018-09-24 23:04:56 UTC (rev 236436)
+++ trunk/JSTests/ChangeLog 2018-09-24 23:05:54 UTC (rev 236437)
@@ -1,3 +1,14 @@
+2018-09-24 Saam Barati <[email protected]>
+
+ Array.prototype.indexOf fast path needs to ensure the length is still valid after performing effects
+ https://bugs.webkit.org/show_bug.cgi?id=189922
+ <rdar://problem/44651275>
+
+ Reviewed by Mark Lam.
+
+ * stress/array-indexof-fast-path-effects.js: Added.
+ * stress/array-indexof-cached-length.js: Added.
+
2018-09-24 Saam barati <[email protected]>
ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
Added: trunk/JSTests/stress/array-indexof-cached-length.js (0 => 236437)
--- trunk/JSTests/stress/array-indexof-cached-length.js (rev 0)
+++ trunk/JSTests/stress/array-indexof-cached-length.js 2018-09-24 23:05:54 UTC (rev 236437)
@@ -0,0 +1,24 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+
+}
+
+const originalLength = 10000;
+let arr = new Proxy([], {
+ has(...args) {
+ assert(parseInt(args[1]) < originalLength);
+ assert(args[0].length - 10 === originalLength);
+ return Reflect.has(...args);
+ }
+});
+
+for (var i = 0; i < originalLength; i++)
+ arr[i] = [];
+
+arr.indexOf(new Object(), {
+ valueOf: function () {
+ arr.length += 10;
+ return 0;
+ }
+});
Added: trunk/JSTests/stress/array-indexof-fast-path-effects.js (0 => 236437)
--- trunk/JSTests/stress/array-indexof-fast-path-effects.js (rev 0)
+++ trunk/JSTests/stress/array-indexof-fast-path-effects.js 2018-09-24 23:05:54 UTC (rev 236437)
@@ -0,0 +1,11 @@
+// This shouldn't crash when running with ASAN.
+let arr = [];
+for (var i = 0; i < 1000000; i++)
+ arr[i] = [];
+
+arr.indexOf(new Object(), {
+ valueOf: function () {
+ arr.length = 0;
+ return 0;
+ }
+});
Modified: trunk/Source/_javascript_Core/ChangeLog (236436 => 236437)
--- trunk/Source/_javascript_Core/ChangeLog 2018-09-24 23:04:56 UTC (rev 236436)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-09-24 23:05:54 UTC (rev 236437)
@@ -1,3 +1,19 @@
+2018-09-24 Saam Barati <[email protected]>
+
+ Array.prototype.indexOf fast path needs to ensure the length is still valid after performing effects
+ https://bugs.webkit.org/show_bug.cgi?id=189922
+ <rdar://problem/44651275>
+
+ Reviewed by Mark Lam.
+
+ The implementation was first getting the length to iterate up to,
+ then getting the starting index. However, getting the starting
+ index may perform effects. e.g, it could change the length of the
+ array. This changes it so we verify the length is still valid.
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::arrayProtoFuncIndexOf):
+
2018-09-24 Tadeu Zagallo <[email protected]>
offlineasm: fix macro scoping
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (236436 => 236437)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2018-09-24 23:04:56 UTC (rev 236436)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2018-09-24 23:05:54 UTC (rev 236437)
@@ -1169,7 +1169,9 @@
if (isJSArray(thisObject)) {
JSArray* array = asArray(thisObject);
- if (array->canDoFastIndexedAccess(vm)) {
+ bool canDoFastPath = array->canDoFastIndexedAccess(vm)
+ && array->getArrayLength() == length; // The effects in getting `index` could have changed the length of this array.
+ if (canDoFastPath) {
switch (array->indexingType()) {
case ALL_INT32_INDEXING_TYPES: {
if (!searchElement.isNumber())
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
