Bug 196089 – JSC::createError should clear exception thrown by errorDescriptionForValue

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Tadeu Zagallo]

...

[Tadeu Zagallo]

Created attachment 365566 [details]
Patch

[Mark Lam]

Comment on attachment 365566 [details]
Patch

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

r=me with fix.

> Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:279
>      String valueDescription = errorDescriptionForValue(exec, value);
> -    if (!valueDescription)
> +    if (!valueDescription) {
> +        scope.clearException();

I think it's a hack that we return an OOME when we fail to create the requested Error object.  I suspect that this is a semantic error because:

    var error = new RangeError();
    error.toString()l // expects "RangeError", but may get "Out of memory".

Anyway, that was a pre-existing thing and can be fixed later.  For this patch, you also need to placate the exception validator before the call to tryMakeString() below.  The way to do this is to add the following before the "if (!valueDescription)" above:
    ASSERT(scope.exception() || valueDescription);

The assertion tickles scope.exception(), thereby telling the validator that we did something with the exception.  This placates the validator.  Note: the assertion assumes that we will never return a null string unless there's no exception.  If that's not the case, just do a real exception check instead.

[EWS Watchlist]

Comment on attachment 365566 [details]
Patch

Attachment 365566 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11602569

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html

[EWS Watchlist]

Created attachment 365596 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Tadeu Zagallo]

Created attachment 365608 [details]
Patch for landing

[Mark Lam]

Comment on attachment 365608 [details]
Patch for landing

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

> Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:278
> +    ASSERT(scope.exception() || valueDescription);

You also need to clear the exception because you’re in a CatchScope here.

[Tadeu Zagallo]

Created attachment 365615 [details]
Patch for landing

[Tadeu Zagallo]

Created attachment 365619 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 365619 [details]
Patch for landing

Clearing flags on attachment: 365619

Committed r243335: <https://trac.webkit.org/changeset/243335>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49126673>