Remove an invalid assertion in the DFG backend's GetById emitter.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Aug 2016 21:00:45 +0000 (21:00 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Aug 2016 21:00:45 +0000 (21:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160925
<rdar://problem/27248961>

Reviewed by Filip Pizlo.

JSTests:

* stress/dfg-get-by-id-should-not-assert-non-null-prediction.js: Added.

Source/JavaScriptCore:

The DFG backend's GetById assertion that the node's prediction not be SpecNone
is just plain wrong.  It assumes that we can never have a GetById node without a
type prediction, but this is not true.  The following test case proves otherwise:

    function foo() {
        "use strict";
        return --arguments["callee"];
    }

Will remove the assertion.  Nothing else needs to change as the DFG is working
correctly without the assertion.

* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):

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

JSTests/ChangeLog
JSTests/stress/dfg-get-by-id-should-not-assert-non-null-prediction.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

index 1d2002e..7dceec6 100644 (file)
@@ -1,3 +1,13 @@
+2016-08-17  Mark Lam  <mark.lam@apple.com>
+
+        Remove an invalid assertion in the DFG backend's GetById emitter.
+        https://bugs.webkit.org/show_bug.cgi?id=160925
+        <rdar://problem/27248961>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/dfg-get-by-id-should-not-assert-non-null-prediction.js: Added.
+
 2016-08-16  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r204464.
diff --git a/JSTests/stress/dfg-get-by-id-should-not-assert-non-null-prediction.js b/JSTests/stress/dfg-get-by-id-should-not-assert-non-null-prediction.js
new file mode 100644 (file)
index 0000000..b6fac02
--- /dev/null
@@ -0,0 +1,18 @@
+//@ runDefault
+// This test should not crash.
+
+function foo() {
+    "use strict";
+    return --arguments["callee"];
+};
+
+function test() {
+    for (var i = 0; i < 10000; i++) {
+        try {
+            foo();
+        } catch(e) {
+        }
+    }
+}
+
+test();
index 7952bbc..0b1523d 100644 (file)
@@ -1,3 +1,28 @@
+2016-08-17  Mark Lam  <mark.lam@apple.com>
+
+        Remove an invalid assertion in the DFG backend's GetById emitter.
+        https://bugs.webkit.org/show_bug.cgi?id=160925
+        <rdar://problem/27248961>
+
+        Reviewed by Filip Pizlo.
+
+        The DFG backend's GetById assertion that the node's prediction not be SpecNone
+        is just plain wrong.  It assumes that we can never have a GetById node without a
+        type prediction, but this is not true.  The following test case proves otherwise:
+
+            function foo() {
+                "use strict";
+                return --arguments["callee"];
+            }
+
+        Will remove the assertion.  Nothing else needs to change as the DFG is working
+        correctly without the assertion.
+
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2016-08-16  Mark Lam  <mark.lam@apple.com>
 
         Heap::collectAllGarbage() should work with JSC_useImmortalObjects=true.
index af4217a..87e1515 100644 (file)
@@ -4175,8 +4175,6 @@ void SpeculativeJIT::compile(Node* node)
     }
 
     case GetById: {
-        ASSERT(node->prediction());
-        
         switch (node->child1().useKind()) {
         case CellUse: {
             SpeculateCellOperand base(this, node->child1());
index dd6a68f..5be7a5e 100644 (file)
@@ -4120,8 +4120,6 @@ void SpeculativeJIT::compile(Node* node)
     }
 
     case GetById: {
-        ASSERT(node->prediction());
-
         switch (node->child1().useKind()) {
         case CellUse: {
             SpeculateCellOperand base(this, node->child1());