1255695 - wasm: don't mask load/store addresses

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Dan Gohman [:sunfish]]

Attachment: wasm-unaligned.patch

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

[Luke Wagner [:luke]]

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);

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dbe6031ba6b

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3dbe6031ba6b