1627181 - Could the arm64 simulator handle unreachables the same way as hardware?

Status: RESOLVED FIXED

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

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

To efficiently implement the unreachable wasm opcode, Spidermonkey generates an actually undefined instruction on all platforms. On x86, it's ud2, which triggers a SIGILL signal. On ARM64, it's a udf variant, which has the same behavior.

Only that... it's udf on real hardware, and something different in the simulator (hlt with a custom payload). Now, I'm trying to integrate Spidermonkey with the new Cranelift ARM64 backend in bug 1618595, and this difference in behaviors makes it complicated: we'd have to leak Spidermonkey's behavior one way or another into Cranelift (with a special setting if the spidermonkey simulator is enabled, or an env var, etc.).

To avoid this unfortunate situation, could we have the simulator properly handle the one undefined instruction pattern we're using on real hardware?

The only concern i have is that it will imply modifying the vixl's decoder. I seem to recall we don't want to do this, to make upstream integration easier. But as far as I recall, it hasn't been updated for at least a few years anyways, so maybe this is rather a theoretical concern? Lars, Jan, thoughts?

Comment 1

[Nicolas B. Pierron [:nbp]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #0)

To avoid this unfortunate situation, could we have the simulator properly handle the one undefined instruction pattern we're using on real hardware?

This sounds ok to me.

The only concern i have is that it will imply modifying the vixl's decoder. I seem to recall we don't want to do this, to make upstream integration easier. But as far as I recall, it hasn't been updated for at least a few years anyways, so maybe this is rather a theoretical concern? Lars, Jan, thoughts?

We already have a few limited changes on the code imported from vixl. Sean updated it a year ago, such as Bug 1551339.
Usually we try to isolate our changes to wrapper headers, but for decoding you might have to modify the imported code. Which is ok.

Comment 2

[Jan de Mooij [:jandem]]

+1. We shouldn't work around issues with the simulator by emitting different code.

Comment 3

[Lars T Hansen [:lth]]

I agree, we should make the simulator work for us, modifications are OK, especially when they are fairly local.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1627181: Unify arm64 simulator and hardware representation of an undefined instruction; r?lth

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e84db114345
Unify arm64 simulator and hardware representation of an undefined instruction; r=lth

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/4e84db114345