firefox crashed during start in ppc64le architecture

RESOLVED FIXED in Firefox -esr60



10 months ago
15 days ago


(Reporter: menantea, Assigned: menantea, NeedInfo)



60 Branch

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 fixed, firefox60 wontfix, firefox61 fixed, firefox62 fixed)



(1 attachment, 1 obsolete attachment)



10 months ago
Posted patch firefox_ppc64le.patch (obsolete) — Splinter Review
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

Please, could you consider to review and push this patch. Thanks


10 months ago
Assignee: nobody → menantea
Severity: normal → critical
Has STR: --- → yes
Component: Untriaged → JavaScript Engine: JIT
Ever confirmed: true
Keywords: crash
Product: Firefox → Core
Hardware: Unspecified → Other
Summary: firefox crashed during start in pcc64le architecture → firefox crashed during start in ppc64le architecture


10 months ago
Attachment #8976811 - Flags: review?(bbouvier)
Priority: -- → P5
Comment on attachment 8976811 [details] [diff] [review]

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: 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[31])
>  # 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[01])

nit: gp_regs[1]
Attachment #8976811 - Flags: review?(bbouvier) → review+

Comment 2

10 months ago
Hi Benjamin, please feel free to do the modifications to be in line with rules and trunk. Thank you a lot !!!

Comment 3

10 months ago
Pushed by
Add signal handling registers for powerpc; r=bbouvier

Comment 4

10 months ago
Last Resolved: 10 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Is this something we need to consider backporting?
status-firefox60: --- → wontfix
status-firefox61: --- → affected
status-firefox-esr52: --- → unaffected
status-firefox-esr60: --- → affected
Flags: needinfo?(bbouvier)
Posted patch updated.patchSplinter Review
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
Attachment #8976811 - Attachment is obsolete: true
Flags: needinfo?(bbouvier) → needinfo?(menantea)
Attachment #8980010 - Flags: review+
Attachment #8980010 - Flags: approval-mozilla-release?
Attachment #8980010 - Flags: approval-mozilla-esr60?
Attachment #8980010 - Flags: approval-mozilla-beta?
Comment on attachment 8980010 [details] [diff] [review]

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+

Comment 8

10 months ago
status-firefox61: affected → fixed

Comment 9

10 months ago
status-firefox-esr60: affected → fixed


15 days ago
Depends on: 1532851
You need to log in before you can comment on or make changes to this bug.