1687949 - ARM64 simulator has no oob handling for SIMD load/store instructions

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Lars T Hansen [:lth]]

Everywhere in the simulator we see ReadUintFromMem or WriteUintToMem, we need to have OOB handling for the simulator to work correctly on SIMD code. The baseline compiler's generated code does not seem to be affected here, but Cranelift's code is.

Note you need the patch from bug 1687936 to enable SIMD with the simulator, and to get an important bugfix.

Simple test case which will make Cranelift crash (and probably Ion, once we get that far):

var run = new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
(module
  (memory 1)
  (func $f (export "splat") (param $address i32) (result v128)
    (v128.load8_splat (local.get $address)))
  (func (export "run")
    (call $f ( i32.const -1 ))
    drop))
`)));
run.exports.run();

Comment 1

[Lars T Hansen [:lth]]

This is compounded by a double-fun part of the simulator: it double-faults for SIMD loads and stores because it performs some reads twice. But when we get to the double fault, the PC has been updated, so we crash hard. That has been fixed upstream, so let's translate the fix to the local version.

Comment 2

[Lars T Hansen [:lth]]

Attachment: Bug 1687949 - Make the vector load/store instructions fault properly. r?nbp

Two fixes here:

First, every time we try to execute a vector memory instruction, check
if the access would fault, and bail out in the standard manner if it
would.

Second, fix a bug in the implementation of the NEON decoder where some
load instructions would be executed twice. If the first execution
faults then the fault handler moves the PC, and then the second fault
will crash hard. The fix here has been imported from upstream, more
or less.

Depends on D102594

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/becb6a284f38
Make the vector load/store instructions fault properly.  r=nbp

Comment 4

[Natalia Csoregi [:nataliaCs]]

Backed out for spidermonkey bustage

backout: https://hg.mozilla.org/integration/autoland/rev/87fc8320394fff94d5a47651e4a2d95143d71880

push: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=linux%2Cdebug%2Cspidermonkey%2Cbuilds%2Cspidermonkey-sm-arm-sim-linux32%2Fdebug%2Carm&revision=becb6a284f3835529cab8da85591874785a8cab4&selectedTaskRun=f8hoz5luQK6xIRPHxN5SqA.1

failure log: https://treeherder.mozilla.org/logviewer?job_id=327726708&repo=autoland&lineNumber=551

[task 2021-01-25T19:14:04.246Z] ERROR: --enable-wasm-simd only possible when targeting the x86_64/x86/arm64 jits
[task 2021-01-25T19:14:04.298Z] Traceback (most recent call last):
[task 2021-01-25T19:14:04.298Z] File "/builds/worker/checkouts/gecko/js/src/devtools/automation/autospider.py", line 514, in <module>
[task 2021-01-25T19:14:04.298Z] check=True,
[task 2021-01-25T19:14:04.298Z] File "/builds/worker/checkouts/gecko/js/src/devtools/automation/autospider.py", line 429, in run_command
[task 2021-01-25T19:14:04.298Z] raise subprocess.CalledProcessError(status, command, output=stderr)
[task 2021-01-25T19:14:04.298Z] subprocess.CalledProcessError: Command '['sh', '-c', '/builds/worker/checkouts/gecko/js/src/configure --enable-stdcxx-compat --disable-gold --enable-simulator=arm --target=i686-pc-linux --enable-rust-simd --enable-optimize --enable-debug --target=i686-pc-linux --enable-nspr-build --prefix=/builds/worker/workspace/obj-spider/dist']' returned non-zero exit status 1.
[task 2021-01-25T19:14:04.305Z] + BUILD_STATUS=1
[task 2021-01-25T19:14:04.305Z] + upload=0
[task 2021-01-25T19:14:04.305Z] + '[' -n '' ']'
[task 2021-01-25T19:14:04.306Z] + '[' 0 = 1 ']'
[task 2021-01-25T19:14:04.306Z] + cat
[task 2021-01-25T19:14:04.306Z] + exit 1
[taskcluster 2021-01-25 19:14:06.650Z] === Task Finished ===
[taskcluster 2021-01-25 19:14:07.164Z] Unsuccessful task run with exit code: 1 completed in 486.595 seconds

Comment 5

[Lars T Hansen [:lth]]

Actually a problem with the patch on bug 1687936.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70f991dba3a8
Make the vector load/store instructions fault properly.  r=nbp

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/70f991dba3a8