firefox crashed during start in ppc64le architecture

RESOLVED FIXED in Firefox -esr60



a year ago
3 months ago


(Reporter: menantea, Assigned: menantea, NeedInfo)



60 Branch
Dependency tree / graph

Firefox Tracking Flags

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



(1 attachment, 1 obsolete attachment)



a year 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


a year 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


a year 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

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

Comment 3

a year ago
Pushed by
Add signal handling registers for powerpc; r=bbouvier

Comment 4

a year ago
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Is this something we need to consider backporting?
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+


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