[webkit-changes] [121629] trunk

Sat, 30 Jun 2012 12:28:56 -0700

Title: [121629] trunk
Revision
121629
Author
[email protected]
Date
2012-06-30 12:28:26 -0700 (Sat, 30 Jun 2012)

Log Message

Webkit crashes in DFG on Google Docs when creating a new document
https://bugs.webkit.org/show_bug.cgi?id=90209

Source/_javascript_Core: 

Reviewed by Gavin Barraclough.
        
Don't attempt to short-circuit Phantom(GetLocal) if the GetLocal is for a
captured variable.

* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::mergeBlocks):

LayoutTests: 

Reviewed by Gavin Barraclough.

* fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local-expected.txt: Added.
* fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.html: Added.
* fast/js/script-tests/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.js: Added.
(baz):
(stuff):
(foo):
(o.g):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (121628 => 121629)


--- trunk/LayoutTests/ChangeLog	2012-06-30 15:55:54 UTC (rev 121628)
+++ trunk/LayoutTests/ChangeLog	2012-06-30 19:28:26 UTC (rev 121629)
@@ -1,3 +1,18 @@
+2012-06-29  Filip Pizlo  <[email protected]>
+
+        Webkit crashes in DFG on Google Docs when creating a new document
+        https://bugs.webkit.org/show_bug.cgi?id=90209
+
+        Reviewed by Gavin Barraclough.
+
+        * fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local-expected.txt: Added.
+        * fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.html: Added.
+        * fast/js/script-tests/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.js: Added.
+        (baz):
+        (stuff):
+        (foo):
+        (o.g):
+
 2012-06-30  Zan Dobersek  <[email protected]>
 
         Unreviewed GTK and EFL gardening, adding image expectation

Added: trunk/LayoutTests/fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local-expected.txt (0 => 121629)


--- trunk/LayoutTests/fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local-expected.txt	2012-06-30 19:28:26 UTC (rev 121629)
@@ -0,0 +1,209 @@
+Tests that attempts by the DFG simplification to short-circuit a Phantom to a GetLocal on a variable that is SetLocal'd in the same block, and where the predecessor block(s) make no mention of that variable, do not result in crashes.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS thingy(o) is 42
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.html (0 => 121629)


--- trunk/LayoutTests/fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.html	2012-06-30 19:28:26 UTC (rev 121629)
@@ -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/fast/js/script-tests/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.js (0 => 121629)


--- trunk/LayoutTests/fast/js/script-tests/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.js	                        (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-cfg-simplify-phantom-get-local-on-same-block-set-local.js	2012-06-30 19:28:26 UTC (rev 121629)
@@ -0,0 +1,40 @@
+description(
+"Tests that attempts by the DFG simplification to short-circuit a Phantom to a GetLocal on a variable that is SetLocal'd in the same block, and where the predecessor block(s) make no mention of that variable, do not result in crashes."
+);
+
+function baz() {
+    // Do something that prevents inlining.
+    return function() { }
+}
+
+function stuff(z) { }
+
+function foo(x, y) {
+    var a = arguments; // Force arguments to be captured, so that x is captured.
+    baz();
+    var z = x;
+    stuff(z); // Force a Flush, and then a Phantom on the GetLocal of x.
+    return 42;
+}
+
+var o = {
+    g: function(x) { }
+};
+
+function thingy(o) {
+    var p = {};
+    var result;
+    // Trick to delay control flow graph simplification until after the flush of x above gets turned into a phantom.
+    if (o.g)
+        p.f = true;
+    if (p.f) {
+        // Basic block that stores to x in foo(), which is a captured variable, with
+        // the predecessor block making no mention of x.
+        result = foo("hello", 2);
+    }
+    return result;
+}
+
+for (var i = 0; i < 200; ++i)
+    shouldBe("thingy(o)", "42");
+

Modified: trunk/Source/_javascript_Core/ChangeLog (121628 => 121629)


--- trunk/Source/_javascript_Core/ChangeLog	2012-06-30 15:55:54 UTC (rev 121628)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-06-30 19:28:26 UTC (rev 121629)
@@ -1,3 +1,16 @@
+2012-06-29  Filip Pizlo  <[email protected]>
+
+        Webkit crashes in DFG on Google Docs when creating a new document
+        https://bugs.webkit.org/show_bug.cgi?id=90209
+
+        Reviewed by Gavin Barraclough.
+        
+        Don't attempt to short-circuit Phantom(GetLocal) if the GetLocal is for a
+        captured variable.
+
+        * dfg/DFGCFGSimplificationPhase.cpp:
+        (JSC::DFG::CFGSimplificationPhase::mergeBlocks):
+
 2012-06-30  Zan Dobersek  <[email protected]>
 
         Unreviewed, rolling out r121605.

Modified: trunk/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp (121628 => 121629)


--- trunk/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp	2012-06-30 15:55:54 UTC (rev 121628)
+++ trunk/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp	2012-06-30 19:28:26 UTC (rev 121629)
@@ -613,7 +613,7 @@
                 
                 ASSERT(node.shouldGenerate());
                 Node& possibleLocalOp = m_graph[node.child1()];
-                if (possibleLocalOp.hasLocal()) {
+                if (possibleLocalOp.hasLocal() && !possibleLocalOp.variableAccessData()->isCaptured()) {
                     NodeIndex setLocalIndex =
                         firstBlock->variablesAtTail.operand(possibleLocalOp.local());
                     Node& setLocal = m_graph[setLocalIndex];

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to