Array.prototype.indexOf fast path needs to ensure the length is still valid after...

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/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236437 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 06b9ff8..95ce2ed 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,14 @@
+2018-09-24  Saam Barati  <sbarati@apple.com>
+
+        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  <sbarati@apple.com>
 
         ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
 2018-09-24  Saam barati  <sbarati@apple.com>
 
         ArgumentsEliminationPhase should snip basic blocks after proven OSR exits

diff --git a/JSTests/stress/array-indexof-cached-length.js b/JSTests/stress/array-indexof-cached-length.js
new file mode 100644 (file)
index 0000000..602f8a2
--- /dev/null
+++ b/JSTests/stress/array-indexof-cached-length.js
@@ -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;
+    }
+});

diff --git a/JSTests/stress/array-indexof-fast-path-effects.js b/JSTests/stress/array-indexof-fast-path-effects.js
new file mode 100644 (file)
index 0000000..2dac83c
--- /dev/null
+++ b/JSTests/stress/array-indexof-fast-path-effects.js
@@ -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;
+    }
+});

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index a5eccf3..7d33a4c 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2018-09-24  Saam Barati  <sbarati@apple.com>
+
+        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  <tzagallo@apple.com>
 
         offlineasm: fix macro scoping
 2018-09-24  Tadeu Zagallo  <tzagallo@apple.com>
 
         offlineasm: fix macro scoping

diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
index f714c1f..31a03a7 100644 (file)
--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
@@ -1169,7 +1169,9 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncIndexOf(ExecState* exec)
 
     if (isJSArray(thisObject)) {
         JSArray* array = asArray(thisObject);
 
     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())
             switch (array->indexingType()) {
             case ALL_INT32_INDEXING_TYPES: {
                 if (!searchElement.isNumber())