| Summary: | WebAssembly: import updated spec tests | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | JF Bastien <jfbastien> | ||||||||||||||
| Component: | JavaScriptCore | Assignee: | JF Bastien <jfbastien> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | buildbot, commit-queue, csaavedra, fpizlo, jfbastien, keith_miller, mark.lam, msaboff, ossy, saam, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Bug Depends on: | |||||||||||||||||
| Bug Blocks: | 173159, 173279, 173285, 173290, 165826, 173180 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
JF Bastien
2017-06-12 16:41:31 PDT
Created attachment 312732 [details]
patch
This is ready for review, I still have to run some more tests through so cq- for now.
Created attachment 312733 [details]
Just the JSC changes from the same patch
Attachment 312732 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:94: One line control clauses should not use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:99: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Total errors found: 2 in 75 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 312732 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=312732&action=review > Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:94 > + if (UNLIKELY(scope.exception())) { > + reject(exec, scope, promise); > + } else { This is a semantic difference. Is this what the spec says to do? > Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:118 > + RETURN_IF_EXCEPTION(scope, reject(exec, scope, promise)); I like the old code better. Makes it explicit that we're only rejecting is an exception happens > Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:162 > + RETURN_IF_EXCEPTION(scope, reject(exec, scope, promise)); ditto about semantics here. > Source/JavaScriptCore/wasm/js/WebAssemblyRuntimeErrorPrototype.cpp:38 > +const ClassInfo WebAssemblyRuntimeErrorPrototype::s_info = { "WebAssembly.RuntimeError", &Base::s_info, &prototypeTableWebAssemblyRuntimeError, nullptr, CREATE_METHOD_TABLE(WebAssemblyRuntimeErrorPrototype) }; Why? > Source/JavaScriptCore/wasm/js/WebAssemblyTablePrototype.cpp:47 > +const ClassInfo WebAssemblyTablePrototype::s_info = { "WebAssembly.Table", &Base::s_info, &prototypeTableWebAssemblyTable, nullptr, CREATE_METHOD_TABLE(WebAssemblyTablePrototype) }; Why? Created attachment 312735 [details]
patch
Fix ChangeLog.
Created attachment 312736 [details]
Only the JSC changes for the same patch
Attachment 312735 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:94: One line control clauses should not use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:99: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Total errors found: 2 in 77 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 312732 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=312732&action=review >> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:162 >> + RETURN_IF_EXCEPTION(scope, reject(exec, scope, promise)); > > ditto about semantics here. Ignore me. It is what it says. Comment on attachment 312735 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=312735&action=review r=me > Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:102 > + if (std::numeric_limits<uint32_t>::max() - m_size < delta) > + return false; Please used Checked instead. I think it's easier to read, and it's our way of detecting overflow. > Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:258 > + ? static_cast<uint32_t>(m_instance->loadI32Global(segment->offset.globalImportIndex())) > + : segment->offset.constValue(); indentation is wrong > Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:280 > + if (UNLIKELY(sizeInBytes < segment->sizeInBytes)) > + exception = dataSegmentFail(exec, vm, scope, sizeInBytes, segment->sizeInBytes, offset, ASCIILiteral(", segment is too big")); you should return early here as to not throw another exception over the current one. > Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:94 > + if (UNLIKELY(scope.exception())) { > + reject(exec, scope, promise); > + } else { no braces here. > Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:118 > + RETURN_IF_EXCEPTION(scope, reject(exec, scope, promise)); Ignore my previous comment. This code is more terse. I like it. > Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:185 > + RETURN_IF_EXCEPTION(scope, encodedJSValue()); Since you're touching this code, might as well change it to use "{ }" instead of "encodedJSValue()" Comment on attachment 312735 [details] patch Attachment 312735 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/3920328 New failing tests: wasm.yaml/wasm/spec-tests/start.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/int_literals.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/custom_section.wast.js.default-wasm wasm.yaml/wasm/spec-tests/select.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f64.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/skip-stack-guard-page.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/switch.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/utf8-import-field.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/float_memory.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f64_bitwise.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/const.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f64.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/type.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/get_local.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f32_bitwise.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/inline-module.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/stack.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/unreachable.wast.js.default-wasm wasm.yaml/wasm/spec-tests/call.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/float_literals.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/unwind.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/f32_bitwise.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/break-drop.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/float_memory.wast.js.default-wasm wasm.yaml/wasm/spec-tests/br.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/address.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f64_cmp.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/float_literals.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/func.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/int_exprs.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/conversions.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/comments.wast.js.default-wasm wasm.yaml/wasm/spec-tests/unwind.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/names.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/labels.wast.js.default-wasm wasm.yaml/wasm/spec-tests/custom_section.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/call_indirect.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/call.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/comments.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/i64.wast.js.default-wasm wasm.yaml/wasm/spec-tests/memory_redundancy.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/traps.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/break-drop.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/br.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f64_bitwise.wast.js.default-wasm wasm.yaml/wasm/spec-tests/utf8-import-module.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f32.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/float_misc.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/exports.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f64.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/store_retval.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/block.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f64.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/globals.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/br_if.wast.js.default-wasm wasm.yaml/wasm/spec-tests/get_local.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/br_table.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/globals.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/memory_redundancy.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/stack.wast.js.default-wasm wasm.yaml/wasm/spec-tests/endianness.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/break-drop.wast.js.default-wasm wasm.yaml/wasm/spec-tests/unwind.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/comments.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/f32_bitwise.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/labels.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/if.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/loop.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/forward.wast.js.default-wasm wasm.yaml/wasm/spec-tests/token.wast.js.default-wasm wasm.yaml/wasm/spec-tests/func_ptrs.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/f32_cmp.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/return.wast.js.default-wasm wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/address.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/unwind.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/get_local.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f32.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/call_indirect.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f32.wast.js.default-wasm wasm.yaml/wasm/spec-tests/float_literals.wast.js.default-wasm wasm.yaml/wasm/spec-tests/nop.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/names.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/type.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/i64.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/call_indirect.wast.js.default-wasm wasm.yaml/wasm/spec-tests/custom_section.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/store_retval.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f32_cmp.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/stack.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f32.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/labels.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/resizing.wast.js.default-wasm wasm.yaml/wasm/spec-tests/tee_local.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/float_exprs.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/fac.wast.js.default-wasm wasm.yaml/wasm/spec-tests/imports.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/f64.wast.js.default-wasm wasm.yaml/wasm/spec-tests/utf8-import-field.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/br.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/left-to-right.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/block.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/traps.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/inline-module.wast.js.default-wasm wasm.yaml/wasm/spec-tests/switch.wast.js.default-wasm wasm.yaml/wasm/spec-tests/typecheck.wast.js.default-wasm wasm.yaml/wasm/spec-tests/endianness.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/globals.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/imports.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/start.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/block.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/endianness.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/fac.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/traps.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/call_indirect.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/linking.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/const.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/align.wast.js.default-wasm wasm.yaml/wasm/spec-tests/exports.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/exports.wast.js.default-wasm wasm.yaml/wasm/spec-tests/func.wast.js.default-wasm wasm.yaml/wasm/spec-tests/binary.wast.js.default-wasm wasm.yaml/wasm/spec-tests/labels.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/set_local.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/switch.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/br_if.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/return.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/comments.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/i32.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory.wast.js.default-wasm wasm.yaml/wasm/spec-tests/func_ptrs.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/int_literals.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory_redundancy.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/skip-stack-guard-page.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/switch.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/br.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/address.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/int_literals.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/br_if.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f64_cmp.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/stack.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/f64_cmp.wast.js.default-wasm wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/names.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/set_local.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/break-drop.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/custom_section.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/utf8-import-field.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f64_cmp.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/exports.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/call.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f64_bitwise.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/linking.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/start.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/if.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/select.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/call.wast.js.default-wasm wasm.yaml/wasm/spec-tests/return.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/labels.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/int_exprs.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/return.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/fac.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/jsapi.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/i32.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f32_bitwise.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f32_cmp.wast.js.default-wasm wasm.yaml/wasm/spec-tests/br_table.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/float_misc.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/call.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/token.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/left-to-right.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/f64_bitwise.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/int_literals.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f32.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/float_literals.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/utf8-import-field.wast.js.default-wasm wasm.yaml/wasm/spec-tests/names.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f32_cmp.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/left-to-right.wast.js.default-wasm wasm.yaml/wasm/spec-tests/loop.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/linking.wast.js.default-wasm wasm.yaml/wasm/spec-tests/br_table.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/type.wast.js.default-wasm wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/endianness.wast.js.default-wasm wasm.yaml/wasm/spec-tests/stack.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/globals.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/float_exprs.wast.js.default-wasm wasm.yaml/wasm/spec-tests/i32.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/unreachable.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/br_table.wast.js.default-wasm wasm.yaml/wasm/spec-tests/get_local.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/forward.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/switch.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/br_if.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/call_indirect.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/set_local.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/func_ptrs.wast.js.default-wasm wasm.yaml/wasm/spec-tests/endianness.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/exports.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/br_if.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/unwind.wast.js.default-wasm wasm.yaml/wasm/spec-tests/float_misc.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f64_cmp.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/fac.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/resizing.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/linking.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/tee_local.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/set_local.wast.js.default-wasm wasm.yaml/wasm/spec-tests/memory_redundancy.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/float_exprs.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/func.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/type.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/float_memory.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/linking.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/nop.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/memory_redundancy.wast.js.default-wasm wasm.yaml/wasm/spec-tests/traps.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/store_retval.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/imports.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/if.wast.js.default-wasm wasm.yaml/wasm/spec-tests/conversions.wast.js.default-wasm wasm.yaml/wasm/spec-tests/left-to-right.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/func_ptrs.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/resizing.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/func.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/comments.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/inline-module.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/address.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/traps.wast.js.default-wasm wasm.yaml/wasm/spec-tests/return.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/i64.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/const.wast.js.default-wasm wasm.yaml/wasm/spec-tests/utf8-import-module.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/if.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/tee_local.wast.js.default-wasm wasm.yaml/wasm/spec-tests/imports.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/f64_bitwise.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/nop.wast.js.default-wasm wasm.yaml/wasm/spec-tests/utf8-custom-section-id.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/float_memory.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/binary.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/token.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/tee_local.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/block.wast.js.default-wasm wasm.yaml/wasm/spec-tests/token.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/f32_bitwise.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/align.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/i64.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/forward.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/get_local.wast.js.default-wasm wasm.yaml/wasm/spec-tests/loop.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/float_exprs.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/block.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/skip-stack-guard-page.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/type.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/unreachable.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/break-drop.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/utf8-custom-section-id.wast.js.default-wasm wasm.yaml/wasm/spec-tests/skip-stack-guard-page.wast.js.default-wasm wasm.yaml/wasm/spec-tests/unreachable.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/conversions.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/float_exprs.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/loop.wast.js.default-wasm wasm.yaml/wasm/spec-tests/nop.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/start.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/i32.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/utf8-custom-section-id.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/resizing.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/float_memory.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/tee_local.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/const.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/store_retval.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/resizing.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/forward.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/conversions.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/binary.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/select.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/forward.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/func.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/float_misc.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/float_misc.wast.js.default-wasm wasm.yaml/wasm/spec-tests/typecheck.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/int_exprs.wast.js.default-wasm wasm.yaml/wasm/spec-tests/memory_trap.wast.js.default-wasm wasm.yaml/wasm/spec-tests/start.wast.js.default-wasm wasm.yaml/wasm/spec-tests/int_exprs.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/binary.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/utf8-import-module.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/binary.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/i64.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/float_literals.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/select.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/typecheck.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/custom_section.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/imports.wast.js.default-wasm wasm.yaml/wasm/spec-tests/f32_cmp.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/int_literals.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/fac.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/typecheck.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/const.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/int_exprs.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/jsapi.js.default-wasm wasm.yaml/wasm/spec-tests/utf8-import-field.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/br_table.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/if.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/names.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/utf8-custom-section-id.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/set_local.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/globals.wast.js.default-wasm wasm.yaml/wasm/spec-tests/utf8-custom-section-id.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/loop.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/address.wast.js.default-wasm wasm.yaml/wasm/spec-tests/skip-stack-guard-page.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/token.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/conversions.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/inline-module.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/store_retval.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/unreachable.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/utf8-import-module.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/br.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/typecheck.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/select.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/func_ptrs.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/nop.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/left-to-right.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/i32.wast.js.default-wasm wasm.yaml/wasm/spec-tests/inline-module.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/utf8-import-module.wast.js.wasm-eager-jettison *** Bug 165826 has been marked as a duplicate of this bug. *** Created attachment 312806 [details] patch > > Source/JavaScriptCore/wasm/js/WebAssemblyRuntimeErrorPrototype.cpp:38 > > +const ClassInfo WebAssemblyRuntimeErrorPrototype::s_info = { "WebAssembly.RuntimeError", &Base::s_info, &prototypeTableWebAssemblyRuntimeError, nullptr, CREATE_METHOD_TABLE(WebAssemblyRuntimeErrorPrototype) }; > > Why? > > > Source/JavaScriptCore/wasm/js/WebAssemblyTablePrototype.cpp:47 > > +const ClassInfo WebAssemblyTablePrototype::s_info = { "WebAssembly.Table", &Base::s_info, &prototypeTableWebAssemblyTable, nullptr, CREATE_METHOD_TABLE(WebAssemblyTablePrototype) }; > > Why? Apparently the prototypes print as WebAssembly.XYZ, not WebAssembly.prototype.XYZ ¯\_(ツ)_/¯ > > Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:280 > > + if (UNLIKELY(sizeInBytes < segment->sizeInBytes)) > > + exception = dataSegmentFail(exec, vm, scope, sizeInBytes, segment->sizeInBytes, offset, ASCIILiteral(", segment is too big")); > > you should return early here as to not throw another exception over the > current one. Right, I changed it to an "else". The "for each" thing exits early if there is an exception. Attachment 312806 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:100: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Total errors found: 1 in 75 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 312806 [details] patch Rejecting attachment 312806 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 312806, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: .webkit.org/git/WebKit d2c03b1..3674e61 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 218201 = d2c03b13091d7b8887a9b242f949a59f95fe8cf7 r218202 = 3674e61ad802902da91225b72874a23e8e659596 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/3925408 Created attachment 312813 [details]
patch
Rebase.
Comment on attachment 312813 [details] patch Clearing flags on attachment: 312813 Committed r218216: <http://trac.webkit.org/changeset/218216> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #17) > Comment on attachment 312813 [details] > patch > > Clearing flags on attachment: 312813 > > Committed r218216: <http://trac.webkit.org/changeset/218216> FYI, the following WASM tests fail on AArch64: wasm.yaml/wasm/function-tests/memory-alignment.js.default-wasm wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/align.wast.js.default-wasm wasm.yaml/wasm/spec-tests/align.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory.wast.js.default-wasm wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-tls-context See build.webkit.org for details. (In reply to Csaba Osztrogonác from comment #19) > (In reply to WebKit Commit Bot from comment #17) > > Comment on attachment 312813 [details] > > patch > > > > Clearing flags on attachment: 312813 > > > > Committed r218216: <http://trac.webkit.org/changeset/218216> > > FYI, the following WASM tests fail on AArch64: > wasm.yaml/wasm/function-tests/memory-alignment.js.default-wasm > wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-eager-jettison > wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-call-ic > wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-cjit-yes-tls- > context > wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-tls-context > wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm > wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison > wasm.yaml/wasm/spec-tests/align.wast.js.default-wasm > wasm.yaml/wasm/spec-tests/align.wast.js.wasm-eager-jettison > wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-call-ic > wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-cjit-yes-tls-context > wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-tls-context > wasm.yaml/wasm/spec-tests/memory.wast.js.default-wasm > wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-eager-jettison > wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-call-ic > wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-cjit-yes-tls-context > wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-tls-context > wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-eager-jettison > wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-call-ic > wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-cjit-yes-tls-context > wasm.yaml/wasm/spec-tests/memory_trap.wast.js.wasm-no-tls-context > > See build.webkit.org for details. Ugh crap, I knew I had to test on ARM too! I totally forgot, sorry. I'll build and fix it now, but if it's a problem we can revert my patch. The alignment builds look like WasmOps.h wasn't properly regenerated, similar to: https://bugs.webkit.org/show_bug.cgi?id=173349 Maybe a missing dependency in: Source/JavaScriptCore/DerivedSources.make ? Other failures I'll look into as well, they're separate. I think these are rebuild dependency problems with WasmOps.h being stale despite generateWasmOpsHeader.py changing: function-tests/memory-alignment.js spec-tests/align.wast.js spec-tests/memory.wast.js And these are ARM specific problems: js-api/test_memory_constructor.js spec-tests/memory_trap.wast.js I haven't confirmed yet, I'm still running my build. (In reply to JF Bastien from comment #23) > I think these are rebuild dependency problems with WasmOps.h being stale > despite generateWasmOpsHeader.py changing: > function-tests/memory-alignment.js > spec-tests/align.wast.js > spec-tests/memory.wast.js > > And these are ARM specific problems: > js-api/test_memory_constructor.js > spec-tests/memory_trap.wast.js > > I haven't confirmed yet, I'm still running my build. I just ran all of these with tip-of-tree JSC locally, and they all pass. I'm guessing it's an issue with derived source dependencies as well. Odd, DerivedSources.make seem to do the right thing: WasmOps.h: $(JavaScriptCore)/wasm/generateWasmOpsHeader.py $(JavaScriptCore)/wasm/generateWasm.py $(JavaScriptCore)/wasm/wasm.json $(PYTHON) $(JavaScriptCore)/wasm/generateWasmOpsHeader.py $(JavaScriptCore)/wasm/wasm.json ./WasmOps.h WasmValidateInlines.h: $(JavaScriptCore)/wasm/generateWasmValidateInlinesHeader.py $(JavaScriptCore)/wasm/generateWasm.py $(JavaScriptCore)/wasm/wasm.json $(PYTHON) $(JavaScriptCore)/wasm/generateWasmValidateInlinesHeader.py $(JavaScriptCore)/wasm/wasm.json ./WasmValidateInlines.h WasmB3IRGeneratorInlines.h: $(JavaScriptCore)/wasm/generateWasmB3IRGeneratorInlinesHeader.py $(JavaScriptCore)/wasm/generateWasm.py $(JavaScriptCore)/wasm/wasm.json $(PYTHON) $(JavaScriptCore)/wasm/generateWasmB3IRGeneratorInlinesHeader.py $(JavaScriptCore)/wasm/wasm.json ./WasmB3IRGeneratorInlines.h There's a dependency on generateWasm.py, which is the header that changed. 🤔 Oh this is a Linux build for aarch64! The problem with cmake dependencies should be fixed here: https://bugs.webkit.org/show_bug.cgi?id=173349 IIUC the Xcode build has the dependencies right, it's just cmake that didn't. So everything is good. Let me know if blowing away WasmOps.h and re-building doesn't fix the problems you're seeing. It shouldn't happen once the above patch lands. (In reply to JF Bastien from comment #26) > So everything is good. Let me know if blowing away WasmOps.h and re-building > doesn't fix the problems you're seeing. It shouldn't happen once the above > patch lands. You'll have to manually delete that file from your bots though. Unfortunately the cmake changes won't trigger a regeneration of it until it's changed again. (In reply to Claudio Saavedra from comment #27) > (In reply to JF Bastien from comment #26) > > > So everything is good. Let me know if blowing away WasmOps.h and re-building > > doesn't fix the problems you're seeing. It shouldn't happen once the above > > patch lands. > > You'll have to manually delete that file from your bots though. > Unfortunately the cmake changes won't trigger a regeneration of it until > it's changed again. I don't have access to the bots, I think Ossy does so I CC'd on: https://bugs.webkit.org/show_bug.cgi?id=173349 |