firefox crashed during start in ppc64le architecture

RESOLVED FIXED in Firefox -esr60

Status

()

P5
critical
RESOLVED FIXED
10 months ago
15 days ago

People

(Reporter: menantea, Assigned: menantea, NeedInfo)

Tracking

({crash})

60 Branch
mozilla62
Other
Unspecified
crash
Points:
---

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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 https://bugzilla.redhat.com/show_bug.cgi?id=1498561

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

Updated

10 months ago
Assignee: nobody → menantea
Severity: normal → critical
Status: UNCONFIRMED → ASSIGNED
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

Updated

10 months ago
Attachment #8976811 - Flags: review?(bbouvier)
Priority: -- → P5
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[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+
(Assignee)

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 bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9746e2316ea2
Add signal handling registers for powerpc; r=bbouvier

Comment 4

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9746e2316ea2
Status: ASSIGNED → RESOLVED
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]
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+

Comment 8

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/323494cc7e95
status-firefox61: affected → fixed

Comment 9

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr60/rev/d5eb098522b5
status-firefox-esr60: affected → fixed

Updated

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