Merge r176399 - WTFCrashWithSecurityImplication under SpeculativeJIT:…

…:compile() when loading a page from theblaze.com. <https://webkit.org/b/137642> Reviewed by Filip Pizlo. Source/JavaScriptCore: In the DFG, we have a ConstantFolding phase that occurs after all LocalCSE phases have already transpired. Hence, Identity nodes introduced in the ConstantFolding phase will be left in the node graph. Subsequently, the DFG code generator asserts that CSE phases have consumed all Identity nodes. This turns out to not be true. Hence, the crash. We fix this by teaching the DFG code generator to emit code for Identity nodes. Unlike the DFG, the FTL does not have this issue. That is because the FTL plan has GlobalCSE phases that come after ConstantFolding and any other phases that can generate Identity nodes. Hence, for the FTL, it is true that CSE will consume all Identity nodes, and the code generator should not see any Identity nodes. * dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): LayoutTests: * js/dfg-inline-identity-expected.txt: Added. * js/dfg-inline-identity.html: Added. * js/script-tests/dfg-inline-identity.js: Added. (o.toKey): (foo): (test): Canonical link: https://commits.webkit.org/154760.231@webkitgtk/2.6 git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@178264 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jan 12, 2015 · fb8cc1e · fb8cc1e
1 parent dbb2255
commit fb8cc1e
Show file tree

Hide file tree

Showing 7 changed files with 133 additions and 3 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2014-11-20  Mark Lam  <mark.lam@apple.com>
+
+        WTFCrashWithSecurityImplication under SpeculativeJIT::compile() when loading a page from theblaze.com.
+        <https://webkit.org/b/137642>
+
+        Reviewed by Filip Pizlo.
+
+        * js/dfg-inline-identity-expected.txt: Added.
+        * js/dfg-inline-identity.html: Added.
+        * js/script-tests/dfg-inline-identity.js: Added.
+        (o.toKey):
+        (foo):
+        (test):
+
 2014-11-18  Chris Dumez  <cdumez@apple.com>
 
         Crash when setting 'z-index' / 'flex-shrink' CSS properties to a calculated value

diff --git a/LayoutTests/js/dfg-inline-identity-expected.txt b/LayoutTests/js/dfg-inline-identity-expected.txt
@@ -0,0 +1,9 @@
+This tests that an identity node in the inlined function does not crash the DFG's code generator.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/dfg-inline-identity.html b/LayoutTests/js/dfg-inline-identity.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/dfg-inline-identity.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/script-tests/dfg-inline-identity.js b/LayoutTests/js/script-tests/dfg-inline-identity.js
@@ -0,0 +1,35 @@
+description(
+"This tests that an identity node in the inlined function does not crash the DFG's code generator."
+);
+
+var o = {
+    x1: 0,
+    x2: 0,
+    x3: 0,
+    toKey: function() {
+        return this.x1 + "," + this.x2 + "," + this.x3;
+    },
+};
+
+var a = [];
+
+var x1Adjust = 1.3;
+var x2Adjust = 2.7;
+var x3Adjust = 1.2;
+
+function foo(i) {
+    o.x1 += x1Adjust;
+    o.x2 += x2Adjust;
+    o.x3 += x3Adjust;
+
+    a[i] = o.toKey();
+}
+
+function test() {
+    for (var i = 0; i < 1000; i++)
+        foo(i);
+}
+
+test();
+
+var successfullyParsed = true;
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,28 @@
+2014-11-19  Mark Lam  <mark.lam@apple.com>
+
+        WTFCrashWithSecurityImplication under SpeculativeJIT::compile() when loading a page from theblaze.com.
+        <https://webkit.org/b/137642>
+
+        Reviewed by Filip Pizlo.
+
+        In the DFG, we have a ConstantFolding phase that occurs after all LocalCSE
+        phases have already transpired.  Hence, Identity nodes introduced in the
+        ConstantFolding phase will be left in the node graph.  Subsequently, the
+        DFG code generator asserts that CSE phases have consumed all Identity nodes.
+        This turns out to not be true.  Hence, the crash.  We fix this by teaching
+        the DFG code generator to emit code for Identity nodes.
+
+        Unlike the DFG, the FTL does not have this issue.  That is because the FTL
+        plan has GlobalCSE phases that come after ConstantFolding and any other
+        phases that can generate Identity nodes.  Hence, for the FTL, it is true that
+        CSE will consume all Identity nodes, and the code generator should not see any
+        Identity nodes.
+
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2014-11-01  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         REGRESSION(CMake): Make it possible to build without introspection

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
@@ -1692,7 +1692,26 @@ void SpeculativeJIT::compile(Node* node)
         break;
 
     case Identity: {
-        RELEASE_ASSERT_NOT_REACHED();
+        speculate(node, node->child1());
+        switch (node->child1().useKind()) {
+        case DoubleRepUse:
+        case DoubleRepRealUse: {
+            SpeculateDoubleOperand op(this, node->child1());
+            doubleResult(op.fpr(), node);
+            break;
+        }
+        case Int52RepUse: 
+        case MachineIntUse:
+        case DoubleRepMachineIntUse: {
+            RELEASE_ASSERT_NOT_REACHED();   
+            break;
+        }
+        default: {
+            JSValueOperand op(this, node->child1());
+            jsValueResult(op.tagGPR(), op.payloadGPR(), node);
+            break;
+        }
+        } // switch
         break;
     }
 

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -1795,8 +1795,26 @@ void SpeculativeJIT::compile(Node* node)
         break;
 
     case Identity: {
-        // CSE should always eliminate this.
-        DFG_CRASH(m_jit.graph(), node, "Unexpected Identity node");
+        speculate(node, node->child1());
+        switch (node->child1().useKind()) {
+        case DoubleRepUse:
+        case DoubleRepRealUse:
+        case DoubleRepMachineIntUse: {
+            SpeculateDoubleOperand op(this, node->child1());
+            doubleResult(op.fpr(), node);
+            break;
+        }
+        case Int52RepUse: {
+            SpeculateInt52Operand op(this, node->child1());
+            int52Result(op.gpr(), node);
+            break;
+        }
+        default: {
+            JSValueOperand op(this, node->child1());
+            jsValueResult(op.gpr(), node);
+            break;
+        }
+        } // switch
         break;
     }