Bug 164969 – Fix exception scope verification failures in LLIntSlowPaths.cpp

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

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 295223 [details]
proposed patch.

[Geoffrey Garen]

Comment on attachment 295223 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=295223&action=review

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1452
> +    throwScope.release();
>      return setUpCall(execCallee, pc, kind, calleeAsValue);

I think the throwScope.release() function should assert that there is no pending exception. Otherwise, this release point is an opportunity to make an error when calling into setUpCall.

[Mark Lam]

(In reply to comment #2)
> Comment on attachment 295223 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295223&action=review
> 
> > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1452
> > +    throwScope.release();
> >      return setUpCall(execCallee, pc, kind, calleeAsValue);
> 
> I think the throwScope.release() function should assert that there is no
> pending exception. Otherwise, this release point is an opportunity to make
> an error when calling into setUpCall.

I agree. but I'll do this in a separate patch because I think this will result in some failures due to some old scope.release() being added this way (for various reasons):

    result = callSomethingThatThrows();
    scope.release();
    return result;
 
These need to be fixed like so:

    scope.release();
    return callSomethingThatThrows();

I'll do that fix up along with adding the assertion to scope.release() in one patch.

[Mark Lam]

(In reply to comment #3)
> I'll do that fix up along with adding the assertion to scope.release() in
> one patch.

Bug for making scope.release() assert no pending exceptions is https://bugs.webkit.org/show_bug.cgi?id=165105.

[Mark Lam]

Thanks for the review.  Landed in r209007: <http://trac.webkit.org/r209007>.