Closed Bug 1403914 Opened 5 years ago Closed 5 years ago

WASM_HUGE_MEMORY is defined after its first use

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: lth, Assigned: lth)

Details

Attachments

(1 file)

WASM_HUGE_MEMORY is defined in WasmTypes.h but only after there's an #ifdef (actually an #ifndef) that depends on it: it guards TlsData::boundsCheckLimit.

This has probably been benign so far because no code breaks if I move the definition to the top of WasmTypes.h, but I tripped over this when trying to implement explicit bounds checking for atomics on x64, all my tests were failing because this field is zero on this platform, while it really should not have been available at all.

Config options like these that affect many files probably should not be in individual headers if it can be helped, but should instead be set up in eg moz.build.
Move the definition to moz.build and add a sanity check in WasmTypes.cpp to check that the configuration is what we expect.
Attachment #8913201 - Flags: review?(luke)
Comment on attachment 8913201 [details] [diff] [review]
bug1403914-define-wasm-huge-memory.patch

Review of attachment 8913201 [details] [diff] [review]:
-----------------------------------------------------------------

Oof good one, and *much* better fix.
Attachment #8913201 - Flags: review?(luke) → review+
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67f7280bb9ff
define WASM_HUGE_MEMORY so it's visible to all. r=luke
https://hg.mozilla.org/mozilla-central/rev/67f7280bb9ff
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.