Closed
Bug 1316156
Opened 8 years ago
Closed 8 years ago
Sonic/JSMESS/wasm crashes browser
Categories
(Core :: JavaScript Engine: JIT, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: azakai, Assigned: bbouvier)
References
Details
Attachments
(1 file, 1 obsolete file)
4.71 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
STR: Load http://assets.metacade.com/emulators/sonic-wasm.html , which is the Sonic game emulated in JSMESS compiled to wasm. This crashes 100% of the time for me, https://crash-stats.mozilla.com/report/index/62099980-650c-4a47-b800-95f792161108 https://crash-stats.mozilla.com/report/index/c03eeb26-5267-484a-8343-79db62161108 Interestingly, it doesn't appear at first glance to be a wasm issue? Trace is in js::frontend::Parser.
Reporter | ||
Comment 1•8 years ago
|
||
Yet it seems it must be wasm-related somehow, as the asm.js version (which should be identical, just replacing the wasm with asm.js) works ok, http://assets.metacade.com/emulators/sonic.html
Comment 2•8 years ago
|
||
Actually, when I run this in a debug build, I see an assert in CodeGeneratorX86Shared::visitWasmStackArgI64, so this probably is related to wasm. What's bizarre is we're in the !IsConstant() branch, but when I evaluate IsConstant(ins->arg()) in the debugger, it's true, so something is weird...
Assignee | ||
Comment 3•8 years ago
|
||
Wow, ins->arg() doesn't return twice the same value indeed (bits are getting modified), under gcc5.4.0 and clang3.8.0. We might fall under some sort of undefined behavior. Trying to understand a bit more and finding a fix.
Comment 5•8 years ago
|
||
Indeed, I compiled with clang 3.8 (Apple LLVM 8.0.0 clang-800.0.42.1) for the test case in bug 1316321.
Assignee | ||
Comment 6•8 years ago
|
||
As said on irc, lowering should ask for register-or-constant here, not constant or any use (that can end up in a stack slot, which is the case here). Now trying to make a test case.
Attachment #8809045 -
Flags: review?(luke)
Assignee | ||
Comment 7•8 years ago
|
||
Test case actually trivial.
Assignee: nobody → bbouvier
Attachment #8809045 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8809045 -
Flags: review?(luke)
Attachment #8809050 -
Flags: review?(luke)
Assignee | ||
Comment 8•8 years ago
|
||
(confirmed it fixes the test case in bug 1316321 too)
Comment 9•8 years ago
|
||
Comment on attachment 8809050 [details] [diff] [review] usereg.patch Review of attachment 8809050 [details] [diff] [review]: ----------------------------------------------------------------- Great spot!
Attachment #8809050 -
Flags: review?(luke) → review+
Comment 10•8 years ago
|
||
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/56280cec9fe2 Use a constant-or-register for int64 pass-stack-arg; r=luke
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/56280cec9fe2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee | ||
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•