MakeRope fixup shouldn't lead to an Identity without kids

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

Source/JavaScriptCore:

Reviewed by Mark Hahnenberg.

Make the empty string pruning part of fixupMakeRope() stop if it's on the last child.

Make Node::convertToIdentity release-assert that it has exactly one kid.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupMakeRope):
* dfg/DFGNode.h:
(JSC::DFG::Node::convertToIdentity):

LayoutTests:

Reviewed by Mark Hahnenberg.

* fast/js/dfg-make-rope-all-empty-strings.html: Added.
* fast/js/dfg-make-rope-all-empty-strings-expected.txt: Added.
* fast/js/jsc-test-list:
* fast/js/script-tests/dfg-make-rope-all-empty-strings.js: Added.
(foo):
(bar):

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index af44d5f..6184c01 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2013-07-16  Filip Pizlo  <fpizlo@apple.com>
+
+        MakeRope fixup shouldn't lead to an Identity without kids
+        https://bugs.webkit.org/show_bug.cgi?id=118745
+
+        Reviewed by Mark Hahnenberg.
+
+        * fast/js/dfg-make-rope-all-empty-strings.html: Added.
+        * fast/js/dfg-make-rope-all-empty-strings-expected.txt: Added.
+        * fast/js/jsc-test-list:
+        * fast/js/script-tests/dfg-make-rope-all-empty-strings.js: Added.
+        (foo):
+        (bar):
+
 2013-07-16  Roger Fong  <roger_fong@apple.com>
 
         Unreviewed rebaseline test added in r152612 for Apple Windows port.

diff --git a/LayoutTests/fast/js/dfg-make-rope-all-empty-strings-expected.txt b/LayoutTests/fast/js/dfg-make-rope-all-empty-strings-expected.txt
new file mode 100644 (file)
index 0000000..f5c429f
--- /dev/null
+++ b/LayoutTests/fast/js/dfg-make-rope-all-empty-strings-expected.txt
@@ -0,0 +1,109 @@
+Check that the DFG can handle MakeRope or ValueAdd on all empty strings.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS bar() is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

diff --git a/LayoutTests/fast/js/dfg-make-rope-all-empty-strings.html b/LayoutTests/fast/js/dfg-make-rope-all-empty-strings.html
new file mode 100644 (file)
index 0000000..602f4cb
--- /dev/null
+++ b/LayoutTests/fast/js/dfg-make-rope-all-empty-strings.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/dfg-make-rope-all-empty-strings.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>

diff --git a/LayoutTests/fast/js/jsc-test-list b/LayoutTests/fast/js/jsc-test-list
index a687348..2911149 100644 (file)
--- a/LayoutTests/fast/js/jsc-test-list
+++ b/LayoutTests/fast/js/jsc-test-list
@@ -181,6 +181,7 @@ fast/js/dfg-intrinsic-osr-exit
 fast/js/dfg-intrinsic-side-effect-assignment-osr-exit
 fast/js/dfg-intrinsic-unused-this
 fast/js/dfg-intrinsic-unused-this-method-check
+fast/js/dfg-make-rope-all-empty-strings
 fast/js/dfg-max-backwards-propagation
 fast/js/dfg-min-backwards-propagation
 fast/js/dfg-min-max

diff --git a/LayoutTests/fast/js/script-tests/dfg-make-rope-all-empty-strings.js b/LayoutTests/fast/js/script-tests/dfg-make-rope-all-empty-strings.js
new file mode 100644 (file)
index 0000000..3fee744
--- /dev/null
+++ b/LayoutTests/fast/js/script-tests/dfg-make-rope-all-empty-strings.js
@@ -0,0 +1,15 @@
+description(
+"Check that the DFG can handle MakeRope or ValueAdd on all empty strings."
+);
+
+function foo(a, b) {
+    return a + b;
+}
+
+function bar() {
+    return foo("", "");
+}
+
+for (var i = 0; i < 100; ++i)
+    shouldBe("bar()", "\"\"");
+

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index e5c2fd3..84dcde2 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2013-07-16  Filip Pizlo  <fpizlo@apple.com>
+
+        MakeRope fixup shouldn't lead to an Identity without kids
+        https://bugs.webkit.org/show_bug.cgi?id=118745
+
+        Reviewed by Mark Hahnenberg.
+        
+        Make the empty string pruning part of fixupMakeRope() stop if it's on the last child.
+        
+        Make Node::convertToIdentity release-assert that it has exactly one kid.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupMakeRope):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::convertToIdentity):
+
 2013-07-16  Mark Hahnenberg  <mhahnenberg@apple.com>
 
         Remove reference to JSValueStructSupport.h from JSExport.h

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index c9a58d6..d5f6922 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -960,6 +960,11 @@ private:
             JSString* string = jsCast<JSString*>(m_graph.valueOfJSConstant(edge.node()).asCell());
             if (string->length())
                 continue;
+            
+            // Don't allow the MakeRope to have zero children.
+            if (!i && !node->child2())
+                break;
+            
             node->children.removeEdge(i--);
         }
         

diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
index 2bc1ffd..fdfa294 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGNode.h
+++ b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -221,6 +221,8 @@ struct Node {
 
     void convertToIdentity()
     {
+        RELEASE_ASSERT(child1());
+        RELEASE_ASSERT(!child2());
         setOpAndDefaultNonExitFlags(Identity);
     }