JavaScriptCore:
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2007 20:50:28 +0000 (20:50 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2007 20:50:28 +0000 (20:50 +0000)
        Reviewed by Maciej Stachowiak.

        Fixed http://bugs.webkit.org/show_bug.cgi?id=15785
        REGRESSION(r27344): Crash on load at finance.yahoo.com

        Reverted a small portion of my last check-in. (The speedup and the List
        removal are still there, though.)

        ActivationImp needs to hold a pointer to its function, and mark that
        pointer (rather than accessing its function through its ExecState, and
        counting on the active scope to mark its function) because a closure
        can cause an ActivationImp to outlive its ExecState along with any
        active scope.

        * kjs/ExecState.cpp:
        (KJS::ExecState::ExecState):
        * kjs/function.cpp:
        (KJS::FunctionImp::~FunctionImp):
        (KJS::ActivationImp::ActivationImp):
        * kjs/function.h:
        (KJS::ActivationImp::ActivationImpPrivate::ActivationImpPrivate):

        Also made HashTable a little more crash-happy in debug builds, so
        problems like this will show up earlier:

        * wtf/HashTable.h:
        (WTF::HashTable::~HashTable):

LayoutTests:

        Reviewed by Maciej Stachowiak.

        Layout test for http://bugs.webkit.org/show_bug.cgi?id=15785
        REGRESSION(r27344): Crash on load at finance.yahoo.com

        * fast/js/activation-object-function-lifetime-expected.txt: Added.
        * fast/js/activation-object-function-lifetime.html: Added.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/function.cpp
JavaScriptCore/kjs/function.h
JavaScriptCore/wtf/HashTable.h
LayoutTests/ChangeLog
LayoutTests/fast/js/activation-object-function-lifetime-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/activation-object-function-lifetime.html [new file with mode: 0644]

index 52e8dc1037477014b765ee8587a6a1fbd650d829..19cd52ad016520af137a0cb37f6c55bfa199f782 100644 (file)
@@ -1,3 +1,33 @@
+2007-11-01  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Fixed http://bugs.webkit.org/show_bug.cgi?id=15785
+        REGRESSION(r27344): Crash on load at finance.yahoo.com
+        
+        Reverted a small portion of my last check-in. (The speedup and the List 
+        removal are still there, though.)
+        
+        ActivationImp needs to hold a pointer to its function, and mark that 
+        pointer (rather than accessing its function through its ExecState, and 
+        counting on the active scope to mark its function) because a closure 
+        can cause an ActivationImp to outlive its ExecState along with any 
+        active scope.
+
+        * kjs/ExecState.cpp:
+        (KJS::ExecState::ExecState):
+        * kjs/function.cpp:
+        (KJS::FunctionImp::~FunctionImp):
+        (KJS::ActivationImp::ActivationImp):
+        * kjs/function.h:
+        (KJS::ActivationImp::ActivationImpPrivate::ActivationImpPrivate):
+
+        Also made HashTable a little more crash-happy in debug builds, so 
+        problems like this will show up earlier:
+        
+        * wtf/HashTable.h:
+        (WTF::HashTable::~HashTable):
+
 2007-11-01  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Adam Roben.
index b35f63300d5e8c0175351991dad2b14e414e592c..86de1aedb0879e96221f120cc95ef1c5ec5f900c 100644 (file)
@@ -484,6 +484,10 @@ void ActivationImp::mark()
         if (!value->marked())
             value->mark();
     }
+    
+    ASSERT(d->function);
+    if (!d->function->marked())
+        d->function->mark();
 
     if (d->argumentsObject && !d->argumentsObject->marked())
         d->argumentsObject->mark();
@@ -491,7 +495,10 @@ void ActivationImp::mark()
 
 void ActivationImp::createArgumentsObject(ExecState* exec)
 {
-    d->argumentsObject = new Arguments(exec, d->exec->function(), *d->exec->arguments(), this);
+    // Since "arguments" is only accessible while a function is being called,
+    // we can retrieve our argument list from the ExecState for our function 
+    // call instead of storing the list ourselves.
+    d->argumentsObject = new Arguments(exec, d->function, *d->exec->arguments(), this);
 }
 
 // ------------------------------ GlobalFunc -----------------------------------
