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)
Core
JavaScript: WebAssembly
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: jseward, Assigned: jseward)
References
Details
Attachments
(1 file)
25.22 KB,
patch
|
lth
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Updated•7 years ago
|
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 | ||
Comment 1•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jseward
Assignee | ||
Comment 2•7 years ago
|
||
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 3•7 years ago
|
||
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+
Updated•7 years ago
|
Component: JavaScript Engine: JIT → Javascript: Web Assembly
Updated•7 years ago
|
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.
Comment 5•7 years ago
|
||
Backed out for failing wpt at /_mozilla/wasm/jsapi.js.html
Push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=88c929206539f80b6ea7b9954cc2465b76bf5af9
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=182452432&repo=mozilla-inbound&lineNumber=5631
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/08cdcfa7b3f03b4c5911069f0c5922a496de7203
Flags: needinfo?(jseward)
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.
Comment 7•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jseward)
Updated•7 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•