1554053 - Wasm via Cranelift: size of interrupt check comparison is incorrect

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect)

Description

[Julian Seward [:jseward]]

Wasm-via-Cranelift generates code to do interrupt checks by doing a machine-word sized comparison. However, the compared-against value, TlsData::interrupt, is an Atomic<uint32_t, mozilla::Relaxed>, and so the comparison is incorrect on all 64 bit targets: it also compares the 4 bytes following TlsData::interrupt, which look to me as if they are an alignment hole (iow, junk).

This is obviously incorrect, and it's observably inconsistent with what the -via-Ion and -baseline routes do, which is to always generate a 32-bit comparison.

It also holds a potential danger of a store-forwarding stall (big read after small write), although, based on the struct layout and detailed reading of the Intel opt guide, I think it's probably harmless for the Intel Core architecture family.

Comment 1

[Julian Seward [:jseward]]

Attachment: Bug 1554053 - Wasm via Cranelift: size of interrupt check comparison is incorrect.

This patch changes Wasm-via-CL to always use a 32-bit comparison for interrupt checks.

Wasm-via-Cranelift generates code to do interrupt checks by doing a
machine-word sized comparison. However, the compared-against value,
TlsData::interrupt, is an Atomic<uint32_t, mozilla::Relaxed>, and so the
comparison is incorrect on all 64 bit targets: it also compares the 4 bytes
following TlsData::interrupt, which look to me as if they are an alignment
hole (iow, junk).

This is obviously incorrect, and it's observably inconsistent with what the
-via-Ion and -baseline routes do, which is to always generate a 32-bit
comparison.

It also holds a potential danger of a store-forwarding stall (big read after
small write), although, based on the struct layout and detailed reading of the
Intel opt guide, I think it's probably harmless for the Intel Core
architecture family.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c858f52192ad
Wasm via Cranelift: size of interrupt check comparison is incorrect. r=bbouvier

Comment 3

[Cristian Brindusan [:cbrindusan]]

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