1264836 - bigendian support for ICU

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Steve Singer (:stevensn)]

See the https://bugzilla.mozilla.org/show_bug.cgi?id=1239083#c32

Since Bug 1239083 builds on my big endian machine (ppc64) have been crashing at startup

  ./dist/bin/firefox --new-instance
[29484] ###!!! ABORT: JS_InitWithFailureDiagnostic: u_init() failed: file /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/build/XPCOMInit.cpp, line 709
[29484] ###!!! ABORT: JS_InitWithFailureDiagnostic: u_init() failed: file /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/build/XPCOMInit.cpp, line 709
Segmentation fault (core dumped)

Comment 1

[Mike Hommey [:glandium]]

Huh, we definitely should fail at configure time.

Comment 2

[(not currently active) Ted Mielczarek]

Yeah, I didn't have an endianness check handy when I was writing the patch, but we could expose that now and make this into an error:
https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/build/autoconf/icu.m4#79

It should just tell you to either use --with-system-icu or --without-intl-api.

Comment 3

[Mike Hommey [:glandium]]

We could maybe add an option to just feed a big-endian data file, and still build icu from our code base. Not sure how valuable that would be.

Comment 4

[Steve Singer (:stevensn)]

--with-system-icu  on a system with icu 54 doesn't work.  js/src/builtin/Intl.cpp has a few things in it that seem to need a newer ICU (maybe we should update the ICU version  in the configure check)

Things build against a locally installed ICU 56 using --with-system-icu but I suspect this will cause a lot of pain for downstream packagers on platoforms that have the older ICU.

I think Mike's idea of an option to feed in a big endian datafile would be valuable but someone actually involved in packaging firefox for downstream ppc64 distributions should comment.

Comment 5

[mcspainhower]

I am getting the same segfault on an i7 (afaik little endian)


$ /usr/bin/firefox 
[7563] ###!!! ABORT: JS_InitWithFailureDiagnostic: u_init() failed: file /build/firefox-HNVPVM/firefox-48.0~a2~hg20160509r318685/xpcom/build/XPCOMInit.cpp, line 709
[7563] ###!!! ABORT: JS_InitWithFailureDiagnostic: u_init() failed: file /build/firefox-HNVPVM/firefox-48.0~a2~hg20160509r318685/xpcom/build/XPCOMInit.cpp, line 709
Segmentation fault (core dumped)

Using the nightly PPA - http://ppa.launchpad.net/ubuntu-mozilla-daily/firefox-aurora/ubuntu


System
------

kernel: Linux 4.4.0-22-generic #39-Ubuntu SMP Thu May 5 16:53:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
distro: Ubuntu 16.04 LTS
cpu: Intel(R) Core(TM) i7-4712HQ CPU @ 2.30GHz

Comment 6

[harpreet]

same here.

Comment 7

[Andre Klapper]

The output
> ABORT: JS_InitWithFailureDiagnostic: u_init() failed: file /firefox-48.0~a2~hg20160509r318685/xpcom/build/XPCOMInit.cpp, line 709")
is mentioned in bug 1262731, bug 1264836, bug 1272246, bug 1272209. (In case some might be dups.)

Comment 8

[Makoto Kato [:m_kato]]

icupkg can create big endian data from little endian data.  But icupkg generates error when converting our data file...

Comment 9

[Makoto Kato [:m_kato]]

Reported to ICU team.

Comment 10

[Petr Sumbera]

I ran into this issue while building FF for Solaris on sparc. And yes icupkg fail. It's failing because of 'confusables' record. So following works:

icupkg -tb -r confusables.cfu icudt58l.dat

At this moment I use this:

--- firefox-52.0b8/build/autoconf/icu.m4.orig   2017-03-01 04:31:04.958804345 +0000
+++ firefox-52.0b8/build/autoconf/icu.m4        2017-03-01 04:38:54.277500982 +0000
@@ -75,10 +75,14 @@
     fi
     MOZ_ICU_VERSION="$version"

-    # TODO: the l is actually endian-dependent
-    # We could make this set as 'l' or 'b' for little or big, respectively,
-    # but we'd need to check in a big-endian version of the file.
     ICU_DATA_FILE="icudt${version}l.dat"
+    case "$CPU_ARCH" in
+    sparc*)
+      # create big-endian data file (v58 fails with 'confusables' record)
+      icupkg -tb -r confusables.cfu $_topsrcdir/config/external/icu/data/$ICU_DATA_FILE
+      ICU_DATA_FILE="icudt${version}b.dat"
+      ;;
+    esac

     dnl We won't build ICU data as a separate file when building
     dnl JS standalone so that embedders don't have to deal with it.

