[FTL] inlined GetMyArgumentByVal with no arguments passed causes instant crash

https://bugs.webkit.org/show_bug.cgi?id=141180
rdar://problem/19677552

Reviewed by Benjamin Poulain.

If we do a GetMyArgumentByVal on an inlined call frame that has no arguments, then the
bounds check already terminates execution. This means we can skip the part where we
previously did an out-of-bound array access on the inlined call frame arguments vector.

* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::safelyInvalidateAfterTermination):
(JSC::FTL::LowerDFGToLLVM::compileGetMyArgumentByVal):
(JSC::FTL::LowerDFGToLLVM::terminate):
(JSC::FTL::LowerDFGToLLVM::didAlreadyTerminate):
(JSC::FTL::LowerDFGToLLVM::crash):
* tests/stress/get-my-argument-by-val-inlined-no-formal-parameters.js: Added.
(foo):
(bar):

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 944cf32..cfd5b06 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,27 @@
 2015-02-02  Filip Pizlo  <fpizlo@apple.com>
 
+        [FTL] inlined GetMyArgumentByVal with no arguments passed causes instant crash
+        https://bugs.webkit.org/show_bug.cgi?id=141180
+        rdar://problem/19677552
+
+        Reviewed by Benjamin Poulain.
+        
+        If we do a GetMyArgumentByVal on an inlined call frame that has no arguments, then the
+        bounds check already terminates execution. This means we can skip the part where we
+        previously did an out-of-bound array access on the inlined call frame arguments vector.
+
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::safelyInvalidateAfterTermination):
+        (JSC::FTL::LowerDFGToLLVM::compileGetMyArgumentByVal):
+        (JSC::FTL::LowerDFGToLLVM::terminate):
+        (JSC::FTL::LowerDFGToLLVM::didAlreadyTerminate):
+        (JSC::FTL::LowerDFGToLLVM::crash):
+        * tests/stress/get-my-argument-by-val-inlined-no-formal-parameters.js: Added.
+        (foo):
+        (bar):
+
+2015-02-02  Filip Pizlo  <fpizlo@apple.com>
+
         REGRESSION(r179477): arguments simplification no longer works
         https://bugs.webkit.org/show_bug.cgi?id=141169
 

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
index 1166fb9..cf89b2d 100644 (file)
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
@@ -311,7 +311,7 @@ private:
     {
         if (verboseCompilationEnabled())
             dataLog("Bailing.\n");
-        crash(m_highBlock->index, m_node->index());
+        crash();
 
         // Invalidate dominated blocks. Under normal circumstances we would expect
         // them to be invalidated already. But you can have the CFA become more
@@ -2017,9 +2017,18 @@ private:
         }
         
         TypedPointer base;
-        if (codeOrigin.inlineCallFrame)
+        if (codeOrigin.inlineCallFrame) {
+            if (codeOrigin.inlineCallFrame->arguments.size() <= 1) {
+                // We should have already exited due to the bounds check, above. Just tell the
+                // compiler that anything dominated by this instruction is not reachable, so
+                // that we don't waste time generating such code. This will also plant some
+                // kind of crashing instruction so that if by some fluke the bounds check didn't
+                // work, we'll crash in an easy-to-see way.
+                didAlreadyTerminate();
+                return;
+            }
             base = addressFor(codeOrigin.inlineCallFrame->arguments[1].virtualRegister());
-        else
+        } else
             base = addressFor(virtualRegisterForArgument(1));
         
         LValue pointer = m_out.baseIndex(
@@ -5426,6 +5435,11 @@ private:
     void terminate(ExitKind kind)
     {
         speculate(kind, noValue(), nullptr, m_out.booleanTrue);
+        didAlreadyTerminate();
+    }
+    
+    void didAlreadyTerminate()
+    {
         m_state.setIsValid(false);
     }
     
@@ -6889,6 +6903,10 @@ private:
         return addressFor(operand, TagOffset);
     }
     
+    void crash()
+    {
+        crash(m_highBlock->index, m_node->index());
+    }
     void crash(BlockIndex blockIndex, unsigned nodeIndex)
     {
 #if ASSERT_DISABLED

diff --git a/Source/JavaScriptCore/tests/stress/get-my-argument-by-val-inlined-no-formal-parameters.js b/Source/JavaScriptCore/tests/stress/get-my-argument-by-val-inlined-no-formal-parameters.js
new file mode 100644 (file)
index 0000000..eef298f
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/get-my-argument-by-val-inlined-no-formal-parameters.js
@@ -0,0 +1,33 @@
+var index;
+
+function foo() {
+    if (index >= 0)
+        return arguments[index];
+    else
+        return 13;
+}
+
+function bar() {
+    return foo();
+}
+
+noInline(bar);
+
+for (var i = 0; i < 100; ++i) {
+    index = i & 1;
+    var result = foo(42, 53);
+    if (result != [42, 53][index])
+        throw "Error: bad result in first loop: " + result;
+}
+
+for (var i = 0; i < 100000; ++i) {
+    index = -(i & 1) - 1;
+    var result = bar();
+    if (result !== 13)
+        throw "Error: bad result in second loop: " + result;
+}
+
+index = 0;
+var result = bar();
+if (result !== void 0)
+    throw "Error: bad result at end: " + result;