Bug 162584 – Add some needed CatchScopes in code that should not throw.

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

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 289893 [details]
proposed patch.

[Keith Miller]

Comment on attachment 289893 [details]
proposed patch.

r=me.

[Mark Lam]

Thanks for the review.  I fixed a typo in the ChangeLog before landing.

Landed in r206405: <http://trac.webkit.org/r206405>.

[Geoffrey Garen]

Comment on attachment 289893 [details]
proposed patch.

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

> Source/JavaScriptCore/jsc.cpp:2549
> +    RETURN_IF_EXCEPTION(scope, 3);

How can GlobalObject::create throw?

[Ryan Haddad]

This change appears to have caused two LayoutTests to crash (EWS seems to have caught this, but the patch was landed before results could be verified).

plugins/npruntime/object-from-destroyed-plugin.html
plugins/npruntime/object-from-destroyed-plugin-in-subframe.html

https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r206406%20(9738)/results.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 162588

[Mark Lam]

Comment on attachment 289893 [details]
proposed patch.

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

>> Source/JavaScriptCore/jsc.cpp:2549
>> +    RETURN_IF_EXCEPTION(scope, 3);
> 
> How can GlobalObject::create throw?

I forgot the details of what motivated this change in the first place.  I'll leave it out for now until the motivation arises again when I turn on exception check verification.

> Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:246
> +    VM& vm = exec->vm();
> +    auto scope = DECLARE_CATCH_SCOPE(vm);
> +
>      String errorMessage = makeString(errorDescriptionForValue(exec, value)->value(exec), ' ', message);
> +    if (UNLIKELY(scope.exception())) {
> +        scope.clearException();
> +        errorMessage = message;
> +    }

This change appears to need additional support from exception checks that are missing.  I will leave it out for now until I have the missing exception checks in place.

[Mark Lam]

Created attachment 289989 [details]
Patch for re-landing.

[Mark Lam]

Tests are happy.  Revised patch landed in r206459: <http://trac.webkit.org/r206459>.