[webkit-reviews] review granted: [Bug 214953] validate untagArrayPtr : [Attachment 424271] 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 214953: validate untagArrayPtr
https://bugs.webkit.org/show_bug.cgi?id=214953

Attachment 424271: patch

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




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

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

r=me

>>> Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h:157
>>> +	     and64(TrustedImmPtr(nonPACBitsMask), target);
>> 
>> Isn't nonPACBitsMask a member of CagedPtr?  Did this actually build?
> 
> It's also defined above in this file:
>     static constexpr uintptr_t nonPACBitsMask = (1ull << (64 -
numberOfPACBits)) - 1;

sorry, I had a bad grep.

>>> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:1074
>>> +#endif
>> 
>> Isn't this also needed for x86_64 because null != neutered?
> 
> Detached array buffer is zero on x86:
> void JSArrayBufferView::detach()
> {
>     auto locker = holdLock(cellLock());
>     RELEASE_ASSERT(hasArrayBuffer());
>     RELEASE_ASSERT(!isShared());
>     m_length = 0;
>     m_vector.clear();
> }

I think it is a bug for x86_64 to use 0 to both mean null and neutered.  But
that's unrelated to this patch.  So, moving on.