Closed Bug 1507730 Opened 6 years ago Closed 6 years ago

Assertion failure: Code(reg_) < Registers::Total, at /js/src/jit/Registers.h:62

Categories

(Core :: JavaScript: WebAssembly, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 64+ fixed
firefox63 --- wontfix
firefox64 - fixed
firefox65 - fixed

People

(Reporter: bbouvier, Assigned: bbouvier)

References

Details

Attachments

(1 file, 1 obsolete file)

The following test case (reduced from awsm) found a crash in x86 Ion codegen: new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(` (module (func (param i64)) (func (export "f") i64.const 2 i64.const -9223372036854775808 i64.mul call 0 ) ) `))).exports.f(); Lowering and codegen aren't properly sync'd, causing a invalid register to be used. Not sec critical, because this means eax is used in place of the temp register, and eax *is* a fixed register input for this operation anyway, so this just implies correctness issue. Would be nice to have uplifted though.
Attached patch fix.patch (obsolete) (deleted) — Splinter Review
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Attachment #9025594 - Flags: review?(lhansen)
Comment on attachment 9025594 [details] [diff] [review] fix.patch Oh great, same bug on ARM32.
Attachment #9025594 - Flags: review?(lhansen)
Attached patch fix.patch (deleted) — Splinter Review
Attachment #9025594 - Attachment is obsolete: true
Attachment #9025599 - Flags: review?(lhansen)
Attachment #9025599 - Flags: review?(lhansen) → review+
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/98b78581ff76 Generate a temporary for negative power-of-two constants in mul64 on 32 bits platforms; r=lth
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Please request Beta approval on this when you get a chance. Also, can you please provide some context for the tracking request? It's not clear to me what the impact of this bug is for our users.
Flags: needinfo?(bbouvier)
Flags: in-testsuite+
Comment on attachment 9025599 [details] [diff] [review] fix.patch [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: wasm User impact if declined: Incorrect behavior in wasm code on 32 bits platforms. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Very small patch. String changes made/needed: [ESR Uplift Approval Request] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: Fix Landed on Version: Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): String or UUID changes made by this patch:
Flags: needinfo?(bbouvier)
Attachment #9025599 - Flags: approval-mozilla-esr60?
Attachment #9025599 - Flags: approval-mozilla-beta?
Comment on attachment 9025599 [details] [diff] [review] fix.patch wasm fix for 32bit x86 and arm, approved for 64.0b11 and 60.4.0esr
Attachment #9025599 - Flags: approval-mozilla-esr60?
Attachment #9025599 - Flags: approval-mozilla-esr60+
Attachment #9025599 - Flags: approval-mozilla-beta?
Attachment #9025599 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: