1441436 - ARM64: Simulator should incorporate icache checking

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Lars T Hansen [:lth]]

On ARM and MIPS the instruction-level simulators incorporate a mode for checking that we handle the instruction cache properly when we compile and patch.  We might wish to incorporate this for ARM64 as well.

Work items, roughly:

- existing patch points are under control of JS_SIMULATOR_ARM macro
- some obvious code in JitInterruptHandler that must be adapted
- some obvious code in ExecutableAllocator.h
- some shell support (command line flags, as in --arm-sim-icache-checks)
- needs support in the simulator engine

Comment 1

[Nicolas B. Pierron [:nbp]]

I am currently working on this issue, because the last time we had to track a similar issue was a complete nightmare and a lucky finding. We should avoid relying on luck for finding these nasty bugs.

Comment 2

[Nicolas B. Pierron [:nbp]]

Attachment: Bug 1441436 - ARM64 Simulator: Add D&I cache coherency checks.

Comment 3

[Nicolas B. Pierron [:nbp]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)

Created attachment 9044252 [details]
Bug 1441436 - Draft: Add a Cahcing decoder to catch miss instruction cache invalidation. r=

At the moment this patch is still a work-in-progress and does not contain any mechanism for handling the invalidation (in order to test if it can catch missing invalidation cases).

It successfully catch any rewritten instruction and assert if there is a mismatch between the instruction which used to be present and the current instruction.

I designed this code also as a way to potentially speed-up the decoding phase of the simulator, therefore instead of storing a yes/no boolean like done on ARM/MIPS to check the validity of the recorded instruction, I record the name of the visitor to which it should be dispatched to. Also, to avoid going through the hash-map lookup all the time, the last hash-map hit is cached.

This patch is also meant to minimize the number of lines modified in the imported vixl files. This is achieved by adding one extra line to the Simulator class.

using Decode = CachingDecoder;

Comment 4

[Nicolas B. Pierron [:nbp]]

Attachment: Bug 1441436 - ARM64 Simulator: Add D&I cache coherency checks. r=

Comment 5

[Sean Stangl [:sstangl]]

Nicolas, did you mean to abandon https://phabricator.services.mozilla.com/D19970?

That one seems more recent than https://phabricator.services.mozilla.com/D20378 (which is not abandoned but does not have a reviewer).

Now that Ion has landed it would be appropriate to land these checks.

Comment 6

[Nicolas B. Pierron [:nbp]]

(In reply to Sean Stangl [:sstangl] from comment #5)

Nicolas, did you mean to abandon https://phabricator.services.mozilla.com/D19970?

That one seems more recent than https://phabricator.services.mozilla.com/D20378 (which is not abandoned but does not have a reviewer).

No, I meant the opposite, since https://phabricator.services.mozilla.com/D20378 should now be part of https://phabricator.services.mozilla.com/D19970 .

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aadb254e2a5a
ARM64 Simulator: Add D&I cache coherency checks. r=sstangl

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/aadb254e2a5a