Bug 166437 - WebAssembly: Make calling Wasm functions that returns or takes an i64 as a parameter an early exception
Summary: WebAssembly: Make calling Wasm functions that returns or takes an i64 as a pa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-12-22 14:51 PST by Saam Barati
Modified: 2016-12-22 17:37 PST (History)
12 users (show)

See Also:

Attachments
patch (9.08 KB, patch)
2016-12-22 15:38 PST, Saam Barati 		keith_miller: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-12-22 14:51:20 PST 
...
Comment 1 Radar WebKit Bug Importer 2016-12-22 15:23:48 PST 
<rdar://problem/29793949>
Comment 2 Saam Barati 2016-12-22 15:38:09 PST 
Created attachment 297684 [details]
patch
Comment 3 JF Bastien 2016-12-22 15:49:20 PST 
Comment on attachment 297684 [details]
patch

lgtm
Comment 4 Keith Miller 2016-12-22 17:06:37 PST 
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);
Comment 5 Saam Barati 2016-12-22 17:14:42 PST 
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.
Comment 6 Saam Barati 2016-12-22 17:37:14 PST 
landed in:
https://trac.webkit.org/changeset/210126