Bug 207423

Summary: Throw OutOfMemory exception instead of crashing if DirectArguments/ScopedArguments can't be created
Product: WebKit Reporter: Robin Morisset <rmorisset>
Component: JavaScriptCoreAssignee: 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 Flags
Patch
mark.lam: review+, mark.lam: commit-queue-
Patch none

Description Robin Morisset 2020-02-07 17:33:25 PST
rdar://problem/58945874
Comment 1 Robin Morisset 2020-02-07 17:51:51 PST
Created attachment 390155 [details]
Patch
Comment 2 Mark Lam 2020-02-07 20:41:18 PST
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()?
Comment 3 Robin Morisset 2020-02-07 21:25:41 PST
Created attachment 390165 [details]
Patch

Thanks for the review!
I applied all of your suggestions.
Comment 4 WebKit Commit Bot 2020-02-07 22:08:40 PST
Comment on attachment 390165 [details]
Patch

Clearing flags on attachment: 390165

Committed r256087: <https://trac.webkit.org/changeset/256087>
Comment 5 WebKit Commit Bot 2020-02-07 22:08:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2020-02-07 22:09:17 PST
<rdar://problem/59283467>