[JSC] CheckArray+NonArray is not filtering out Array in AI
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Sep 2019 19:52:43 +0000 (19:52 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Sep 2019 19:52:43 +0000 (19:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201857
<rdar://problem/54194820>

Reviewed by Keith Miller.

JSTests:

* stress/check-array-with-non-array-does-not-filter-arrays.js: Added.
(foo):

Source/JavaScriptCore:

The code of DFG::ArrayMode::alreadyChecked is different from SpeculativeJIT's CheckArray / CheckStructure.
While we assume CheckArray+NonArray ensures it only passes non-array inputs, DFG::ArrayMode::alreadyChecked
accepts arrays too. So CheckArray+NonArray is removed in AI if the input is proven that it is an array.
This patch aligns DFG::ArrayMode::alreadyChecked to the checks done at runtime.

* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::alreadyChecked const):

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

JSTests/ChangeLog
JSTests/stress/check-array-with-non-array-does-not-filter-arrays.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGArrayMode.cpp

index 88882fa..9c6444d 100644 (file)
@@ -1,3 +1,14 @@
+2019-09-17  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] CheckArray+NonArray is not filtering out Array in AI
+        https://bugs.webkit.org/show_bug.cgi?id=201857
+        <rdar://problem/54194820>
+
+        Reviewed by Keith Miller.
+
+        * stress/check-array-with-non-array-does-not-filter-arrays.js: Added.
+        (foo):
+
 2019-09-17  Saam Barati  <sbarati@apple.com>
 
         CheckArray on DirectArguments/ScopedArguments does not filter out slow put array storage
diff --git a/JSTests/stress/check-array-with-non-array-does-not-filter-arrays.js b/JSTests/stress/check-array-with-non-array-does-not-filter-arrays.js
new file mode 100644 (file)
index 0000000..b630993
--- /dev/null
@@ -0,0 +1,16 @@
+//@ runDefault("--thresholdForFTLOptimizeAfterWarmUp=0", "--useConcurrentJIT=0")
+
+function foo(...a) {
+    for (const w of a) {
+        for (const q of arguments) {
+        }
+    }
+}
+
+foo(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13);
+
+for (let i = 0; i < 1000; i++) {
+    foo();
+}
+
+foo(0);
index 2367655..25d8bd3 100644 (file)
@@ -1,3 +1,19 @@
+2019-09-17  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] CheckArray+NonArray is not filtering out Array in AI
+        https://bugs.webkit.org/show_bug.cgi?id=201857
+        <rdar://problem/54194820>
+
+        Reviewed by Keith Miller.
+
+        The code of DFG::ArrayMode::alreadyChecked is different from SpeculativeJIT's CheckArray / CheckStructure.
+        While we assume CheckArray+NonArray ensures it only passes non-array inputs, DFG::ArrayMode::alreadyChecked
+        accepts arrays too. So CheckArray+NonArray is removed in AI if the input is proven that it is an array.
+        This patch aligns DFG::ArrayMode::alreadyChecked to the checks done at runtime.
+
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::ArrayMode::alreadyChecked const):
+
 2019-09-17  Saam Barati  <sbarati@apple.com>
 
         CheckArray on DirectArguments/ScopedArguments does not filter out slow put array storage
index c0b4377..6db0d2b 100644 (file)
@@ -417,55 +417,67 @@ Structure* ArrayMode::originalArrayStructure(Graph& graph, Node* node) const
 
 bool ArrayMode::alreadyChecked(Graph& graph, Node* node, const AbstractValue& value, IndexingType shape) const
 {
+    ASSERT(isSpecific());
+
+    IndexingType indexingModeMask = IsArray | IndexingShapeMask;
+    if (action() == Array::Write)
+        indexingModeMask |= CopyOnWrite;
+
     switch (arrayClass()) {
-    case Array::OriginalArray: {
-        if (value.m_structure.isTop())
+    case Array::Array: {
+        if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModesIgnoringTypedArrays(shape | IsArray)))
+            return true;
+        if (!value.m_structure.isFinite())
             return false;
         for (unsigned i = value.m_structure.size(); i--;) {
             RegisteredStructure structure = value.m_structure[i];
-            if ((structure->indexingType() & IndexingShapeMask) != shape)
-                return false;
-            if (isCopyOnWrite(structure->indexingMode()) && action() == Array::Write)
-                return false;
-            if (!(structure->indexingType() & IsArray))
-                return false;
-            if (!graph.globalObjectFor(node->origin.semantic)->isOriginalArrayStructure(structure.get()))
+            if ((structure->indexingMode() & indexingModeMask) != (shape | IsArray))
                 return false;
         }
         return true;
     }
-        
-    case Array::Array: {
-        if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModesIgnoringTypedArrays(shape | IsArray)))
+
+    // Array::OriginalNonArray can be shown when the value is a TypedArray with original structure.
+    // But here, we already filtered TypedArrays. So, just handle it like a NonArray.
+    case Array::OriginalNonArray:
+    case Array::NonArray: {
+        if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModesIgnoringTypedArrays(shape)))
             return true;
-        if (value.m_structure.isTop())
+        if (!value.m_structure.isFinite())
             return false;
         for (unsigned i = value.m_structure.size(); i--;) {
             RegisteredStructure structure = value.m_structure[i];
-            if ((structure->indexingMode() & IndexingShapeMask) != shape)
-                return false;
-            if (isCopyOnWrite(structure->indexingMode()) && action() == Array::Write)
-                return false;
-            if (!(structure->indexingType() & IsArray))
+            if ((structure->indexingMode() & indexingModeMask) != shape)
                 return false;
         }
         return true;
     }
-        
-    default: {
+
+    case Array::PossiblyArray: {
         if (arrayModesAlreadyChecked(value.m_arrayModes, asArrayModesIgnoringTypedArrays(shape) | asArrayModesIgnoringTypedArrays(shape | IsArray)))
             return true;
-        if (value.m_structure.isTop())
+        if (!value.m_structure.isFinite())
             return false;
         for (unsigned i = value.m_structure.size(); i--;) {
             RegisteredStructure structure = value.m_structure[i];
-            if ((structure->indexingMode() & IndexingShapeMask) != shape)
-                return false;
-            if (isCopyOnWrite(structure->indexingMode()) && action() == Array::Write)
+            if ((structure->indexingMode() & (indexingModeMask & ~IsArray)) != shape)
                 return false;
         }
         return true;
-    } }
+    }
+
+    // If ArrayMode is Array::OriginalCopyOnWriteArray or Array::OriginalArray, CheckArray is never emitted. Instead, we always emit CheckStructure.
+    // So, we should perform the same check to the CheckStructure here.
+    case Array::OriginalArray:
+    case Array::OriginalCopyOnWriteArray: {
+        if (!value.m_structure.isFinite())
+            return false;
+        Structure* originalStructure = originalArrayStructure(graph, node);
+        if (value.m_structure.size() != 1)
+            return false;
+        return value.m_structure.onlyStructure().get() == originalStructure;
+    }
+    }
 }
 
 bool ArrayMode::alreadyChecked(Graph& graph, Node* node, const AbstractValue& value) const