Closed Bug 1462566 Opened 2 years ago Closed 2 years ago
firefox crashed during start in ppc64le architecture
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20180509235650 Steps to reproduce: firefox crashes during start in pcc64le architecture. Actual results: A signal is received soon in the process trigs the crash, this signal is processed thru RedirectJitCodeToInterruptCheck() function in WasmSignalHandlers.cpp and when it reach *ContextToPC(context) , it crashes because we are defined as JS_CODEGEN_NONE for powerpc but there are no defines to handle powerpc KNOWS_MACHINE_STATE. See last comments of bug 1360254. Expected results: I wrote a small patch correcting this problem so far and allow firefox to run on ppc64le. see fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=1498561 Please, could you consider to review and push this patch. Thanks
Assignee: nobody → menantea
Severity: normal → critical
Status: UNCONFIRMED → ASSIGNED
Has STR: --- → yes
Ever confirmed: true
Product: Firefox → Core
Hardware: Unspecified → Other
Summary: firefox crashed during start in pcc64le architecture → firefox crashed during start in ppc64le architecture
Comment on attachment 8976811 [details] [diff] [review] firefox_ppc64le.patch Review of attachment 8976811 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for opening a bug and making a patch! Would you mind please: - format the patch according to our requested rules: https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F see changeset message conventions - make a patch that's based on trunk (I *think* it wouldn't require changes, but would be nice to make sure of that) - address the one review comment below I'd be happy to do those for you, if you didn't get time to do it. Thanks! ::: firefox-58.0.1/js/src/wasm/WasmSignalHandlers.cpp.ori @@ +175,5 @@ > # define R31_sig(p) ((p)->uc_mcontext.gregs) > # endif > +# if defined(__linux__) && (defined(__ppc64__) || defined (__PPC64__) || defined(__ppc64le__) || defined (__PPC64LE__)) > +// powerpc stack frame pointer (SFP or SP or FP) > +# define R01_sig(p) ((p)->uc_mcontext.gp_regs) nit: gp_regs
Attachment #8976811 - Flags: review?(bbouvier) → review+
Hi Benjamin, please feel free to do the modifications to be in line with rules and trunk. Thank you a lot !!!
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/9746e2316ea2 Add signal handling registers for powerpc; r=bbouvier
Is this something we need to consider backporting?
Yes indeed. It's definitely a nice to have on all versions (so if it could be a ride-along for esr/release, would be nice, but doesn't warrant too much extra work). Reporter: can you confirm Spidermonkey runs fine on a powerpc machine now, please? Approval Request Comment [Feature/Bug causing the regression]: (none in particular) [User impact if declined]: can't compile Spidermonkey / Firefox on powerpc [Is this code covered by automated tests?]: no (tier 3 platform) [Has the fix been verified in Nightly?]: don't have a powerpc machine to test [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: (ifdef on powerpc) [String changes made/needed]: none
Comment on attachment 8980010 [details] [diff] [review] updated.patch Approved for 61.0b8 and ESR 60.1. Not needed for an Fx60 dot release.
Attachment #8980010 - Flags: approval-mozilla-release?
Attachment #8980010 - Flags: approval-mozilla-release-
Attachment #8980010 - Flags: approval-mozilla-esr60?
Attachment #8980010 - Flags: approval-mozilla-esr60+
Attachment #8980010 - Flags: approval-mozilla-beta?
Attachment #8980010 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.