memory/table.init with dropped segment and zero length should not trap
Categories
(Core :: JavaScript: WebAssembly, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox72 | --- | fixed |
People
(Reporter: rhunt, Assigned: rhunt)
References
Details
Attachments
(2 files)
There was a change to the bulk-memory spec to allow 'memory/table.init' with a dropped segment when the length is zero [1]. Glancing at our code, it appears we will trap in this case [2].
[1] https://github.com/WebAssembly/bulk-memory-operations/pull/123#issuecomment-550162679
[2] https://searchfox.org/mozilla-central/rev/d061ba55ac76f41129618d638f4ef674303ec103/js/src/wasm/WasmInstance.cpp#573
Assignee | ||
Comment 1•5 years ago
|
||
This was an ambiguity in the spec between the prose and formalism. The spec
interpreter implements it this way.
Pushed by rhunt@eqrion.net: https://hg.mozilla.org/integration/autoland/rev/4bf64cf38713 Allow dropped segs with mem/table.init when len=0. r=lth
Comment 3•5 years ago
|
||
Backed out for build bustages.
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4bf64cf387138bda40ae1d98dfec594c536b103c&selectedJob=275080780
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=275080780&repo=autoland&lineNumber=24687
Backout: https://hg.mozilla.org/integration/autoland/rev/02538f9dceed8e940abfe9cc8eb56ffee1dab2ec
Assignee | ||
Comment 4•5 years ago
|
||
Oh duh, I knew last night this was going to need a spec-test update.
Assignee | ||
Comment 5•5 years ago
|
||
Our import of the spec tests don't include the change to allow dropped segments
if the length = 0. Ideally we would update the test branch to the latest canon
master, then merge bulk, then merge ref. Unfortunately ref and bulk have
non-trivial interpreter changes that conflict. I solved some of these issues
on the current test branch, but it's gotten more challenging to do again.
For now, it seems easiest to backport the change we need to our test branch,
ask for either bulk-mem or reftypes to move to phase 4, then fix the merge
conflicts publically in the repo of other proposal.
Link to the backport patch [1] [2].
[1] github.com/eqrion/wasm-spec/commit/cb1eb6f3235574ae043d5e1e7e9adc56852d2e4f
[2] github.com/eqrion/wasm-spec/tree/spidermonkey-tree-tests
Depends on D52130
Assignee | ||
Comment 6•5 years ago
|
||
Pushed by rhunt@eqrion.net: https://hg.mozilla.org/integration/autoland/rev/263c02951907 Allow dropped segs with mem/table.init when len=0. r=lth https://hg.mozilla.org/integration/autoland/rev/e318a3f2027c Update spec tests for backported spec change. r=lth
Comment 8•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/263c02951907
https://hg.mozilla.org/mozilla-central/rev/e318a3f2027c
Description
•