1462566 - firefox crashed during start in ppc64le architecture

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[menantea]

Attachment: firefox_ppc64le.patch

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

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

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]

Comment 2

[menantea]

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

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9746e2316ea2
Add signal handling registers for powerpc; r=bbouvier

Comment 4

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/9746e2316ea2

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Is this something we need to consider backporting?

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: updated.patch

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 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8980010 [details] [diff] [review]
updated.patch

Approved for 61.0b8 and ESR 60.1. Not needed for an Fx60 dot release.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/323494cc7e95

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/d5eb098522b5

Comment 10

[Holger Scherer]

Hi all,
sorry i am no linux expert - but just was playing with a CentOS 7.6.1810 on IBM PowerPC7 (ppc7el) and get the same seg fault with 60.8.0 - how can i apply this patch or report details?
-h

Comment 11

[Lars T Hansen [:lth]]

If you're going to apply the patch you'll need to rebuild Firefox from source for your system, and you probably won't want to do that unless you're already comfortable doing that kind of thing. And we're not going to fix this in the Firefox 60.x series.

Your best bet might be to see if CentOS has available a build of Firefox ESR ("Extended Support Relase") 60 for your system, since (according to comment 9) it was fixed on that branch. Failing that, it also appears to be fixed in Firefox 61 and later, if you're able to install a newer version.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.