Bug 160154 - various math operations don't properly check for an exception after calling toNumber() on the lhs
Summary: various math operations don't properly check for an exception after calling t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-24 20:39 PDT by Saam Barati
Modified: 2016-08-05 17:48 PDT (History)
12 users (show)

See Also:

Attachments
patch (22.08 KB, patch)
2016-08-05 16:16 PDT, Saam Barati 		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 Saam Barati 2016-07-24 20:39:05 PDT 
...
Comment 1 Saam Barati 2016-08-05 16:16:24 PDT 
Created attachment 285460 [details]
patch
Comment 2 Mark Lam 2016-08-05 16:24:36 PDT 
Comment on attachment 285460 [details]
patch

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

r=me.

> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:441
>      RETURN_WITH_PROFILING(result, {

Is it acceptable to update the ArithProfile here based on a wrong result if right.toNumber() throws?

> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:456
>      RETURN_WITH_PROFILING(result, {

Ditto.
Comment 3 Saam Barati 2016-08-05 16:29:04 PDT 
Comment on attachment 285460 [details]
patch

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

>> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:441
>>      RETURN_WITH_PROFILING(result, {
> 
> Is it acceptable to update the ArithProfile here based on a wrong result if right.toNumber() throws?

It is always OK to update it. However, this code isn't actually doing that.
RETURN_WITH_PROFILING always does CHECK_EXCEPTION before updating the profile.
Comment 4 Saam Barati 2016-08-05 17:27:42 PDT 
Comment on attachment 285460 [details]
patch

Thanks for the review
Comment 5 WebKit Commit Bot 2016-08-05 17:48:13 PDT 
Comment on attachment 285460 [details]
patch

Clearing flags on attachment: 285460

Committed r204206: <http://trac.webkit.org/changeset/204206>
Comment 6 WebKit Commit Bot 2016-08-05 17:48:18 PDT 
All reviewed patches have been landed.  Closing bug.