Bug 179202 – Only cage double butterfly accesses

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

[Saam Barati]

...

[Saam Barati]

Created attachment 326240 [details]
patch

[Mark Lam]

Comment on attachment 326240 [details]
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.

[Saam Barati]

Created attachment 326298 [details]
patch for landing

Thanks for the review. I've removed the scratch register and I've made Butterfly:createOrGrowPropertyStorage cage when oldButterfly is non-null.

[WebKit Commit Bot]

Comment on attachment 326298 [details]
patch for landing

Clearing flags on attachment: 326298

Committed r224564: <https://trac.webkit.org/changeset/224564>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Suzuki]

Oh, this improves ARES-6 by 4.4% at AWFY.

[Radar WebKit Bug Importer]

<rdar://problem/35567338>