Closed Bug 1022142 Opened 6 years ago Closed 6 years ago

OdinMonkey: ARM Simulator crashes running the BananaBench demo.

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
blocking-b2g 2.0+
Tracking Status
firefox31 --- unaffected
firefox32 --- fixed
firefox33 --- fixed
b2g-v1.4 --- unaffected
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: dougc, Assigned: luke)

References

Details

Attachments

(2 files)

After locally reverting the regression noted in bug 1021754, the BananaBench demo still fails with: "JavaScript error: , line 0: uncaught exception: Assertion: Assertion failed: undefined"

Bisecting, with a local revert of bug 1021754, leads to changeset c8a1656249fc from bug 860736.
Summary: Bug 1021754 - OdinMonkey: ARM Simulator crashes running the BananaBench demo. → OdinMonkey: ARM Simulator crashes running the BananaBench demo.
There are some dependant bugs that landed after bug 860736. Here's what I am trying for a backout, in this order:
bug 1021251 changeset 4511d9ac1000
bug 860736 changeset ba505f710435
bug 1001346 changeset b56930ff7e05
bug 1001346 changeset f7c59e556cc6
bug 1001346 changeset c69a670294fa
bug 860736 changeset 066f499d0544
bug 860736 changeset c8a1656249fc
fwiw here's the combined backout patch. With the patch from bug 1021754 and this patch the demo is again working on m-c tip.
Blocks: 1022802
Investigating. Deactivating the asm.js to Ion fast paths makes it not crash anymore.
nominate to "b2g-v2.0+". This bug blocks b2g-v2.0+ bug. Can this bug be fixed soon? This blocks some people's WegGL game works.
blocking-b2g: --- → 2.0?
(In reply to Sotaro Ikeda [:sotaro] from comment #4)
> nominate to "b2g-v2.0+". This bug blocks b2g-v2.0+ bug. Can this bug be
> fixed soon? This blocks some people's WegGL game works.

I am looking for a fix right now, if I haven't found anything in the next few hours I'll consider a backout (which means in this case, set r+ on douglas' patch). Setting myself to assigned to show that there's activity here.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attached patch fix-arm-ion-ffiSplinter Review
I think we found it; comment 3 pointed out a bug in GenerateFFIIonExit.  To hit the bug, you need an FFI with many arguments that gets Ion compiled, so that explains why this this only repro'd for large browser benchmarks.

Thanks dougc for helping to track all these down and finding a simulator testcase.  Benjamin was able to confirm that this fixes the BananaBread crash.
Assignee: benj → luke
Attachment #8438731 - Flags: review?(benj)
Comment on attachment 8438731 [details] [diff] [review]
fix-arm-ion-ffi

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

Thanks!
Attachment #8438731 - Flags: review?(benj) → review+
Douglas, thanks to your help and to a chroot env, I've managed to build firefox for 32 bits with the ARM simulator and this patch fixes the correctness issue (the JS assertion isn't triggered anymore).

Sotaro, could you please confirm the last patch (fix-arm-ion-ffi) fixes the issues you're seeing?
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Benjamin Bouvier [:bbouvier] from comment #8)
> 
> Sotaro, could you please confirm the last patch (fix-arm-ion-ffi) fixes the
> issues you're seeing?

The problem is fixed by applying the patch! Thanks.
Flags: needinfo?(sotaro.ikeda.g)
https://hg.mozilla.org/mozilla-central/rev/da911b57ccdb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8438731 [details] [diff] [review]
fix-arm-ion-ffi

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 860736 (landed in 32)
User impact if declined: correctness issues, if not crashes, in asm.js code on ARM platforms (phone devices, tablets). Game partners complained about this issue in relevant bugs.
Testing completed (on m-c, etc.): m-i, m-c
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: n/a
Attachment #8438731 - Flags: approval-mozilla-aurora?
Blocks a blocker.
blocking-b2g: 2.0? → 2.0+
Comment on attachment 8438731 [details] [diff] [review]
fix-arm-ion-ffi

2.0+ = auto-approval :)
Attachment #8438731 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.