Bug 199783 – [JSC] Improve wasm wpt test results by fixing miscellaneous issues

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Yusuke Suzuki]

...

[Yusuke Suzuki]

Created attachment 374083 [details]
Current Score

[Yusuke Suzuki]

Created attachment 374084 [details]
Patch

[EWS Watchlist]

Attachment 374084 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:342:  Extra space before [.  [whitespace/brackets] [5]
Total errors found: 1 in 163 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Comment on attachment 374084 [details]
Patch

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

Add several notes.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/constructor/instantiate-bad-imports.any-expected.txt:3
> +Harness Error (FAIL), message = TypeError: undefined is not an object (evaluating 'WebAssembly.Global.prototype')

WebAssembly.Global is not implemented.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/instance/constructor.any-expected.txt:11
> +FAIL exports WebAssembly.Module doesn't parse at byte 87: 3th Export isn't immutable, named 'global' (evaluating 'new WebAssembly.Module(buffer)')

We should fix it in a separate patch.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/instance/constructor.any.worker-expected.txt:11
> +FAIL exports WebAssembly.Module doesn't parse at byte 87: 3th Export isn't immutable, named 'global' (evaluating 'new WebAssembly.Module(buffer)')

We should fix it in a separate patch.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/interface.any-expected.txt:79
> +FAIL WebAssembly.Module.exports assert_equals: expected (undefined) undefined but got (function) function "function exports() {
> +    [native code]
> +}"
> +PASS WebAssembly.Module.exports: name 
> +PASS WebAssembly.Module.exports: length 
> +FAIL WebAssembly.Module.imports assert_equals: expected (undefined) undefined but got (function) function "function imports() {
> +    [native code]
> +}"
> +PASS WebAssembly.Module.imports: name 
> +PASS WebAssembly.Module.imports: length 
> +FAIL WebAssembly.Module.customSections assert_equals: expected (undefined) undefined but got (function) function "function customSections() {
> +    [native code]
> +}"
> +PASS WebAssembly.Module.customSections: name 
> +PASS WebAssembly.Module.customSections: length 
> +PASS WebAssembly.Instance.exports 
> +PASS WebAssembly.Instance.exports: getter 
> +PASS WebAssembly.Instance.exports: setter 
> +FAIL WebAssembly.Memory.grow assert_equals: expected (undefined) undefined but got (function) function "function grow() {
> +    [native code]
> +}"
> +PASS WebAssembly.Memory.grow: name 
> +PASS WebAssembly.Memory.grow: length 
> +PASS WebAssembly.Memory.buffer 
> +PASS WebAssembly.Memory.buffer: getter 
> +PASS WebAssembly.Memory.buffer: setter 
> +FAIL WebAssembly.Table.grow assert_equals: expected (undefined) undefined but got (function) function "function grow() {
> +    [native code]
> +}"
> +PASS WebAssembly.Table.grow: name 
> +PASS WebAssembly.Table.grow: length 
> +FAIL WebAssembly.Table.get assert_equals: expected (undefined) undefined but got (function) function "function get() {
> +    [native code]
> +}"
> +PASS WebAssembly.Table.get: name 
> +PASS WebAssembly.Table.get: length 
> +FAIL WebAssembly.Table.set assert_equals: expected (undefined) undefined but got (function) function "function set() {
> +    [native code]
> +}"