Should we include some patch like this? Or what should be final resolution?

Comment 11

[Makoto Kato [:m_kato]]

(In reply to Petr Sumbera from comment #10)
> Should we include some patch like this? Or what should be final resolution?

Not enough.  We don't put big endian's dat file into our tree because it is no way to create it on little endian environment.  If http://bugs.icu-project.org/trac/ticket/11046 is fixed, we can generate it and include it.

So if you use big endian CPU, you should use --with-system-icu in .mozconfig now.

Comment 12

[Charles Robertson]

We were having problems building FF 52esr on a s390x and ppc64, both big-endian with the same error described in the description. After a couple of weeks of intense research we were able to solve this problem. (NOTE: I develop on a x86_64 box and push to other systems to build.)

We were able to successfully create a icudt58b.dat (big-endian) data file (on a little-endian machine) and plug it into the mozilla-esr52 source (via a similar patch as provided by Petr Sumbera in comment 10) which successfully built on our big-endian machines. According to the ICU documentation these data files are architecture independent except for endianness which can be converted using the icupkg utility.

The problem is trying to convert the icudt58l.dat file provided in the mozilla-esr52 repo. I believe this file is corrupt because it is not the same size as the same file provided by ICU source (http://download.icu-project.org/files/icu4c/58.2/icu4c-58_2-src.tgz) and it produces an error when converting:

$ ~/icu/source/bin/icupkg -tb ~/mozilla-esr52/config/external/icu/data/icudt58l.dat icudt58b.dat
udata_swap(): unknown data format 00.00.00.00 ("")
icupkg: udata_swap(item 249) failed - U_UNSUPPORTED_ERROR
$ echo $?
16

However, using the data file from ICU the conversion works perfect:

$ ~/icu/source/bin/icupkg -tb ~/icu/source/data/in/icudt58l.dat icudt58b.dat
$ echo $?
0
$ ll icudt58*
-rw-r--r-- 1 me users 26213232 May 25 17:26 icudt58b.dat

We had no problems building FF52esr on big-endian machines using this icudt58b.dat data file.

You might want to consider replacing the icudt58l.dat file with the good data file form ICU and applying the patch from Petr Sumbera and it should fix this problem for everyone.

Comment 13

[Makoto Kato [:m_kato]]

(In reply to Charles Robertson from comment #12)
> You might want to consider replacing the icudt58l.dat file with the good
> data file form ICU and applying the patch from Petr Sumbera and it should
> fix this problem for everyone.

Charles, although I add comment #11, since we have to land ICU Data file into our repository, we need generate bigendian data on littleendian PC such as macOS/x86-64 if we support this.  So if http://bugs.icu-project.org/trac/ticket/11046 is fixed, we can support it without any patches.

Comment 14

[John Paul Adrian Glaubitz]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> Yeah, I didn't have an endianness check handy when I was writing the patch,
> but we could expose that now and make this into an error:
> https://dxr.mozilla.org/mozilla-central/rev/
> 21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/build/autoconf/icu.m4#79

I'm honestly a bit shocked to see that such a patch that breaks the build on all big-endian targets was merged without any problems. "I didn't have an endianness check handy" sounds like a very bad excuse to me. This patch shouldn't have been merged in the first place as-is.

Lots of users have run into this issue, this includes Thunderbird users. Debian's Thunderbird have racked their brains about why they weren't able to build Thunderbird on Big-Endian targets anymore.

Comment 15

[(not currently active) Ted Mielczarek]

We don't support any big-endian targets as Tier-1. Sorry. This work fixed a lot of other issues for us so it was worthwhile.

Comment 16

[John Paul Adrian Glaubitz]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #15)
> We don't support any big-endian targets as Tier-1. Sorry. This work fixed a
> lot of other issues for us so it was worthwhile.

It's one thing to not support a target, but another intentionally break that target.

In five years it will be:

> We don't support any Linux targets as Tier-1. Sorry.

I don't quite know what Mozilla's goals in the future are, but I really don't like the direction where this is all going.

If the future means that only a certain target audience on certain platforms is allowed to run Firefox, then I don't think it will be justified to call Firefox the browser developed by the community for the community.

Comment 17

[(not currently active) Ted Mielczarek]

You are making quite a ridiculous argument here. Aside from Mac PowerPC, we have never supported a big-endian platform as Tier-1. We break unsupported platforms all the time in service of fixes that benefit other platforms. Your supposed slippery slope argument doesn't hold water.

People have always been free to build Firefox for unsupported platforms, but that does not mean Mozilla is under any obligation to keep those builds working. Even with this particular breakage you could continue to build Firefox --with-system-icu, so it's not terribly onerous.

I would take a patch to make configure produce an error when targeting big-endian platforms when --with-system-icu has not been specified, but I doubt we're likely to do much more than that.

Comment 18

[John Paul Adrian Glaubitz]

> You are making quite a ridiculous argument here. Aside from Mac PowerPC, we have never supported a big-endian platform as Tier-1. We break unsupported platforms all the time in service of fixes that benefit other platforms. Your supposed slippery slope argument doesn't hold water.

Such a ridiculous argument that you have to use your admin rights to hide it.

> People have always been free to build Firefox for unsupported platforms, but that does not mean Mozilla is under any obligation to keep those builds working. Even with this particular breakage you could continue to build Firefox --with-system-icu, so it's not terribly onerous.

This is basically what's necessary to fix the build on big-endian architectures. I agree, that's quite a lot of work.

> https://anonscm.debian.org/cgit/pkg-mozilla/iceweasel.git/commit/?h=esr52/master&id=4dd8c5d6dc4d6a712f1c73d5c8e12bc372a331e1
> https://anonscm.debian.org/cgit/pkg-mozilla/iceweasel.git/commit/?h=esr52/master&id=1255e75f0cb491f34467f4c2e684b5ddec91c4ee

> I would take a patch to make configure produce an error when targeting big-endian platforms when --with-system-icu has not been specified, but I doubt we're likely to do much more than that.

You're free to do whatever you want with your own project, of course. But don't be surprised if the community loses interest over time. There have been other projects like OpenOffice and XFree86 where upstream started to ignore the community and we all know where this lead to. Either way, I will probably not contribute to Firefox anymore.

Comment 19

[Nathan Froyd [:froydnj]]

(In reply to John Paul Adrian Glaubitz from comment #18)
> > You are making quite a ridiculous argument here. Aside from Mac PowerPC, we have never supported a big-endian platform as Tier-1. We break unsupported platforms all the time in service of fixes that benefit other platforms. Your supposed slippery slope argument doesn't hold water.
> 
> Such a ridiculous argument that you have to use your admin rights to hide it.

Nobody is using admin rights here; https://wiki.mozilla.org/BMO/comment_tagging#Automatic_Collapsing_of_Comments describes what is going on.  Somebody decided to flag your comment as something an admin should look at, and Bugzilla collapses such comments by default.  All the content is still there.

Ted's correct: we have broken non-Tier 1 builds all the time.  Ask Sparc/PowerPC *BSD maintainers about Skia changes breaking their big-endian platforms.  This is simply a fact of life.  And we have taken patches for such breakages where people have been motivated enough to fix them.

Comment 20

[Landry Breuil (:gaston)]

(In reply to John Paul Adrian Glaubitz from comment #18)

> > People have always been free to build Firefox for unsupported platforms, but that does not mean Mozilla is under any obligation to keep those builds working. Even with this particular breakage you could continue to build Firefox --with-system-icu, so it's not terribly onerous.
> 
> This is basically what's necessary to fix the build on big-endian
> architectures. I agree, that's quite a lot of work.

Using sarcasm here wont work. Why not providing those as patches for m-c so that they get merged and everyone is happy ? Tier-3 platforms break regularly, and their respective maintainers have to deal with it, even if that's sad, that's the way things are.

> You're free to do whatever you want with your own project, of course. But
> don't be surprised if the community loses interest over time. There have
> been other projects like OpenOffice and XFree86 where upstream started to
> ignore the community and we all know where this lead to. Either way, I will
> probably not contribute to Firefox anymore.

Yeah, on powerpc, you're likely to get support from chromi^Wwebkit^W.. oh wait. Yeah, there's netsurf :)

 I stopped caring about firefox on OpenBSD/ppc some years ago to concentrate on OpenBSD/amd64+i386, you decide where you put your time if you want a graphical browser on exotic platforms...

Comment 21

[André Bargull [:anba]]

(In reply to Petr Sumbera from comment #10)

I ran into this issue while building FF for Solaris on sparc. And yes icupkg
fail. It's failing because of 'confusables' record. So following works:

icupkg -tb -r confusables.cfu icudt58l.dat

I've removed the "confusables" record in bug 1569567 and posted two patches to bug 1322212 to add and process big endian ICU data files. I wasn't able to verify the generated data file works correctly (https://phabricator.services.mozilla.com/D40803#1226799), though.

Comment 24

[Dan Horák]

I hit the missing icudt${ver}b.dat recently (after changes from 1635764). Looks to me that the filename is generated correctly in the buildsystem. So my question is what blocks generating the missing file on runtime with "./mach python intl/icu_sources_data.py ." (as done for example for big endian builds in Fedora)?

Comment 25

[André Bargull [:anba]]

The main obstacle is that no one volunteered to write and test a patch. For example I don't have any big endian environments ready for testing, so my patches in bug 1322212 were all only best effort.

And in addition to calling intl/icu_sources_data.py, preferably there also needs to be some way to update the time zone data included in the ICU data file. We're currently using ICU 67, which ships with tzdata 2019c, but the most current tzdata version is 2020a. To update tzdata in ICU, there's the update-tzdata.sh shell script, but that one requires to be called with a compatible icupkg binary. (Some overdue documentation for all this is currently in-progress at bug 1642505.)

Comment 26

[Dan Horák]

If it would help, then I can provide access to a big endian system and naturally test patches. Writing the patch myself would be difficult, because I'm not much familiar with the FF build-system.

Comment 27

[John Paul Adrian Glaubitz]

FWIW, access to big-endian systems (PPC64 and SPARC64) can be obtained through the GCC compile farm, see:

https://gcc.gnu.org/wiki/CompileFarm

The only requirement is that the account is used for the intent of developing open source software.

The machines gcc202 (SPARC64) and gcc203 (PPC64) are co-maintained by me, so I can install packages if required.

Comment 28

[Petr Sumbera]

Seems that it's possible to convert ICU data file without an error now:

icupkg -tb config/external/icu/data/icudt67l.dat config/external/icu/data/icudt67b.dat

But I'm not good with Mozilla build system. How can we execute this line? Any suggestion or example?

Also it would be probably better to build icupkg ourself (intl/icu/source/tools/icupkg). Again I don't really know where to start...

But anyway by executing above command I'm able to build Firefox again.

Comment 29

[Jeff Walden [:Waldo]]

intl/icu/README.md documents all the various relevant commands involved in updating ICU, tzdata, currency information, etc.

I've noted there that we don't have the big-endian version of the file checked in for what are primarily size reasons. Also, those commands currently depend upon a compatible icupkg being provided by the user.

It wouldn't be a crazy nit to fix to build icupkg from the downloaded source and then use that (indeed bug 1449677 would do just that). But it's not really on the radar of the two of us who do most of this work given we can basically just suck it up and do a little extra work.

Comment 30

[Mike Hommey [:glandium]]

Attachment: Bug 1264836 - Automatically convert the little-endian ICU data file for big-endian builds.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3004626988d
Automatically convert the little-endian ICU data file for big-endian builds. r=firefox-build-system-reviewers,rstewart

Comment 32

[Mike Hommey [:glandium]]

Attachment: Additional patch for 78

For people who would want to cherry-pick this on their copy of esr78, this additional patch is necessary (on top of all the dependencies of this bug).

Comment 33

[Narcis Beleuzu [:NarcisB]]

Backed out for breaking the Gecko Decision Task

Backout link: https://hg.mozilla.org/integration/autoland/rev/d803e7067b2cdfbe2d157e5ac91a071d5e75cad8
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310220185&repo=autoland&lineNumber=3740

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cfaeba06ee87
Automatically convert the little-endian ICU data file for big-endian builds. r=firefox-build-system-reviewers,rstewart

Comment 35

[Atila Butkovits]

Backed out for for bustages at AccessibleRole.h.

Backout link: https://hg.mozilla.org/integration/autoland/rev/0c0f777161a9499dd149853ff62d356f75d16c2a

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=cfaeba06ee872fe11a3b3487dc545b24deb8631f&selectedTaskRun=GaYTvBYvTAGV7uGMTXE7Ug.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310232645&repo=autoland&lineNumber=7315

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65418a2fb344
Automatically convert the little-endian ICU data file for big-endian builds. r=firefox-build-system-reviewers,rstewart

Comment 37

[Atila Butkovits]

Apologies for troubles that I caused.
It was relanded: https://hg.mozilla.org/integration/autoland/rev/65418a2fb34497160402d09acadeab78974994d3

Comment 38

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/65418a2fb344