Status

()

defect
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: emilio, Assigned: emilio)

Tracking

unspecified
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox63 fixed)

Details

Attachments

(3 attachments)

No description provided.
Comment on attachment 8989047 [details]
Bug 1472538: Update bindgen.

https://reviewboard.mozilla.org/r/254128/#review260846
Attachment #8989047 - Flags: review?(xidorn+moz) → review+
https://hg.mozilla.org/mozilla-central/rev/2e71ff3c9bd3
https://hg.mozilla.org/mozilla-central/rev/ad59c8bf30b4
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Duplicate of this bug: 1462623
Here at SUSE we have encountered this exact same bug running Firefox 60 ESR as described in https://bugzilla.mozilla.org/show_bug.cgi?id=1462623 (and fixed in this bug) but on a s390x IBM (big-endian) machine. 

I am asking you to push/uplift this fix to the ESR branch and make it available for Firefox 60.3 ESR when it is released in October.

We have "BIG" customers waiting for a clean build on s390x and so far no ESR 60 versions of Firefox will run on a big-endian
machine without crashing due to this bug.

Thank you!
Flags: needinfo?(emilio)
Yeah, I think uplifting this is reasonable. I'll request it.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Firefox is unusable without this patch in big-endian machines (see bug 1462623).
User impact if declined: Crashes at startup in tier-3 platforms.
Fix Landed on Version: 63
Risk to taking this patch (and alternatives if risky): I think it's not very risky.

This updates a build-time-only dependency, which is maintained by us (by me mostly, in fact), and this particular version we've been using since FF63, so it's pretty well tested (and again, it's build-time only).

Note that the patch looks way bigger and more complex than what it really is (the patch is mostly https://hg.mozilla.org/mozilla-central/rev/2e71ff3c9bd3) because I merged the automated dependency update / vendored code update in the patch (the result of ./mach vendor rust after applying the previous patch and resolving conflicts). I could try to make that part a bit more minimal if wanted / needed.

String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(emilio)
Attachment #9011149 - Flags: approval-mozilla-esr60?
Comment on attachment 9011149 [details] [diff] [review]
Rebased patch for esr60.

This looks mildly terrifying, but I'll take your word for it that it's not as bad as it looks. Fixes crashes for some Tier 3 platforms, approved for ESR 60.3.
Attachment #9011149 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
One thing I had to do to make esr60 build with the rebased patch is to update just the cc crate and re-vendor it.  Otherwise it was creating the argv incorrectly, passing both the program name and the first argument in argv[0] - something like "gcc -first-option" "-second-option" "etc".  Emilio mentioned that he had seen this bug before.

I'm not sure if that cc update should go into the same rebased patch - I'm not sure if the patch is "update just bindgen and its dependencies", or "update all the crates that need bugfixes in esr60" :)
(In reply to Federico Mena Quintero from comment #11)
> One thing I had to do to make esr60 build with the rebased patch is to
> update just the cc crate and re-vendor it.  Otherwise it was creating the
> argv incorrectly, passing both the program name and the first argument in
> argv[0] - something like "gcc -first-option" "-second-option" "etc".  Emilio
> mentioned that he had seen this bug before.

That was bug 1445528. Probably you should request an uplift of that one if you need it. It's build-time only, so should be pretty safe as well.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)
> That was bug 1445528. Probably you should request an uplift of that one if
> you need it. It's build-time only, so should be pretty safe as well.

Thanks, I'll request an uplift there.
You need to log in before you can comment on or make changes to this bug.