The imported wpt revision is old, and this is bug in the test side. In the upstream, it is fixed.
When upgrading our wpt, we can make them PASS.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/interface.any.worker-expected.txt:80
> +FAIL WebAssembly.Module.exports assert_equals: expected (undefined) undefined but got (function) function "function exports() {
> +    [native code]
> +}"
> +PASS WebAssembly.Module.exports: name 
> +PASS WebAssembly.Module.exports: length 
> +FAIL WebAssembly.Module.imports assert_equals: expected (undefined) undefined but got (function) function "function imports() {
> +    [native code]
> +}"
> +PASS WebAssembly.Module.imports: name 
> +PASS WebAssembly.Module.imports: length 
> +FAIL WebAssembly.Module.customSections assert_equals: expected (undefined) undefined but got (function) function "function customSections() {
> +    [native code]
> +}"
> +PASS WebAssembly.Module.customSections: name 
> +PASS WebAssembly.Module.customSections: length 
> +PASS WebAssembly.Instance.exports 
> +PASS WebAssembly.Instance.exports: getter 
> +PASS WebAssembly.Instance.exports: setter 
> +FAIL WebAssembly.Memory.grow assert_equals: expected (undefined) undefined but got (function) function "function grow() {
> +    [native code]
> +}"
> +PASS WebAssembly.Memory.grow: name 
> +PASS WebAssembly.Memory.grow: length 
> +PASS WebAssembly.Memory.buffer 
> +PASS WebAssembly.Memory.buffer: getter 
> +PASS WebAssembly.Memory.buffer: setter 
> +FAIL WebAssembly.Table.grow assert_equals: expected (undefined) undefined but got (function) function "function grow() {
> +    [native code]
> +}"
> +PASS WebAssembly.Table.grow: name 
> +PASS WebAssembly.Table.grow: length 
> +FAIL WebAssembly.Table.get assert_equals: expected (undefined) undefined but got (function) function "function get() {
> +    [native code]
> +}"
> +PASS WebAssembly.Table.get: name 
> +PASS WebAssembly.Table.get: length 
> +FAIL WebAssembly.Table.set assert_equals: expected (undefined) undefined but got (function) function "function set() {
> +    [native code]
> +}"

WPT implementation is broken, and it is fixed in the upstream.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/module/customSections.any-expected.txt:9
> +FAIL Custom sections with U+FFFD assert_equals: expected 1 but got 0

We should fix it in a separate patch.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/module/customSections.any.worker-expected.txt:9
> +FAIL Custom sections with U+FFFD assert_equals: expected 1 but got 0

We should fix it in a separate patch.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/module/exports.any-expected.txt:7
> +FAIL exports WebAssembly.Module doesn't parse at byte 87: 3th Export isn't immutable, named 'global' (evaluating 'new WebAssembly.Module(buffer)')

We should fix it in a separate patch.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/module/exports.any.worker-expected.txt:7
> +FAIL exports WebAssembly.Module doesn't parse at byte 87: 3th Export isn't immutable, named 'global' (evaluating 'new WebAssembly.Module(buffer)')

We should fix it in the separate patch.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/table/get-set.any-expected.txt:11
> +FAIL Basic assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Growing assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting out-of-bounds assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting non-function assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting non-wasm function assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting non-wasm arrow function assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")

WPT implementation is broken, and it is fixed in the upstream.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/table/get-set.any.worker-expected.txt:11
> +FAIL Basic assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Growing assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting out-of-bounds assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting non-function assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting non-wasm function assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Setting non-wasm arrow function assert_throws: undefined: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")

WPT implementation is broken, and it is fixed in the upstream.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/table/grow.any-expected.txt:6
> +FAIL Basic assert_throws: before: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Reached maximum assert_throws: before: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Exceeded maximum assert_throws: before: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")

WPT implementation is broken, and it is fixed in the upstream.

> LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/table/grow.any.worker-expected.txt:6
> +FAIL Basic assert_throws: before: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Reached maximum assert_throws: before: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")
> +FAIL Exceeded maximum assert_throws: before: table.get(-1) function "() => table.get(-1)" threw object "TypeError: Expect an integer argument in the range: [0, 2^32 - 1]" ("TypeError") expected object "RangeError" ("RangeError")

WPT implementation is broken, and it is fixed in the upstream.

[EWS Watchlist]

Comment on attachment 374084 [details]
Patch

Attachment 374084 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12737001

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 374085 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.5

[EWS Watchlist]

Comment on attachment 374084 [details]
Patch

Attachment 374084 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/12736996

New failing tests:
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-eager-jettison
wasm.yaml/wasm/spec-tests/jsapi.js.default-wasm
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-air
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-tls-context
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-collect-continuously
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-call-ic
apiTests

[Yusuke Suzuki]

Created attachment 374105 [details]
Patch

[EWS Watchlist]

Attachment 374105 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:342:  Extra space before [.  [whitespace/brackets] [5]
Total errors found: 1 in 167 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Robin Morisset]

Comment on attachment 374105 [details]
Patch

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

This looks reasonable to me overall, but I don't feel knowledgeable enough about this code to give it the r=me.

> Source/JavaScriptCore/ChangeLog:12
> +        2. Fix various attributes. It does not match to the usual JSC builtin's convension. But this change

convention => convention

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:205
> +

This space is not needed.

> Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:115
> +    if (Options::useWebAssemblyStreamingApi()) {

I have not seen this in the Changelog, it seems like the kind of thing that could be nice to highlight.

[Yusuke Suzuki]

Comment on attachment 374105 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/ChangeLog:12
>> +        2. Fix various attributes. It does not match to the usual JSC builtin's convension. But this change
> 
> convention => convention

Thanks, fixed.

>> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:205
>> +
> 
> This space is not needed.

Fixed.

>> Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:115
>> +    if (Options::useWebAssemblyStreamingApi()) {
> 
> I have not seen this in the Changelog, it seems like the kind of thing that could be nice to highlight.

This is (1)'s change: removing WebAssemblyPrototype and merging WebAssemblyPrototype into JSWebAssembly :)

[Mark Lam]

Comment on attachment 374105 [details]
Patch

LGTM

[Yusuke Suzuki]

Committed r247440: <https://trac.webkit.org/changeset/247440>

[Radar WebKit Bug Importer]

<rdar://problem/53116419>

[Michael Catanzaro]

../../Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp: In member function ‘void JSC::JSWebAssembly::finishCreation(JSC::VM&, JSC::JSGlobalObject*)’:
../../Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:116:69: error: ‘webAssemblyCompileStreamingCodeGenerator’ was not declared in this scope
         JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION("compileStreaming", webAssemblyCompileStreamingCodeGenerator, static_cast<unsigned>(PropertyAttribute::DontEnum));
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/runtime/JSObject.h:1636:58: note: in definition of macro ‘JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION’
         vm, globalObject, makeIdentifier(vm, (jsName)), (generatorName)(vm), (attributes))
                                                          ^~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:116:69: note: suggested alternative: ‘webAssemblyCompileStreamingInternal’
         JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION("compileStreaming", webAssemblyCompileStreamingCodeGenerator, static_cast<unsigned>(PropertyAttribute::DontEnum));
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/runtime/JSObject.h:1636:58: note: in definition of macro ‘JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION’
         vm, globalObject, makeIdentifier(vm, (jsName)), (generatorName)(vm), (attributes))
                                                          ^~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:117:73: error: ‘webAssemblyInstantiateStreamingCodeGenerator’ was not declared in this scope
         JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION("instantiateStreaming", webAssemblyInstantiateStreamingCodeGenerator, static_cast<unsigned>(PropertyAttribute::DontEnum));
                                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/runtime/JSObject.h:1636:58: note: in definition of macro ‘JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION’
         vm, globalObject, makeIdentifier(vm, (jsName)), (generatorName)(vm), (attributes))
                                                          ^~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp:117:73: note: suggested alternative: ‘webAssemblyInstantiateStreamingInternal’
         JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION("instantiateStreaming", webAssemblyInstantiateStreamingCodeGenerator, static_cast<unsigned>(PropertyAttribute::DontEnum));
                                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/runtime/JSObject.h:1636:58: note: in definition of macro ‘JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION’
         vm, globalObject, makeIdentifier(vm, (jsName)), (generatorName)(vm), (attributes))
                                                          ^~~~~~~~~~~~~

[Michael Catanzaro]

This affects clean builds and the solution isn't obvious. I'll do a rollout until you have time to look into it!

[Michael Catanzaro]

Reverted r247440 for reason:

Broke builds

Committed r247445: <https://trac.webkit.org/changeset/247445>

[Yusuke Suzuki]

Committed r247457: <https://trac.webkit.org/changeset/247457>

[Truitt Savell]

It looks like the imported/w3c/web-platform-tests/wasm/jsapi/interface.any.worker.html

added in https://trac.webkit.org/changeset/247457/webkit

is flaky on Mac Debug.
History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fwasm%2Fjsapi%2Finterface.any.worker.html

Diff:
--- /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/wasm/jsapi/interface.any.worker-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/wasm/jsapi/interface.any.worker-actual.txt
@@ -1,5 +1,3 @@
-CONSOLE MESSAGE: line 1774: TypeError: null is not an object (evaluating 'this.message_target.removeEventListener')
-CONSOLE MESSAGE: line 154: TypeError: undefined is not an object (evaluating 'WebAssembly.Global.prototype')
 
 Harness Error (FAIL), message = TypeError: undefined is not an object (evaluating 'WebAssembly.Global.prototype')

[Yusuke Suzuki]

*** Bug 173180 has been marked as a duplicate of this bug. ***