1547682 - Cranelift: properly handle unknown ref types instead of crashing

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

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.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

This requires https://github.com/CraneStation/cranelift/pull/754 to be merged and Spidermonkey to be bumped before we can merge this.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1547682: Cranelift: properly handle unknown type errors; r=lth

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1547682: Reenable wasm fuzzing for cranelift; r=decoder

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1547682: Bump Cranelift to cc216b46b35a797d03c0f3e8b16a2096f1c6db61; r=lth

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9fd0c4622f00
Bump Cranelift to be8a83132df0a277da8fa3e6a9c5d03c4a05d57e; r=lth
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bfcf5af011d
Cranelift: properly handle unknown type errors; r=lth
https://hg.mozilla.org/integration/mozilla-inbound/rev/815455c1634e
Reenable wasm fuzzing for cranelift; r=decoder

Comment 6

[Cristian Brindusan [:cbrindusan]]

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

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

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 the WasmError::User() field to not get a failure::Error (only defined with failure/std support). It can probably be replaced with a Box<std::error::Error>, or some simple error wrapper, and it wouldn't be too bad.
• go the long way: upgrade backtraces in failure, take care of the other link errors etc.

I'll try the former first.

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc152a886c75
Bump Cranelift to cc216b46b35a797d03c0f3e8b16a2096f1c6db61; r=lth
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3e245822b9f
Cranelift: properly handle unknown type errors; r=lth
https://hg.mozilla.org/integration/mozilla-inbound/rev/037e2534c8bd
Reenable wasm fuzzing for cranelift; r=decoder

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/bc152a886c75
https://hg.mozilla.org/mozilla-central/rev/d3e245822b9f
https://hg.mozilla.org/mozilla-central/rev/037e2534c8bd