Bug 23085 – [jsfunfuzz] Over released ScopeChainNode

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Oliver Hunt]

(function(){
    try{
        with({})
            throw this(function(){})
    } catch(x) {
    }
})()
gc()

[Oliver Hunt]

erk, it turns out this test case doesn't reproduce with all of jsfunfuzz included before it, however i've found another reduction like thing that produces the same crash

[Oliver Hunt]

Okay, here we go:
function tryRunning(f){
    try{
        f()
    }catch(r){
    }
}
function tryItOut(){
    function f() {
        try {
            throw "";
        } catch(y) {
            this(function(){})
        } finally {
        }
    };
    v = tryRunning(f)
}
tryItOut();
gc();

[Gavin Barraclough]

function f() {
    try {
        throw "";
    } catch(y) {
        this(function(){})
    } finally {
    }
};
try {
    f()
} catch(r) {
}
gc();

[Gavin Barraclough]

^^ release builds only :-(

[Gavin Barraclough]

try{
(function() {
    try {
        throw "";
    } catch(y) {
        throw (function(){});
    } finally {
    }
})()
}catch(r){
}
(function(){})()
gc();

[Oliver Hunt]

The problem is that a scope node is being deleted prematurely, i believe the scope node being removed in the most recent reduction is the activation for the first function.

I honestly can't work out how/why the ref counting scheme we use for scopechainnodes works, but i blame it for the badness.

[Cameron Zwarich (cpst)]

I'll take a look at this. This one hurts.

[Oliver Hunt]

<rdar://problem/6474110>

[Oliver Hunt]

Okay, so the issue is that the finally block is derefing the activation incorrectly 

[Oliver Hunt]

Scope chain unwinding creates a ScopeChain to wrap the ScopeChainNode, when the ScopeChain is destroyed it does a full deref of the top node, but it has not necessarily ref'd that node.

Basically this is the path to badness
  * Scope chain is represented as {scope object, ref count} -> next scopechainnode  
  * ScopeChainNode* scopeChain = {someScope, 1}->{activation, 1}->not relevant
  * ScopeChain sc(scopeChain) => {someScope, 2}->{activation, 1}->not relevant
  * sc.pop() => {activation, 1}->not relevant
  * sc.~ScopeChain => {activation, 0}->not relevant

So we end up leaking the top of stack, and over releasing whateer is the ToS at the end
   

[Oliver Hunt]

Ah, whooops, not actually correct, the problem is that ScopeChain refs() the origin ToS,  and then derefs() the final ToS.  but pop() and deref() have different behaviour -- deref() on a node that was not explicitly ref'd is basically wrong

[Oliver Hunt]

Created attachment 26465 [details]
Remove use of ScopeChain for the scope chain unwinding

Fixeration

[Cameron Zwarich (cpst)]

Comment on attachment 26465 [details]
Remove use of ScopeChain for the scope chain unwinding

r=me

[Oliver Hunt]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/interpreter/Interpreter.cpp
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/js/exception-try-finally-scope-error-expected.txt
	M	LayoutTests/fast/js/resources/exception-try-finally-scope-error.js
Committed r39660