Bug 119900 – Exception in global setter doesn't unwind correctly

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

[Oliver Hunt]

this.__defineSetter__("d", function h(){ throw ""});
function g() {
    d = 0;
}
for (;;) try { g() } catch(e){ }

Crashes on an assertion due to us not catching the exception properly.  some print debugging shows that we continue executing after d=0

[Radar WebKit Bug Importer]

<rdar://problem/14758580>

[Yi Shen]

Created attachment 209242 [details]
proposal patch

[Oliver Hunt]

Comment on attachment 209242 [details]
proposal patch

Can you check the dfg path as well?  otherwise this looks good

[Geoffrey Garen]

Thanks for the patch.

I think you missed slow_path_put_to_scope in LLIntSlowPaths.cpp (the LLInt version of the same bug). Can you add that to your patch and your test case?

[Yi Shen]

The slow_path_put_to_scope calls the LLINT_END before exiting, which invokes llint_throw_from_slow_path_trampoline to handle the exception if the vm.exception is not null. So, no fix needed here.
(In reply to comment #4)
> Thanks for the patch.
> 
> I think you missed slow_path_put_to_scope in LLIntSlowPaths.cpp (the LLInt version of the same bug). Can you add that to your patch and your test case?

[Yi Shen]

Sure, I will check the dfg path.
(In reply to comment #3)
> (From update of attachment 209242 [details])
> Can you check the dfg path as well?  otherwise this looks good

[Geoffrey Garen]

> The slow_path_put_to_scope calls the LLINT_END before exiting, which invokes llint_throw_from_slow_path_trampoline to handle the exception if the vm.exception is not null. So, no fix needed here.

Nice!

[Yi Shen]

It seems dfg path already providers exception handler by calling JITCompiler::compileExceptionHandlers() when generating dfg jit code. After applied my patch, I ran your test function g() in a loop for 100,000 times and saw dfg path (debugged in xcode) works fine without any assertion failure.
(In reply to comment #3)
> (From update of attachment 209242 [details])
> Can you check the dfg path as well?  otherwise this looks good

[Geoffrey Garen]

Comment on attachment 209242 [details]
proposal patch

r=me

Thanks!

[Yi Shen]

Thanks for review :)
(In reply to comment #9)
> (From update of attachment 209242 [details])
> r=me
> 
> Thanks!

[WebKit Commit Bot]

Comment on attachment 209242 [details]
proposal patch

Clearing flags on attachment: 209242

Committed r154429: <http://trac.webkit.org/changeset/154429>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.