Bug 207423 – Throw OutOfMemory exception instead of crashing if DirectArguments/ScopedArguments can't be created

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Robin Morisset]

rdar://problem/58945874

[Robin Morisset]

Created attachment 390155 [details]
Patch

[Mark Lam]

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()?

[Robin Morisset]

Created attachment 390165 [details]
Patch

Thanks for the review!
I applied all of your suggestions.

[WebKit Commit Bot]

Comment on attachment 390165 [details]
Patch

Clearing flags on attachment: 390165

Committed r256087: <https://trac.webkit.org/changeset/256087>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59283467>