[webkit-reviews] review denied: [Bug 206557] Fix small memory regression caused by r206365 : [Attachment 388370] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 21 16:40:56 PST 2020


Yusuke Suzuki <ysuzuki at apple.com> has denied Justin Michaud
<justin_michaud at apple.com>'s request for review:
Bug 206557: Fix small memory regression caused by r206365
https://bugs.webkit.org/show_bug.cgi?id=206557

Attachment 388370: Patch

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




--- Comment #3 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 388370
  --> https://bugs.webkit.org/attachment.cgi?id=388370
Patch

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

I think this direction is correct. But I found several bugs.

> Source/JavaScriptCore/runtime/StructureRareData.cpp:74
> +    visitor.appendUnbarriered(thisObject->objectToStringValue());

`appendUnbarriered` assumes that whether the cell pointer is valid or nullptr.
Let's do the similar thing done for `m_cachedOwnKeys` below.

> Source/JavaScriptCore/runtime/StructureRareData.cpp:97
> +    if (objectToStringValue() == giveUpOnObjectToStringValueCacheValue())

This never happens since `objectToStringValue()` returns nullptr if the stored
value is `giveUpOnObjectToStringValueCacheValue`.
Let's directly read m_objectToStringValue here, and check it carefully.

> Source/JavaScriptCore/runtime/StructureRareData.cpp:159
> +    if (objectToStringValue() != giveUpOnObjectToStringValueCacheValue())

Ditto.


More information about the webkit-reviews mailing list