Summary: | Throw OutOfMemory exception instead of crashing if DirectArguments/ScopedArguments can't be created | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robin Morisset <rmorisset> | ||||||
Component: | JavaScriptCore | Assignee: | Robin Morisset <rmorisset> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Robin Morisset
2020-02-07 17:33:25 PST
Created attachment 390155 [details]
Patch
Comment on attachment 390155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390155&action=review r=me > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1107 > + LLINT_CHECK_EXCEPTION(); This is redundant because there's a LLINT_CHECK_EXCEPTION() immediately following this if statement. Please remove. > Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:984 > + CHECK_EXCEPTION(); I think you should put this after the if statement to match LLINT_SLOW_PATH_DECL(slow_path_del_by_val). Looks like currently, we're missing an exception check after the call to deleteProperty(). > Source/JavaScriptCore/runtime/DirectArguments.cpp:125 > + if (!backingStore) { UNLIKELY()? > Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:175 > + scope.release(); Hmmm, use RELEASE_AND_RETURN instead of return in both cases below instead? Not a big difference right now, but I think that is less error prone if any of the cases below expand to have more logic that might throw. > Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:285 > + if (!backingStore) { UNLIKELY()? Created attachment 390165 [details]
Patch
Thanks for the review!
I applied all of your suggestions.
Comment on attachment 390165 [details] Patch Clearing flags on attachment: 390165 Committed r256087: <https://trac.webkit.org/changeset/256087> All reviewed patches have been landed. Closing bug. |