Bug 160154

Summary: various math operations don't properly check for an exception after calling toNumber() on the lhs
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, sukolsak, ticaiolima, ysuzuki
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch none

Saam Barati
Reported 2016-07-24 20:39:05 PDT
...
Attachments
patch (22.08 KB, patch)
2016-08-05 16:16 PDT, Saam Barati
no flags
Saam Barati
Comment 1 2016-08-05 16:16:24 PDT
Mark Lam
Comment 2 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.
Saam Barati
Comment 3 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.
Saam Barati
Comment 4 2016-08-05 17:27:42 PDT
Comment on attachment 285460 [details] patch Thanks for the review
WebKit Commit Bot
Comment 5 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>
WebKit Commit Bot
Comment 6 2016-08-05 17:48:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.