[webkit-reviews] review granted: [Bug 84648] Failure to allocate ArrayStorage in emit_op_new_array leads to poisonous JSArray : [Attachment 138605] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 84648: Failure to allocate ArrayStorage in emit_op_new_array leads to
poisonous JSArray
https://bugs.webkit.org/show_bug.cgi?id=84648

Attachment 138605: Patch
https://bugs.webkit.org/attachment.cgi?id=138605&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=138605&action=review


Boy, those perf numbers are whisper-thin. Still, the fix is good, so let's take
it.

> Source/JavaScriptCore/jit/JITInlineMethods.h:490
> +    // Allocate the backing store for the array. We allocate the storage
first
> +    // because it could fail and we have to take the slow path, which would
leave 
> +    // behind a zombie JSArray with inconsistent state, potentially causing
a GC crash.

Hard to understand a comment about a potentially bad alternate version of the
code.

I would just say, "We allocate the backing store first to ensure that garbage
collection doesn't happen during JSArray initialization".