Bug 164964

Summary: Fix missing exception checks in Interpreter.cpp.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, jfbastien, keith_miller, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 162351    
Attachments:
Description Flags
proposed patch.
none
proposed patch: updated to use return { } where possible.
saam: review+
Re-based patch for landing. none

Mark Lam
Reported 2016-11-18 15:44:27 PST
Patch coming.
Attachments
proposed patch. (11.40 KB, patch)
2016-11-18 15:49 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
proposed patch: updated to use return { } where possible. (11.40 KB, patch)
2016-11-21 11:54 PST, Mark Lam 		saam: review+
DetailsFormatted DiffDiff
Re-based patch for landing. (11.40 KB, patch)
2017-03-15 11:58 PDT, Mark Lam 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2016-11-18 15:49:45 PST
Created attachment 295204 [details] proposed patch.
Mark Lam
Comment 2 2016-11-21 11:54:09 PST
Created attachment 295308 [details] proposed patch: updated to use return { } where possible.
Saam Barati
Comment 3 2016-11-21 15:18:03 PST
Comment on attachment 295308 [details] proposed patch: updated to use return { } where possible. View in context: https://bugs.webkit.org/attachment.cgi?id=295308&action=review > Source/JavaScriptCore/interpreter/Interpreter.cpp:140 > + scope.release(); Why wouldn't this be an ASSERT(!scope.exception())? Seems weird to have it be successful and throw an exception. > Source/JavaScriptCore/interpreter/Interpreter.cpp:146 > + scope.release(); Ditto
Mark Lam
Comment 4 2017-03-15 11:29:25 PDT
Comment on attachment 295308 [details] proposed patch: updated to use return { } where possible. View in context: https://bugs.webkit.org/attachment.cgi?id=295308&action=review >> Source/JavaScriptCore/interpreter/Interpreter.cpp:140 >> + scope.release(); > > Why wouldn't this be an ASSERT(!scope.exception())? > Seems weird to have it be successful and throw an exception. The literal parser allocates objects, and can therefore throw OutOfMemoryErrors, StackOverflowErrors, etc. I'm going to update and land this patch.
Mark Lam
Comment 5 2017-03-15 11:58:25 PDT
Created attachment 304522 [details] Re-based patch for landing.
Radar WebKit Bug Importer
Comment 6 2017-03-15 13:17:18 PDT
<rdar://problem/31071591>
Mark Lam
Comment 7 2017-03-15 13:20:46 PDT
Thanks for the review. Landed in r214005: <http://trac.webkit.org/r214005>.
Note You need to log in before you can comment on or make changes to this bug.