Title: [200645] trunk/Source/_javascript_Core
- Revision
- 200645
- Author
- [email protected]
- Date
- 2016-05-10 14:35:32 -0700 (Tue, 10 May 2016)
Log Message
[JSC] FTL can produce GetByVal nodes without proper bounds checking
https://bugs.webkit.org/show_bug.cgi?id=157502
rdar://problem/26027027
Patch by Benjamin Poulain <[email protected]> on 2016-05-10
Reviewed by Filip Pizlo.
It was possible for FTL to generates GetByVal on arbitrary offsets
without any bounds checking.
The bug is caused by the order of optimization phases:
-First, the Integer Range Optimization proves that a CheckInBounds
test can never fail.
This proof is based on control flow or preceeding instructions
inside a loop.
-The Loop Invariant Code Motion phase finds that the GetByVal does not
depend on anything in the loop and hoist it out of the loop.
-> As a result, the conditions that were necessary to eliminate
the CheckInBounds are no longer met before the GetByVal.
This patch just moves the Integer Range Optimization phase after
Loop Invariant Code Motion to make sure no code is moved after
its integer ranges bounds proofs have been used.
* dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* tests/stress/bounds-check-not-eliminated-by-licm.js: Added.
(testInLoopTests):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (200644 => 200645)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-10 21:30:09 UTC (rev 200644)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-10 21:35:32 UTC (rev 200645)
@@ -1,3 +1,33 @@
+2016-05-10 Benjamin Poulain <[email protected]>
+
+ [JSC] FTL can produce GetByVal nodes without proper bounds checking
+ https://bugs.webkit.org/show_bug.cgi?id=157502
+ rdar://problem/26027027
+
+ Reviewed by Filip Pizlo.
+
+ It was possible for FTL to generates GetByVal on arbitrary offsets
+ without any bounds checking.
+
+ The bug is caused by the order of optimization phases:
+ -First, the Integer Range Optimization proves that a CheckInBounds
+ test can never fail.
+ This proof is based on control flow or preceeding instructions
+ inside a loop.
+ -The Loop Invariant Code Motion phase finds that the GetByVal does not
+ depend on anything in the loop and hoist it out of the loop.
+ -> As a result, the conditions that were necessary to eliminate
+ the CheckInBounds are no longer met before the GetByVal.
+
+ This patch just moves the Integer Range Optimization phase after
+ Loop Invariant Code Motion to make sure no code is moved after
+ its integer ranges bounds proofs have been used.
+
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::compileInThreadImpl):
+ * tests/stress/bounds-check-not-eliminated-by-licm.js: Added.
+ (testInLoopTests):
+
2016-05-10 Joseph Pecoraro <[email protected]>
Web Inspector: Eliminate the crazy code for evaluateOnCallFrame
Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.cpp (200644 => 200645)
--- trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2016-05-10 21:30:09 UTC (rev 200644)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2016-05-10 21:35:32 UTC (rev 200645)
@@ -399,8 +399,6 @@
performConstantHoisting(dfg);
performGlobalCSE(dfg);
performLivenessAnalysis(dfg);
- performIntegerRangeOptimization(dfg);
- performLivenessAnalysis(dfg);
performCFA(dfg);
performConstantFolding(dfg);
performCleanUp(dfg); // Reduce the graph size a lot.
@@ -424,6 +422,16 @@
// Alternatively, we could run loop pre-header creation after SSA conversion - but if we did that
// then we'd need to do some simple SSA fix-up.
performLICM(dfg);
+
+ // FIXME: Currently: IntegerRangeOptimization *must* be run after LICM.
+ //
+ // IntegerRangeOptimization makes changes on nodes based on preceding blocks
+ // and nodes. LICM moves nodes which can invalidates assumptions used
+ // by IntegerRangeOptimization.
+ //
+ // Ideally, the dependencies should be explicit. See https://bugs.webkit.org/show_bug.cgi?id=157534.
+ performLivenessAnalysis(dfg);
+ performIntegerRangeOptimization(dfg);
performCleanUp(dfg);
performIntegerCheckCombining(dfg);
Added: trunk/Source/_javascript_Core/tests/stress/bounds-check-not-eliminated-by-licm.js (0 => 200645)
--- trunk/Source/_javascript_Core/tests/stress/bounds-check-not-eliminated-by-licm.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/bounds-check-not-eliminated-by-licm.js 2016-05-10 21:35:32 UTC (rev 200645)
@@ -0,0 +1,30 @@
+function testInLoopTests(array, index)
+{
+ let arrayLength = array.length;
+ let sum = 0;
+ for (let i = 0; i < 10; ++i) {
+ if (index >= 0 && index < arrayLength) {
+ sum += array[index];
+ }
+ }
+ return sum;
+}
+noInline(testInLoopTests);
+
+
+let testArray = [1, 2, 3];
+
+// Warmup "in-bounds" up to FTL.
+for (let i = 0; i < 1e5; ++i) {
+ if (testInLoopTests(testArray, 1) !== 20)
+ throw "Failed testInLoopTests(testArray, 1)"
+ if (testInLoopTests(testArray, 2) !== 30)
+ throw "Failed testInLoopTests(testArray, 2)"
+}
+
+let largeIntResult = testInLoopTests(testArray, 2147483647);
+if (largeIntResult !== 0)
+ throw "Failed testInLoopTests(testArray, 2147483647)";
+let smallIntResult = testInLoopTests(testArray, -2147483647);
+if (smallIntResult !== 0)
+ throw "Failed testInLoopTests(testArray, -2147483647)";
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
