Bug 164972 – Fix exception scope verification failures in ArrayConstructor.cpp and ArrayPrototype.cpp

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

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 295228 [details]
proposed patch.

[Mark Lam]

Created attachment 295230 [details]
proposed patch.

[Mark Lam]

Comment on attachment 295230 [details]
proposed patch.

New patch with more fixes and returning { } coming soon.

[Mark Lam]

Created attachment 295311 [details]
proposed patch.

Still running tests but preliminary results look good.  Let's get some EWS testing while we wait.

[Mark Lam]

Comment on attachment 295311 [details]
proposed patch.

I think this is ready for a review.

[Mark Lam]

Comment on attachment 295311 [details]
proposed patch.

It is invalid to replace returning encodedJSValue() with returning { }.  On 32-bit builds, the former is non-zero, while the latter is 0.  Will fix this patch.

[Mark Lam]

Created attachment 295419 [details]
proposed patch.

Let's try this on the EWS first.

[Mark Lam]

Comment on attachment 295419 [details]
proposed patch.

Tests paas.  Ready for a review.

[Geoffrey Garen]

Comment on attachment 295419 [details]
proposed patch.

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

r=me

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1253
> +    ASSERT(!scope.exception() || !isValid);

I think you want to assert equality here. Otherwise, you don't check for scope.exception() && isValid.

[Mark Lam]

(In reply to comment #9)
> Comment on attachment 295419 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295419&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1253
> > +    ASSERT(!scope.exception() || !isValid);
> 
> I think you want to assert equality here. Otherwise, you don't check for
> scope.exception() && isValid.

This assertion needs to be as is because it is possible for speciesWatchpointsValid(0 to return false (i.e. isValid is false) without throwing an exception.  The reverse is not true i.e. if an exception was thrown, then isValid must be false.

I confirmed this by running the test JSTests/stress/array-concat-on-frozen-object.js.

[Mark Lam]

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