Bug 165022 – Fix exception scope verification failures in runtime/JSGenericTypedArrayView* files.

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

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 295324 [details]
proposed patch.

[Saam Barati]

Comment on attachment 295324 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:470
> +        scope.release();

Why not put the scope.release() before the switch and also keep the old return statement?

[Mark Lam]

Comment on attachment 295324 [details]
proposed patch.

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

>> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:470
>> +        scope.release();
> 
> Why not put the scope.release() before the switch and also keep the old return statement?

Because we want the scope.release() near the return statement.  Having it before the switch increases the opportunity for new code to be inadvertantly added between the scope.release() and the switch.

[Mark Lam]

(In reply to comment #3)
> Comment on attachment 295324 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295324&action=review
> 
> >> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:470
> >> +        scope.release();
> > 
> > Why not put the scope.release() before the switch and also keep the old return statement?
> 
> Because we want the scope.release() near the return statement.  Having it
> before the switch increases the opportunity for new code to be inadvertantly
> added between the scope.release() and the switch.

Saam pointed out to me offline that if we have 2 ThrowScopes between the scope.release() and the return, the 2nd ThrowScope will catch the unhandled exception from the first.  So, my reasoning above does not apply.

Anyway, I'll leave it as is for this patch because it does make it easier to see the return that corresponds to the scope.release().

[Mark Lam]

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