Bug 168687 - Add missing exception checks detected by running marathon.js.
Summary: Add missing exception checks detected by running marathon.js.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks: 168580
  Show dependency tree / graph
 
Reported: 2017-02-21 15:59 PST by Mark Lam
Modified: 2017-02-21 17:03 PST (History)
7 users (show)

See Also:

Attachments
proposed patch. (3.09 KB, patch)
2017-02-21 16:08 PST, Mark Lam 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2017-02-21 15:59:27 PST 
When running the marathon.js test from https://bugs.webkit.org/show_bug.cgi?id=168580, we get some crashes due to missing exception checks.  This patch adds those missing exception checks.
Comment 1 Mark Lam 2017-02-21 16:08:11 PST 
Created attachment 302328 [details]
proposed patch.

Let's try this on the EWS.
Comment 2 Mark Lam 2017-02-21 16:18:12 PST 
Comment on attachment 302328 [details]
proposed patch.

It builds.  So, this is ready for a review.
Comment 3 Saam Barati 2017-02-21 16:20:09 PST 
Comment on attachment 302328 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/JSCJSValueInlines.h:652
>      return primitive.toString(exec)->toIdentifier(exec);

what about this?
Comment 4 Mark Lam 2017-02-21 16:23:30 PST 
Comment on attachment 302328 [details]
proposed patch.

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

>> Source/JavaScriptCore/runtime/JSCJSValueInlines.h:652
>>      return primitive.toString(exec)->toIdentifier(exec);
> 
> what about this?

I think this is fine.  primitive.toString() will return an empty string here on exception thrown.  Hence, it is safe to call toIdentifier() with it.
Comment 5 Mark Lam 2017-02-21 16:24:09 PST 
Comment on attachment 302328 [details]
proposed patch.

Thanks for the review.  Committing the patch now.
Comment 6 WebKit Commit Bot 2017-02-21 17:03:02 PST 
Comment on attachment 302328 [details]
proposed patch.

Clearing flags on attachment: 302328

Committed r212779: <http://trac.webkit.org/changeset/212779>
Comment 7 WebKit Commit Bot 2017-02-21 17:03:06 PST 
All reviewed patches have been landed.  Closing bug.