[JSC] AI should not propagate AbstractValue relying on constant folding phase
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Mar 2019 22:34:30 +0000 (22:34 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Mar 2019 22:34:30 +0000 (22:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195375

Reviewed by Saam Barati.

JSTests:

* stress/make-rope-should-not-propagate-constant-folded-value-in-ai.js: Added.
(let.array):

Source/JavaScriptCore:

MakeRope rule in AI attempts to propagate the node, which will be produced after constant folding phase runs.
This is wrong since we do not guarantee that constant folding phase runs after AI runs (e.g. DFGSpeculativeJIT
and FTLLowerDFGToB3 run AI). This results in the bug that the value produced at runtime is different from the
proven constant value in AI. In the attached test, AI says the value is SpecStringIdent while the resulted value
at runtime is SpecStringVar, resulting in wrong MakeRope code. This patch removes the path propagating the node
relying on constant folding phase.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

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

JSTests/ChangeLog
JSTests/stress/make-rope-should-not-propagate-constant-folded-value-in-ai.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

index be003d3..10cff83 100644 (file)
@@ -1,3 +1,13 @@
+2019-03-06  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] AI should not propagate AbstractValue relying on constant folding phase
+        https://bugs.webkit.org/show_bug.cgi?id=195375
+
+        Reviewed by Saam Barati.
+
+        * stress/make-rope-should-not-propagate-constant-folded-value-in-ai.js: Added.
+        (let.array):
+
 2019-03-05  Saam barati  <sbarati@apple.com>
 
         op_switch_char broken for rope strings after JSRopeString layout rewrite
diff --git a/JSTests/stress/make-rope-should-not-propagate-constant-folded-value-in-ai.js b/JSTests/stress/make-rope-should-not-propagate-constant-folded-value-in-ai.js
new file mode 100644 (file)
index 0000000..12b5d8c
--- /dev/null
@@ -0,0 +1,14 @@
+//@ runDefault("--forceEagerCompilation=1")
+let string = 'a';
+let array = []
+
+function test() {
+    let x = [][array.length];
+    let y = x ? x + '' : '';
+    let z = string + y;
+    let za = z + 'a';
+    [].join(za);
+}
+
+for (let i=0; i< 1e4; i++)
+    test();
index 82c7335..abe4056 100644 (file)
@@ -1,3 +1,20 @@
+2019-03-06  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] AI should not propagate AbstractValue relying on constant folding phase
+        https://bugs.webkit.org/show_bug.cgi?id=195375
+
+        Reviewed by Saam Barati.
+
+        MakeRope rule in AI attempts to propagate the node, which will be produced after constant folding phase runs.
+        This is wrong since we do not guarantee that constant folding phase runs after AI runs (e.g. DFGSpeculativeJIT
+        and FTLLowerDFGToB3 run AI). This results in the bug that the value produced at runtime is different from the
+        proven constant value in AI. In the attached test, AI says the value is SpecStringIdent while the resulted value
+        at runtime is SpecStringVar, resulting in wrong MakeRope code. This patch removes the path propagating the node
+        relying on constant folding phase.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+
 2019-03-05  Saam barati  <sbarati@apple.com>
 
         op_switch_char broken for rope strings after JSRopeString layout rewrite
index 67e5a87..236360e 100644 (file)
@@ -733,19 +733,8 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
             ++numberOfRemovedChildren;
         }
 
-        if (numberOfRemovedChildren) {
+        if (numberOfRemovedChildren)
             m_state.setFoundConstants(true);
-            if (numberOfRemovedChildren == numberOfChildren) {
-                // Propagate the last child. This is the way taken in the constant folding phase.
-                setForNode(node, forNode(node->children.child(numberOfChildren - 1)));
-                break;
-            }
-            if ((numberOfRemovedChildren + 1) == numberOfChildren) {
-                ASSERT(nonEmptyIndex);
-                setForNode(node, forNode(node->children.child(nonEmptyIndex.value())));
-                break;
-            }
-        }
         setForNode(node, m_vm.stringStructure.get());
         break;
     }