[webkit-reviews] review granted: [Bug 212825] Make FAST_JIT_PERMISSIONS check in LinkBuffer::copyCompactAndLinkCode a runtime check : [Attachment 401220] Patch for Landing

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has granted	review:
Bug 212825: Make FAST_JIT_PERMISSIONS check in
LinkBuffer::copyCompactAndLinkCode a runtime check
https://bugs.webkit.org/show_bug.cgi?id=212825

Attachment 401220: Patch for Landing

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




--- Comment #12 from Michael Saboff <msaboff at apple.com> ---
Created attachment 401220

  --> https://bugs.webkit.org/attachment.cgi?id=401220&action=review

Patch for Landing

> > Source/JavaScriptCore/assembler/LinkBuffer.cpp:137
> > +ALWAYS_INLINE bool useFastJITPermissions()
> > +{
> > +#if ENABLE(FAST_JIT_PERMISSIONS) && !ENABLE(SEPARATED_WX_HEAP)
> > +	 return true;
> > +#elif ENABLE(FAST_JIT_PERMISSIONS)
> > +	 return g_jscConfig.useFastPermisionsJITCopy;
> > +#else
> > +	 return false;
> > +#endif
> > +}
> 
> If you moved this to ExecutableAllocator.h, you could use
> useFastJITPermissions() to slightly simplify performJITMemcpy as well
> (remove one #if/endif ENABLE(SEPARATED_WX_HEAP)).

Done.

> > Source/JavaScriptCore/assembler/LinkBuffer.cpp:250
> > +#if ENABLE(VERIFY_JIT_HASH)
> 
> can we assert that useFastJITPermissions() is true
> 
> alternatively, just return true from that function #if
> ENABLE(VERIFY_JIT_HASH)

Although these sections of code were part of the same "#if CPU(ARM64E) &&
ENABLE(FAST_JIT_PERMISSIONS)" sections, this is actually independent of
FAST_JIT_PERMISSIONS.  You'll see elsewhere (AssemblerBuffer.h ~183) that the
assembly hash code is simply #if CPU(ARM64E).  I have adjusted the setting of
ENABLE_VERIFY_JIT_HASH accordingly.