[webkit-reviews] review granted: [Bug 175037] Baseline JIT should do caging : [Attachment 317486] more

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 175037: Baseline JIT should do caging
https://bugs.webkit.org/show_bug.cgi?id=175037

Attachment 317486: more

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




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

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

r=me with suggestions.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:1322
> +	   if (!Gigacage::shouldBeEnabled())
> +	       return;
> +	   
> +#if GIGACAGE_ENABLED
> +	   andPtr(TrustedImmPtr(static_cast<size_t>(GIGACAGE_MASK)), storage);
> +	   addPtr(TrustedImmPtr(Gigacage::basePtr(kind)), storage);
> +#endif

You can move the #if GIGACAGE_ENABLED before the "if
(!Gigacage::shouldBeEnabled())" check.	Need to add UNUSED_PARAM for kind and
storage in the #else case though.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:1339
> +	   if (!Gigacage::shouldBeEnabled())
> +	       return;
> +	   
> +	   if (kind != Gigacage::Primitive ||
Gigacage::isDisablingPrimitiveGigacageDisabled())
> +	       return cage(kind, storage);
> +	   
> +	   loadPtr(Gigacage::basePtr(kind), scratch);
> +	   Jump done = branchTestPtr(Zero, scratch);
> +#if GIGACAGE_ENABLED
> +	   andPtr(TrustedImmPtr(static_cast<size_t>(GIGACAGE_MASK)), storage);
> +	   addPtr(scratch, storage);
> +#endif
> +	   done.link(this);

You can move the #if GIGACAGE_ENABLED before the "if
(!Gigacage::shouldBeEnabled())" check.	Need to add UNUSED_PARAM for kind,
storage, and scratch in the #else case though.