Closed Bug 1467071 Opened 7 years ago Closed 7 years ago

Wasm: import embedding_limits "limits.js" test and fix any resulting failures

Categories

(Core :: JavaScript: WebAssembly, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: jseward, Assigned: jseward)

References

Details

Attachments

(1 file)

The spec branch [1] (see also, more generally, comments in [2]) contains a new test, limits.js, to check whether the generally agreed embedding limits (numbers of functions, imports, etc) are observed. This bug is to import the test and fix any resulting breakage detected with it. [1] https://github.com/WebAssembly/spec/tree/embedding_limits [2] https://github.com/WebAssembly/spec/issues/607
Summary: Wasm: import embedding_limits.js test and fix any resulting failures → Wasm: import embedding_limits "limits.js" test and fix any resulting failures
Assignee: nobody → jseward
Comment on attachment 8983729 [details] [diff] [review] bug1467071-add-limit-tests-1.diff Lars, what do you think? f? because I'm still running tests. Note that some of the tests fail due to OOM on a 32 bit shell build: ==== Test table entries limit = 10000000 ==== Validate table entries mininum: PASS. Validate table entries limit: FAIL. out of memory Validate table entries over limit: PASS. Compile table entries mininum: PASS. Compile table entries limit: FAIL. Error: UNEXPECTED FAIL: table entries == 10000000 (out of memory) Compile table entries over limit: PASS.
Attachment #8983729 - Flags: feedback?(lhansen)
Comment on attachment 8983729 [details] [diff] [review] bug1467071-add-limit-tests-1.diff Review of attachment 8983729 [details] [diff] [review]: ----------------------------------------------------------------- In the commit msg, re table length, the spec number is 10e7, not 10e5 as here. Otherwise fine. It's sort of grim to land this if we know it's going to OOM (and run for a long time) but I guess we can mark it as slow and intermittently-failing. ::: js/src/wasm/WasmValidate.cpp @@ +1230,5 @@ > > + if (limits.initial > MaxTableInitialLength || > + ((limits.maximum.isSome() && > + (limits.maximum.value() > MaxTableMaximumLength || > + limits.initial > limits.maximum.value())))) DecodeLimits already checks initial <= maximum.
Attachment #8983729 - Flags: feedback?(lhansen) → feedback+
Component: JavaScript Engine: JIT → Javascript: Web Assembly
Attachment #8983729 - Flags: feedback+ → review+
Pushed by jseward@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/88c929206539 Wasm: import embedding_limits "limits.js" test and fix any resulting failures. r=lth.
Pushed by jseward@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/710e191b76ff Wasm: import embedding_limits "limits.js" test and fix any resulting failures. r=lth.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Flags: needinfo?(jseward)
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: