Closed
Bug 1255695
Opened 9 years ago
Closed 9 years ago
wasm: don't mask load/store addresses
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: sunfish, Assigned: sunfish)
References
Details
Attachments
(1 file)
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
WebAssembly doesn't need alignment masks! This patch removes them, adds support to the ARM simulator for specially handling unaligned loads and stores, and adds a signal handler on ARM to catch unaligned access traps.
In the current patch, the signal handler throws an OutOfBounds exception, which is sufficient to support code translated from asm.js which never does unaligned accesses, however it'll need to do more for full wasm conformance.
I don't presently have a suitable ARM device to test this patch on. A try run[0] confirms that it builds (the orange seems unrelated), however try runs do not appear to run jit-tests, so I've not yet actually tested the patch.
[0] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1fb0dc1edc50&selectedJob=17922383
Comment 1•9 years ago
|
||
Comment on attachment 8729339 [details] [diff] [review]
wasm-unaligned.patch
Review of attachment 8729339 [details] [diff] [review]:
-----------------------------------------------------------------
We've tested general segfault signal handling on ARM before so we know that path works so reusing existing functionality to redirect to the oob handler seems like a safe step. We'll obviously need to get a real device to test on before shipping and to test the proper unaligned-access handling. Ship it!
::: js/src/asmjs/WasmSignalHandlers.cpp
@@ +778,2 @@
> // These checks aren't necessary, but, since we can, check anyway to make
> // sure we aren't covering up a real bug.
Could you instead tighten the below assert to require faultingAddress to be within module.heap() + module.heapLength() (instead of MappedSize) here and below x2?
@@ +1384,5 @@
> #endif
> }
> +
> +bool
> +js::wasm::isPCInWasmCode(void *pc)
Nit:
bool
wasm::IsPCInWasmCode
::: js/src/asmjs/WasmSignalHandlers.h
@@ +66,5 @@
> #endif
>
> +// Test whether the given PC is within the innermost wasm activation. Return
> +// false if it is not, or it cannot be determined.
> +bool isPCInWasmCode(void *pc);
Nit:
bool
IsPCInWasmCode(void* pc);
Attachment #8729339 -
Flags: review+
Comment 3•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•