Bug 184163 – Use MacroAssemblerCodePtr in Wasm code for code pointers instead of void*.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

With the use of MacroAssemblerCodePtr, we now get poisoning for Wasm code pointers.

Also rename some structs, methods, and variable names to be more accurate.  Previously, there is some confusion between a code pointer and the address of a code pointer (sometimes referred to in the code as a "LoadLocation").  We will now name the LoadLocation variables appropriately to distinguish them from code pointers.

[Radar WebKit Bug Importer]

<rdar://problem/39020397>

[Radar WebKit Bug Importer]

<rdar://problem/39020437>

[Mark Lam]

Created attachment 336820 [details]
proposed patch.

[Mark Lam]

<rdar://problem/39020397>

[JF Bastien]

Comment on attachment 336820 [details]
proposed patch.

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

This is really great, cleans up the code a bunch :)

r=me with one comment addressed. r- for now because I think it's wrong without the fix.

> Source/JavaScriptCore/wasm/WasmBinding.cpp:79
> +    jit.xorPtr(JIT::TrustedImmPtr(g_JITCodePoison), scratch);

if (Options::usePoisoning())

[Mark Lam]

Comment on attachment 336820 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/wasm/WasmBinding.cpp:79
>> +    jit.xorPtr(JIT::TrustedImmPtr(g_JITCodePoison), scratch);
> 
> if (Options::usePoisoning())

Technically, this is not wrong.  If !Options::usePoisoning(), the poison value is guaranteed to be 0.  Hence, this xorPtr has no effect.  However, it is less efficient.  I will fix it.  Thanks for the catch.

[JF Bastien]

Comment on attachment 336820 [details]
proposed patch.

(In reply to Mark Lam from comment #6)
> Comment on attachment 336820 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=336820&action=review
> 
> Thanks for the review.
> 
> >> Source/JavaScriptCore/wasm/WasmBinding.cpp:79
> >> +    jit.xorPtr(JIT::TrustedImmPtr(g_JITCodePoison), scratch);
> > 
> > if (Options::usePoisoning())
> 
> Technically, this is not wrong.  If !Options::usePoisoning(), the poison
> value is guaranteed to be 0.  Hence, this xorPtr has no effect.  However, it
> is less efficient.  I will fix it.  Thanks for the catch.

Right, r+ then, with change.

[Mark Lam]

Landed in r230096: <http://trac.webkit.org/r230096>.