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

RESOLVED FIXED in Firefox 62

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: jseward, Assigned: jseward)

Tracking

unspecified
mozilla62
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment)

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.
https://hg.mozilla.org/mozilla-central/rev/710e191b76ff
Status: NEW → RESOLVED
Closed: Last year
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.