[JSC] GetByVal on Undecided use its children before its OSR Exit

https://bugs.webkit.org/show_bug.cgi?id=157046

Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-04-26
Reviewed by Mark Lam.

Very silly bug: GetByVal on Undecided uses its children before
the speculationCheck(). If we fail the speculation, we have already
lost how to recover the values.

The existing tests did not catch this because we tier up to B3
before such Exits happen. B3 has explicit liveness and did not suffer
from this bug.
The new test has a smaller warmup to exercise the OSR Exit in DFG
instead of FTL.

* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* tests/stress/get-by-val-on-undecided-out-of-bounds.js: Added.
(string_appeared_here.opaqueGetByValKnownArray):

Canonical link: https://commits.webkit.org/175168@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200113 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,25 @@
+2016-04-26  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] GetByVal on Undecided use its children before its OSR Exit
+        https://bugs.webkit.org/show_bug.cgi?id=157046
+
+        Reviewed by Mark Lam.
+
+        Very silly bug: GetByVal on Undecided uses its children before
+        the speculationCheck(). If we fail the speculation, we have already
+        lost how to recover the values.
+
+        The existing tests did not catch this because we tier up to B3
+        before such Exits happen. B3 has explicit liveness and did not suffer
+        from this bug.
+        The new test has a smaller warmup to exercise the OSR Exit in DFG
+        instead of FTL.
+
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * tests/stress/get-by-val-on-undecided-out-of-bounds.js: Added.
+        (string_appeared_here.opaqueGetByValKnownArray):
+
 2016-04-26  Skachkov Oleksandr  <gskachkov@gmail.com>
 
         calling super() a second time in a constructor should throw

Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

@@ -2554,12 +2554,12 @@ void SpeculativeJIT::compile(Node* node)
             GPRReg indexGPR = index.gpr();
             GPRReg resultGPR = result.gpr();
 
-            use(node->child1());
-            index.use();
-
             speculationCheck(OutOfBounds, JSValueRegs(), node,
                 m_jit.branch32(MacroAssembler::LessThan, indexGPR, MacroAssembler::TrustedImm32(0)));
 
+            use(node->child1());
+            index.use();
+
             m_jit.move(MacroAssembler::TrustedImm64(ValueUndefined), resultGPR);
             jsValueResult(resultGPR, node, UseChildrenCalledExplicitly);
             break;

Source/JavaScriptCore/tests/stress/get-by-val-on-undecided-out-of-bounds.js

@@ -0,0 +1,20 @@
+"use strict"
+
+function opaqueGetByValKnownArray(value)
+{
+    let array = [];
+    return array[value];
+}
+noInline(opaqueGetByValKnownArray);
+
+// Warm up without out-of-bounds access.
+for (let i = 0; i < 1e3; ++i) {
+    if (opaqueGetByValKnownArray(0) !== undefined)
+        throw "Failed opaqueGetByValKnownArray(0)";
+}
+
+// Then access out of bounds.
+for (let i = 0; i < 1e3; ++i) {
+    if (opaqueGetByValKnownArray(-1) !== undefined)
+        throw "Failed opaqueGetByValKnownArray(-1)";
+}