index 8284f3a751d667128697eb3285f44f71efdf67bb..f26ac1f084d9036adcf59c4ed81e0de8c5548b08 100644 (file)
@@ -142,12 +142,14 @@ namespace KJS {
     struct ActivationImpPrivate {
         ActivationImpPrivate(ExecState* e)
             : exec(e)
+            , function(e->function())
             , argumentsObject(0)
         {
         }
         
         LocalStorage localStorage;
         ExecState* exec;
+        FunctionImp* function;
         Arguments* argumentsObject;
     };
 
index ebc41c4d3e7c4d6a875fb084b5f3a2e0dad526dc..a64fed441504a599abc2a7177f2b7f495b3ce26b 100644 (file)
@@ -34,8 +34,10 @@ namespace WTF {
 
 #ifdef NDEBUG
 #define CHECK_HASHTABLE_ITERATORS 0
+#define CHECK_HASHTABLE_USE_AFTER_DESTRUCTION 0
 #else
 #define CHECK_HASHTABLE_ITERATORS 1
+#define CHECK_HASHTABLE_USE_AFTER_DESTRUCTION 1
 #endif
 
 #if DUMP_HASHTABLE_STATS
@@ -279,7 +281,14 @@ namespace WTF {
         typedef IdentityHashTranslator<Key, Value, HashFunctions> IdentityTranslatorType;
 
         HashTable();
-        ~HashTable() { invalidateIterators(); deallocateTable(m_table, m_tableSize); }
+        ~HashTable() 
+        {
+            invalidateIterators(); 
+            deallocateTable(m_table, m_tableSize); 
+#if CHECK_HASHTABLE_USE_AFTER_DESTRUCTION
+            m_table = (ValueType*)(uintptr_t)0xbbadbeef;
+#endif
+        }
 
         HashTable(const HashTable&);
         void swap(HashTable&);
index 30dac9d0a73a7b0ab03896929300ba9a02a37419..63b8fec1ad46cb3a8e639125f8ed65eb564a1e9e 100644 (file)
@@ -1,3 +1,13 @@
+2007-11-01  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+        
+        Layout test for http://bugs.webkit.org/show_bug.cgi?id=15785
+        REGRESSION(r27344): Crash on load at finance.yahoo.com
+
+        * fast/js/activation-object-function-lifetime-expected.txt: Added.
+        * fast/js/activation-object-function-lifetime.html: Added.
+
 2007-11-01  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Dave Hyatt.
diff --git a/LayoutTests/fast/js/activation-object-function-lifetime-expected.txt b/LayoutTests/fast/js/activation-object-function-lifetime-expected.txt
new file mode 100644 (file)
index 0000000..2dea1da
--- /dev/null
@@ -0,0 +1,7 @@
+This page tests for regressions against http://bugs.webkit.org/show_bug.cgi?id=15785 REGRESSION(r27344): Crash on load at finance.yahoo.com
+
+If the test passes, you'll see a PASS message below.
+
+PASS: You didn't crash.
+
+
diff --git a/LayoutTests/fast/js/activation-object-function-lifetime.html b/LayoutTests/fast/js/activation-object-function-lifetime.html
new file mode 100644 (file)
index 0000000..9e118cc
--- /dev/null
@@ -0,0 +1,32 @@
+<p>This page tests for regressions against http://bugs.webkit.org/show_bug.cgi?id=15785
+REGRESSION(r27344): Crash on load at finance.yahoo.com</p>
+<p>If the test passes, you'll see a PASS message below.</p>
+<pre id="console"></pre>
+
+<script>
+if (this.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function log(s)
+{
+    document.getElementById("console").appendChild(document.createTextNode(s + "\n"));
+}
+
+// Make a closure with a function that takes the slow path to variable lookup.
+var f = eval("(function (x) { return function() { return x; } })()");
+
+// Force GC.
+if (this.GCController)
+    GCController.collect();
+else {
+    for (var i = 0; i < 10000; ++i) {
+        ({ });
+    }
+}
+
+// Call the function.
+f();
+
+log("PASS: You didn't crash.\n");
+
+</script>