Bug 196089 - JSC::createError should clear exception thrown by errorDescriptionForValue
Summary: JSC::createError should clear exception thrown by errorDescriptionForValue
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tadeu Zagallo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-21 10:32 PDT by Tadeu Zagallo
Modified: 2019-03-28 11:22 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2019-03-21 10:39 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.87 MB, application/zip)
2019-03-21 12:48 PDT, EWS Watchlist
no flags Details
Patch for landing (1.70 KB, patch)
2019-03-21 13:47 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Patch for landing (1.84 KB, patch)
2019-03-21 14:07 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Patch for landing (1.84 KB, patch)
2019-03-21 14:37 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tadeu Zagallo 2019-03-21 10:32:57 PDT
...
Comment 1 Tadeu Zagallo 2019-03-21 10:39:23 PDT
Created attachment 365566 [details]
Patch
Comment 2 Mark Lam 2019-03-21 12:06:00 PDT
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.
Comment 3 EWS Watchlist 2019-03-21 12:48:34 PDT
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
Comment 4 EWS Watchlist 2019-03-21 12:48:36 PDT
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
Comment 5 Tadeu Zagallo 2019-03-21 13:47:51 PDT
Created attachment 365608 [details]
Patch for landing
Comment 6 Mark Lam 2019-03-21 13:50:36 PDT
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.
Comment 7 Tadeu Zagallo 2019-03-21 14:07:03 PDT
Created attachment 365615 [details]
Patch for landing
Comment 8 Tadeu Zagallo 2019-03-21 14:37:45 PDT
Created attachment 365619 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2019-03-21 15:05:02 PDT
Comment on attachment 365619 [details]
Patch for landing

Clearing flags on attachment: 365619

Committed r243335: <https://trac.webkit.org/changeset/243335>
Comment 10 WebKit Commit Bot 2019-03-21 15:05:04 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2019-03-21 15:21:01 PDT
<rdar://problem/49126673>