wasm: Baseline JIT forgets to free register after setglobal

RESOLVED FIXED in Firefox 52

Status

()

Core
JavaScript Engine: JIT
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

(Blocks: 1 bug)

unspecified
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Test case:

new WebAssembly.Module(wasmTextToBinary(`(module
 (global $mut_local (mut i32) (i32.const 0))
 (global $imm_local i32 (i32.const 37))
 (import $imported "globals" "x" (global i32))
 (func $get (result i32)
  i32.const 13
  set_global $mut_local
  get_global $imported
  get_global $mut_local
  i32.add
  get_global $imm_local
  i32.add
 )
 (export "run" $get)
)`));

This asserts:
Assertion failure: isAvailable(r), at /code/mozilla-inbound/js/src/asmjs/WasmBaselineCompile.cpp:657

The reason is that the joinReg is not available, and the reason it is not available is that it got used by setglobal but not freed.
(Assignee)

Comment 1

2 years ago
Created attachment 8802441 [details] [diff] [review]
bug1311287-free-reg.patch

Free the register after setGlobal.

(In general I think we want an assertion in the main decoding loop that checks that the registers are invariant: at the outset, we have some register set, and after each iteration the union of the available registers and the registers on the evaluation stack equals the initial register set.  That's followup work.)
Attachment #8802441 - Flags: review?(bbouvier)
Comment on attachment 8802441 [details] [diff] [review]
bug1311287-free-reg.patch

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

Thanks!
Attachment #8802441 - Flags: review?(bbouvier) → review+

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e2da3bb6654f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.