[webkit-reviews] review granted: [Bug 205259] ASSERTION FAILED: length <= maximumLength in js-fixed-array-out-of-memory.js : [Attachment 385824] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 205259: ASSERTION FAILED: length <= maximumLength in
js-fixed-array-out-of-memory.js
https://bugs.webkit.org/show_bug.cgi?id=205259

Attachment 385824: Patch

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




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

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

r=me

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7967
>	   m_jit.mutatorFence(vm());

Is this mutatorFence executed on any path?  It looks to me like this is dead
code.  do we need it?

> Source/JavaScriptCore/runtime/JSImmutableButterfly.h:55
> +	   // Because of the above maximumLength requirement, overflowing never
happens.

I suggest rephrasing this comment slightly as /overflowing never
happens/allocationSize can never overflow/.  In the context of this patch, this
is obvious.  But reading back later without this patch in mind, it may not be
clear what the "overflow" is referring to.  Rephrasing it thus should make that
clearer.