Closed Bug 962488 Opened 6 years ago Closed 6 years ago

Update double-conversion for proper AArch64 support

Categories

(Core :: MFBT, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: marcin, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch fix for a bug (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140116030250

Steps to reproduce:

Mozilla code contains a copy of double-conversion code with wrong detection of AArch64 architecture. 

Upstream bug: https://code.google.com/p/double-conversion/issues/detail?id=33 was reopened and then proper fix was merged. There was 2.0.1 release done with this fix.

Without this patch xulrunner/firefox are not buildable.


Actual results:

Build fails with:

#error Target architecture was not detected as supported by Double-Conversion.


Expected results:

should just build
That added check only seems to check for the LE version of AArch64 while the other arch checks don't... On purpose?
Current porting for AArch64 is for little endian only.

There was some initial work done for big endian AArch64 but it is at very early stage.
Blocks: 962534
Attachment #8363539 - Flags: review?(jwalden+bmo)
Comment on attachment 8363539 [details] [diff] [review]
fix for a bug

Review of attachment 8363539 [details] [diff] [review]:
-----------------------------------------------------------------

I have no problem with the fix, but the usual way to update double-conversion is from the upstream sources via mfbt/double-conversion/update.sh.

I guess we could just say that trying to do an update would be potentially time-consuming, and that the poor soul that does get saddled with updating will pick up this fix anyway.

How about a half-way fix: this patch, plus a copy of the patch in some file applied via update.sh.  The patch copy notes that it's already in upstream and can safely be thrown away if we update past 2.0.1.  r=me with that change.
Attachment #8363539 - Flags: review?(jwalden+bmo) → review+
Attached patch mozilla-962488.patch (obsolete) — Splinter Review
Attachment #8363539 - Attachment is obsolete: true
Attachment #8377724 - Flags: review+
Keywords: checkin-needed
Attachment #8377724 - Flags: review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: checkin-needed
Attachment #8377724 - Attachment is obsolete: true
Attachment #8378221 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6a46f53ad944
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.