JavaScriptCore:

2008-05-29  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Oliver.

        - fixed <rdar://problem/5972943> REGRESSION(r33979): Flash clips do not play on cnn.com

        Finally blocks could clobber registers that had to remain live
        until they returned. This patch takes a conservative approach and
        makes sure that finally blocks do not reuse any registers that
        were previously allocated for the function. In the future this
        could probably be tightened up to be less profligate with the
        register allocation.

        * VM/CodeGenerator.cpp:
        (KJS::CodeGenerator::highestUsedRegister):
        * VM/CodeGenerator.h:
        * kjs/nodes.cpp:
        (KJS::TryNode::emitCode):

LayoutTests:

2008-05-29  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Oliver. Test by Geoff Garen.

        - fixed <rdar://problem/5972943> REGRESSION(r33979): Flash clips do not play on cnn.com

        * fast/js/finally-codegen-failure-expected.txt: Added.
        * fast/js/finally-codegen-failure.html: Added.

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 0abb845f6a896dbb18b9dba111c8fe9f8ec23c1e..95e222dad07fd2f0a466d69f9ffe411c03505fa4 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,22 @@
+2008-05-29  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Oliver.
+
+        - fixed <rdar://problem/5972943> REGRESSION(r33979): Flash clips do not play on cnn.com
+        
+        Finally blocks could clobber registers that had to remain live
+        until they returned. This patch takes a conservative approach and
+        makes sure that finally blocks do not reuse any registers that
+        were previously allocated for the function. In the future this
+        could probably be tightened up to be less profligate with the
+        register allocation.
+        
+        * VM/CodeGenerator.cpp:
+        (KJS::CodeGenerator::highestUsedRegister):
+        * VM/CodeGenerator.h:
+        * kjs/nodes.cpp:
+        (KJS::TryNode::emitCode):
+
 2008-05-29  Steve Falkenburg  <sfalken@apple.com>
 
         Build fix.

diff --git a/JavaScriptCore/VM/CodeGenerator.cpp b/JavaScriptCore/VM/CodeGenerator.cpp
index 258e3bcb1c2a8386c1fc7622847e02874acb97f6..d28d5a604f8d6859b5275d5938312b5a7f4f15d1 100644 (file)
--- a/JavaScriptCore/VM/CodeGenerator.cpp
+++ b/JavaScriptCore/VM/CodeGenerator.cpp
@@ -363,6 +363,14 @@ RegisterID* CodeGenerator::newTemporary()
     return &m_temporaries.last();
 }
 
+
+RegisterID* CodeGenerator::highestUsedRegister()
+{
+    while (m_temporaries.size() < static_cast<unsigned>(m_codeBlock->numTemporaries))
+        m_temporaries.append(m_temporaries.size());
+    return &m_temporaries.last();
+}
+
 PassRefPtr<LabelID> CodeGenerator::newLabel()
 {
     // Reclaim free label IDs.

diff --git a/JavaScriptCore/VM/CodeGenerator.h b/JavaScriptCore/VM/CodeGenerator.h
index 3fcad7ba34f411e7c87d74becc6b8d8a6ba038d8..87d558a0f335e683c553005b8ef587dea0bb0c57 100644 (file)
--- a/JavaScriptCore/VM/CodeGenerator.h
+++ b/JavaScriptCore/VM/CodeGenerator.h
@@ -116,6 +116,8 @@ namespace KJS {
         // the next instruction may overwrite it.
         RegisterID* newTemporary();
 
+        RegisterID* highestUsedRegister();
+
         // The same as newTemporary(), but this function returns "suggestion" if
         // "suggestion" is a temporary. This function is helpful in situations
         // where you've put "suggestion" in a RefPtr, but you'd like to allow

diff --git a/JavaScriptCore/kjs/nodes.cpp b/JavaScriptCore/kjs/nodes.cpp
index 6d7b305a793e965cf16192eedc3f216a2ca1406a..3c0b24d5539e8725933bd69bb2149614a62654c2 100644 (file)
--- a/JavaScriptCore/kjs/nodes.cpp
+++ b/JavaScriptCore/kjs/nodes.cpp
@@ -5745,6 +5745,11 @@ RegisterID* TryNode::emitCode(CodeGenerator& generator, RegisterID* dst)
 
     if (m_finallyBlock) {
         generator.popFinallyContext();
+        // there may be important registers live at the time we jump
+        // to a finally block (such as for a return or throw) so we
+        // ref the highest register ever used as a conservative
+        // approach to not clobbering anything important
+        RefPtr<RegisterID> highestUsedRegister = generator.highestUsedRegister();
         RefPtr<LabelID> finallyEndLabel = generator.newLabel();
         generator.emitJumpSubroutine(finallyReturnAddr.get(), finallyStart.get());
         generator.emitJump(finallyEndLabel.get());

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index f3d89c54e5bfbaaeb567ea6ff9779622a78e9a9e..663f74ab1a7b063c20b7af2b08c507a898415d13 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2008-05-29  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Oliver. Test by Geoff Garen.
+        
+        - fixed <rdar://problem/5972943> REGRESSION(r33979): Flash clips do not play on cnn.com
+
+        * fast/js/finally-codegen-failure-expected.txt: Added.
+        * fast/js/finally-codegen-failure.html: Added.
+
 2008-05-28  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Eric.

diff --git a/LayoutTests/fast/js/finally-codegen-failure-expected.txt b/LayoutTests/fast/js/finally-codegen-failure-expected.txt
new file mode 100644 (file)
index 0000000..d6ceb93
--- /dev/null
+++ b/LayoutTests/fast/js/finally-codegen-failure-expected.txt
@@ -0,0 +1,4 @@
+This page tests a codegen bug with finally blocks. If the test passes, you'll see a PASS message below.
+
+PASS
+

diff --git a/LayoutTests/fast/js/finally-codegen-failure.html b/LayoutTests/fast/js/finally-codegen-failure.html
new file mode 100644 (file)
index 0000000..51c8a71
--- /dev/null
+++ b/LayoutTests/fast/js/finally-codegen-failure.html
@@ -0,0 +1,31 @@
+<p>This page tests a codegen bug with finally blocks. If the test passes, you'll
+see a PASS message below.</p>
+
+<pre id="console"></pre>
+
+<script>
+function log(s)
+{
+    document.getElementById("console").appendChild(document.createTextNode(s + "\n"));
+}
+
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+a = {
+    f: function() { return "PASS"; }
+};
+
+a.f.toString = function() { return "FAIL"; };
+
+function f() {
+    try {
+        a.f();
+        a.f();
+        return a.f();
+    } finally {
+        a.f();
+    }
+}
+log(f())
+</script>