Closed Bug 1573815 Opened 2 months ago Closed 2 months ago

ref.func should handle error conditions better

Categories

(Core :: Javascript: WebAssembly, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: rhunt, Assigned: rhunt)

References

Details

Attachments

(1 file)

Not sure how to link to review comments in Phabricator.

WasmInstance.cpp:1025

lth:
This is wrong, I think. It allows an AnyRef::invalid() back into running wasm code, because the jitted code does not guard against an invalid value. However, since the validator has checked that the index is valid, should we not just MOZ_CRASH here?
				
bbouvier:
This can also happen during an OOM in getExportedFunction, and failing gracefully seems nicer than a plain crash here?
				
lth:
A fair point. But then we need to check the value in the generated code and propagate the OOM.

It sounds like we should have the generated code guard against an invalid ref and handle it as an OOM.

WasmInstance::funcRef has 'FailureMode::FailOnInvalidRef' so it looks like we
just need to report the OOM and return InvalidRef.

Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/893922ffdc30
Wasm: Report OOM correctly in 'ref.func' r=lth
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.