Bug 199783

Created attachment 374083 [details] Current Score

Created attachment 374084 [details] Patch

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.

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.

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.

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

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

Created attachment 374105 [details] Patch

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.

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.

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 :)

Comment on attachment 374105 [details] Patch LGTM

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

<rdar://problem/53116419>

../../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)) ^~~~~~~~~~~~~

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

Reverted r247440 for reason: Broke builds Committed r247445: <https://trac.webkit.org/changeset/247445>

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

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')

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