| Summary: | Fix missing exception checks in Interpreter.cpp. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||
| Component: | JavaScriptCore | Assignee: | 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
Mark Lam
2016-11-18 15:44:27 PST
Created attachment 295204 [details]
proposed patch.
Created attachment 295308 [details]
proposed patch: updated to use return { } where possible.
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 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. Created attachment 304522 [details]
Re-based patch for landing.
Thanks for the review. Landed in r214005: <http://trac.webkit.org/r214005>. |