Changeset 39660 in webkit

Timestamp:

Jan 6, 2009 12:33:54 PM (15 years ago)

Author:

oliver@apple.com

Message:

<​https://bugs.webkit.org/show_bug.cgi?id=23085> [jsfunfuzz] Over released ScopeChainNode
<rdar://problem/6474110>

Reviewed by Cameron Zwarich

So this delightful bug was caused by our unwind code using a ScopeChain to perform
the unwind. The ScopeChain would ref the initial top of the scope chain, then deref
the resultant top of scope chain, which is incorrect.

This patch removes the dependency on ScopeChain for the unwind, and i've filed
<​https://bugs.webkit.org/show_bug.cgi?id=23144> to look into the unintuitive
ScopeChain behaviour.

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/interpreter/Interpreter.cpp (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/exception-try-finally-scope-error-expected.txt (modified) (1 diff)
• LayoutTests/fast/js/resources/exception-try-finally-scope-error.js (modified) (2 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2009-01-06  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        <https://bugs.webkit.org/show_bug.cgi?id=23085> [jsfunfuzz] Over released ScopeChainNode
+        <rdar://problem/6474110>
+
+        So this delightful bug was caused by our unwind code using a ScopeChain to perform
+        the unwind.  The ScopeChain would ref the initial top of the scope chain, then deref
+        the resultant top of scope chain, which is incorrect.
+
+        This patch removes the dependency on ScopeChain for the unwind, and i've filed
+        <https://bugs.webkit.org/show_bug.cgi?id=23144> to look into the unintuitive
+        ScopeChain behaviour.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::throwException):
+
 2009-01-06  Adam Roben  <aroben@apple.com>
 

trunk/JavaScriptCore/interpreter/Interpreter.cpp

@@ -851 +851 @@
     // Now unwind the scope chain within the exception handler's call frame.
 
-    ScopeChain sc(callFrame->scopeChain());
+    ScopeChainNode* scopeChain = callFrame->scopeChain();
+    ScopeChain sc(scopeChain);
     int scopeDelta = depth(codeBlock, sc) - handler->scopeDepth;
     ASSERT(scopeDelta >= 0);
     while (scopeDelta--)
-        sc.pop();
-    callFrame->setScopeChain(sc.node());
+        scopeChain = scopeChain->pop();
+    callFrame->setScopeChain(scopeChain);
 
     return handler;

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-01-06  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        <https://bugs.webkit.org/show_bug.cgi?id=23085> [jsfunfuzz] Over released ScopeChainNode
+        <rdar://problem/6474110>
+
+        Add test for over releasing the scopechain.
+
+        * fast/js/exception-try-finally-scope-error-expected.txt:
+        * fast/js/resources/exception-try-finally-scope-error.js:
+
 2008-01-05  Dean Jackson  <dino@apple.com>
 

trunk/LayoutTests/fast/js/exception-try-finally-scope-error-expected.txt

@@ -1 @@
-This test makes sure stack unwinding works correctly when confronted with a 0-depth scope chain without an activation
+This test makes sure stack unwinding works correctly in combination with dynamically added scopes
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

trunk/LayoutTests/fast/js/resources/exception-try-finally-scope-error.js

@@ -1 @@
-description('This test makes sure stack unwinding works correctly when confronted with a 0-depth scope chain without an activation');
+description('This test makes sure stack unwinding works correctly in combination with dynamically added scopes');
+
+function gc()
+{
+    if (this.GCController)
+        GCController.collect();
+    else
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+}
+
 var result;
 function runTest() {
@@ -8 +18 @@
 runTest();
 
+try{
+(function() {
+    try {
+        throw "";
+    } catch(y) {
+        throw (function(){});
+    } finally {
+    }
+})()
+}catch(r){
+}
+
+// Just clobber any temporaries
+a=({});
+a*=a*a*a;
+
+gc();
+
 var successfullyParsed = true;