Bug 166437 – WebAssembly: Make calling Wasm functions that returns or takes an i64 as a parameter an early exception

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

[Saam Barati]

...

[Radar WebKit Bug Importer]

<rdar://problem/29793949>

[Saam Barati]

Created attachment 297684 [details]
patch

[JF Bastien]

Comment on attachment 297684 [details]
patch

lgtm

[Keith Miller]

Comment on attachment 297684 [details]
patch

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

r=me with some comments. I think there is also an issue if the return value from wasm is an impure nan. It would be great if you could fix that when you land. I think it just needs a purifyNan call.

> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:69
> +                    "WebAssembly function with an int64 argument can't be called from JavaScript");

nit "with an i64 argument"

> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:76
> +                "WebAssembly function that returns int64 can't be called from JavaScript");

nit "returns an i64"

> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:84
>          JSValue arg = exec->uncheckedArgument(argIndex);

This should be exec->argument(argIndex);

[Saam Barati]

Comment on attachment 297684 [details]
patch

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

>> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:69
>> +                    "WebAssembly function with an int64 argument can't be called from JavaScript");
> 
> nit "with an i64 argument"

SGTM

>> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:76
>> +                "WebAssembly function that returns int64 can't be called from JavaScript");
> 
> nit "returns an i64"

SGTM

>> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:84
>>          JSValue arg = exec->uncheckedArgument(argIndex);
> 
> This should be exec->argument(argIndex);

We actually check argumentCount above so this isn't needed.

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/210126