Cranelift: properly handle unknown ref types instead of crashing
Categories
(Core :: JavaScript: WebAssembly, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(3 files)
Once this is done, I think we can re-enable fuzzing for Cranelift, because it will properly error out instead of panicking when it sees an unknown ref type. It's also a better UX than crashing Firefox, so I'll classify this as an enhancement.
Assignee | ||
Comment 1•6 years ago
|
||
This requires https://github.com/CraneStation/cranelift/pull/754 to be merged and Spidermonkey to be bumped before we can merge this.
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
Assignee | ||
Comment 4•6 years ago
|
||
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Comment 6•6 years ago
|
||
Backed out 3 changesets (Bug 1547682) for build bustages: "cannot find function init_frame
in this scope".
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/00ca72fd66c249afe66d16ade5d3c6597f939a95
Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=815455c1634eb1ca7f51f03a6117d4fe037809b5&selectedJob=244128029
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244128029&repo=mozilla-inbound&lineNumber=10763
Assignee | ||
Comment 7•6 years ago
|
||
That's unfortunate: these new failures result from the fact that the failure
crate is now built with the std
feature, as recently enabled by the cranelift-wasm
crate. The failure
crate with std support references an old backtraces
crate that doesn't compile on CI; it is probably the reason why std support for this crate has been disabled by other uses of the failure
crate in tree.
We can:
- disable
failure/std
support in cranelift-wasm, which would imply theWasmError::User()
field to not get afailure::Error
(only defined withfailure/std
support). It can probably be replaced with aBox<std::error::Error>
, or some simple error wrapper, and it wouldn't be too bad. - go the long way: upgrade
backtraces
infailure
, take care of the other link errors etc.
I'll try the former first.
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
I've updated the patches 2 and 3 to take into account the Cranelift PR that would need to be accepted first. If somebody wanted to land this before I come back from PTO, then they'd need to bump the Cranelift commit hash too and re-run mach vendor rust
to regenerate the first patch. I'll keep the needinfo, in case it's not done when I get back.
Updated•5 years ago
|
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bc152a886c75
https://hg.mozilla.org/mozilla-central/rev/d3e245822b9f
https://hg.mozilla.org/mozilla-central/rev/037e2534c8bd
Assignee | ||
Updated•5 years ago
|
Description
•