- Revision
- 179457
- Author
- msab...@apple.com
- Date
- 2015-01-31 19:58:39 -0800 (Sat, 31 Jan 2015)
Log Message
Crash (DFG assertion) beneath AbstractInterpreter::verifyEdge() @ http://experilous.com/1/planet-generator/2014-09-28/version-1
https://bugs.webkit.org/show_bug.cgi?id=141111
Reviewed by Filip Pizlo.
Source/_javascript_Core:
In LowerDFGToLLVM::compileNode(), if we determine while compiling a node that we would have
exited, we don't need to process the OSR availability or abstract interpreter.
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::safelyInvalidateAfterTermination): Broke this out a a separate
method since we need to call it at the top and near the bottom of compileNode().
(JSC::FTL::LowerDFGToLLVM::compileNode):
LayoutTests:
New tests.
* js/regress-141111-expected.txt: Added.
* js/regress-141111.html: Added.
* js/script-tests/regress-141111.js: Added.
(MyObject):
(foo):
(.result):
(bar):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (179456 => 179457)
--- trunk/LayoutTests/ChangeLog 2015-02-01 03:11:14 UTC (rev 179456)
+++ trunk/LayoutTests/ChangeLog 2015-02-01 03:58:39 UTC (rev 179457)
@@ -1,3 +1,20 @@
+2015-01-31 Michael Saboff <msab...@apple.com>
+
+ Crash (DFG assertion) beneath AbstractInterpreter::verifyEdge() @ http://experilous.com/1/planet-generator/2014-09-28/version-1
+ https://bugs.webkit.org/show_bug.cgi?id=141111
+
+ Reviewed by Filip Pizlo.
+
+ New tests.
+
+ * js/regress-141111-expected.txt: Added.
+ * js/regress-141111.html: Added.
+ * js/script-tests/regress-141111.js: Added.
+ (MyObject):
+ (foo):
+ (.result):
+ (bar):
+
2015-01-31 Antti Koivisto <an...@apple.com>
Enable WebKit disk cache on OS X
Added: trunk/LayoutTests/js/regress-141111-expected.txt (0 => 179457)
--- trunk/LayoutTests/js/regress-141111-expected.txt (rev 0)
+++ trunk/LayoutTests/js/regress-141111-expected.txt 2015-02-01 03:58:39 UTC (rev 179457)
@@ -0,0 +1,9 @@
+Regression test for https://webkit.org/b/141111. This test should run without crashing.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/regress-141111.html (0 => 179457)
--- trunk/LayoutTests/js/regress-141111.html (rev 0)
+++ trunk/LayoutTests/js/regress-141111.html 2015-02-01 03:58:39 UTC (rev 179457)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/js/script-tests/regress-141111.js (0 => 179457)
--- trunk/LayoutTests/js/script-tests/regress-141111.js (rev 0)
+++ trunk/LayoutTests/js/script-tests/regress-141111.js 2015-02-01 03:58:39 UTC (rev 179457)
@@ -0,0 +1,57 @@
+description(
+"Regression test for https://webkit.org/b/141111. This test should run without crashing."
+);
+
+function MyObject(v) {
+ this.v = v;
+}
+
+function foo(o, a, b, c) {
+ // Don't do anything real but have some control flow. This causes the PutLocals for a,
+ // b, and c to survive into SSA form. But we don't have any effects, so sinking will be
+ // successful.
+ if (o.v)
+ return o;
+ else
+ return z;
+}
+
+function bar(o, y) {
+ var a = y;
+ var b = y + 1;
+ var c = y + 2;
+ var d = y + 3;
+ var e = y + 4;
+ var f = y + 5;
+ var g = y + 6;
+ var h = y + 7;
+ var i = y + 8;
+ var j = y + 9;
+ var k = y + 10;
+ var result = function(p, q) {
+ var x = new MyObject(a + b + c + d + e + f + g + h + i + j + k);
+ if (q) {
+ // Make it appear that it's possible to clobber those closure variables, so that we
+ // load from them again down below.
+ a = b = c = d = e = f = g = h = i = j = k = 42;
+ }
+ if (p)
+ x = foo(o, 1, 2, 3)
+ else
+ x = five;
+ return x.v + a + b + c + d + e + f + g + h + i + j + k;
+ };
+ noInline(result);
+ return result;
+}
+
+var o = new MyObject(42);
+var z = new MyObject(0);
+var five = new MyObject(5);
+
+for (var i = 0; i < 100000; ++i) {
+ var result = bar(o, i)(true, false);
+ if (result != 42 + 11 * i + 55)
+ throw "Error: bad result: " + result;
+}
+
Modified: trunk/Source/_javascript_Core/ChangeLog (179456 => 179457)
--- trunk/Source/_javascript_Core/ChangeLog 2015-02-01 03:11:14 UTC (rev 179456)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-02-01 03:58:39 UTC (rev 179457)
@@ -1,3 +1,18 @@
+2015-01-31 Michael Saboff <msab...@apple.com>
+
+ Crash (DFG assertion) beneath AbstractInterpreter::verifyEdge() @ http://experilous.com/1/planet-generator/2014-09-28/version-1
+ https://bugs.webkit.org/show_bug.cgi?id=141111
+
+ Reviewed by Filip Pizlo.
+
+ In LowerDFGToLLVM::compileNode(), if we determine while compiling a node that we would have
+ exited, we don't need to process the OSR availability or abstract interpreter.
+
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::safelyInvalidateAfterTermination): Broke this out a a separate
+ method since we need to call it at the top and near the bottom of compileNode().
+ (JSC::FTL::LowerDFGToLLVM::compileNode):
+
2015-01-31 Sam Weinig <s...@webkit.org>
Remove even more Mountain Lion support
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (179456 => 179457)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-02-01 03:11:14 UTC (rev 179456)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-02-01 03:58:39 UTC (rev 179457)
@@ -306,30 +306,34 @@
break;
}
}
-
+
+ void safelyInvalidateAfterTermination()
+ {
+ if (verboseCompilationEnabled())
+ dataLog("Bailing.\n");
+ crash(m_highBlock->index, m_node->index());
+
+ // Invalidate dominated blocks. Under normal circumstances we would expect
+ // them to be invalidated already. But you can have the CFA become more
+ // precise over time because the structures of objects change on the main
+ // thread. Failing to do this would result in weird crashes due to a value
+ // being used but not defined. Race conditions FTW!
+ for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
+ BasicBlock* target = m_graph.block(blockIndex);
+ if (!target)
+ continue;
+ if (m_graph.m_dominators.dominates(m_highBlock, target)) {
+ if (verboseCompilationEnabled())
+ dataLog("Block ", *target, " will bail also.\n");
+ target->cfaHasVisited = false;
+ }
+ }
+ }
+
bool compileNode(unsigned nodeIndex)
{
if (!m_state.isValid()) {
- if (verboseCompilationEnabled())
- dataLog("Bailing.\n");
- crash(m_highBlock->index, m_node->index());
-
- // Invalidate dominated blocks. Under normal circumstances we would expect
- // them to be invalidated already. But you can have the CFA become more
- // precise over time because the structures of objects change on the main
- // thread. Failing to do this would result in weird crashes due to a value
- // being used but not defined. Race conditions FTW!
- for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
- BasicBlock* target = m_graph.block(blockIndex);
- if (!target)
- continue;
- if (m_graph.m_dominators.dominates(m_highBlock, target)) {
- if (verboseCompilationEnabled())
- dataLog("Block ", *target, " will bail also.\n");
- target->cfaHasVisited = false;
- }
- }
-
+ safelyInvalidateAfterTermination();
return false;
}
@@ -749,7 +753,12 @@
DFG_CRASH(m_graph, m_node, "Unrecognized node in FTL backend");
break;
}
-
+
+ if (!m_state.isValid()) {
+ safelyInvalidateAfterTermination();
+ return false;
+ }
+
m_availabilityCalculator.executeNode(m_node);
m_interpreter.executeEffects(nodeIndex);
