[webkit-reviews] review granted: [Bug 179202] Only cage double butterfly accesses : [Attachment 326240] patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 179202: Only cage double butterfly accesses
https://bugs.webkit.org/show_bug.cgi?id=179202

Attachment 326240: patch

https://bugs.webkit.org/attachment.cgi?id=326240&action=review




--- Comment #2 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 326240
  --> https://bugs.webkit.org/attachment.cgi?id=326240
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326240&action=review

r=me with issues addressed.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1238
>  macro storePropertyAtVariableOffset(propertyOffsetAsInt, objectAndStorage,
value, scratch)

It doesn't hurt to leave it but scratch is now unused.

> Source/JavaScriptCore/runtime/JSObject.cpp:3250
> +    return Butterfly::createOrGrowPropertyStorage(butterfly(), vm, this,
structure(vm), oldSize, newSize);

Why is this case not caged?  I see that
Butterfly::createOrGrowPropertyStorage() does mem copying.  Either
Butterfly::createOrGrowPropertyStorage() should always cage the butterfly if
not null, or it we should do a null check + caging here.