Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#13419 closed task (fixed)

Fix cross-compiling ICU with mingw-w64 for Windows builds

Reported by: gk Owned by: gk
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-gitian, GeorgKoppen201604, TorBrowserTeam201604R, ff45-esr, tbb-6.0a5
Cc: arthuredelstein, boklm, brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Cross-compiling ICU with mingw-w64 is currently broken which is why we disable it. See: https://bugzilla.mozilla.org/show_bug.cgi?id=1019744#c19 for some details. Jacek does not plan to work on that one he told me.

Child Tickets

Attachments (1)

0001-Bug-13419-Fix-ICU-cross-compilation-for-Windows.patch (2.6 KB) - added by gk 3 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 4 years ago by gk

Keywords: TorBrowserTeam201509 GeorgKoppen201509 added
Owner: changed from tbb-team to gk
Status: newassigned

comment:2 Changed 4 years ago by arthuredelstein

Cc: arthuredelstein added

comment:3 Changed 4 years ago by boklm

Cc: boklm added

comment:4 Changed 4 years ago by mcs

Cc: brade mcs added

comment:5 Changed 4 years ago by mcs

Since adding a simple C++ implementation of the Intl API seems like a big task (please discuss that idea in #16874), I spent a little time trying to learn about and fix the cross-compile problems that are the subject of this ticket. I think I made a little progress but am out of time for today. Probably Georg got further than I did in the past, but I might as well record here what I learned.

Our build is using intl/icu/source/config/mh-mingw, which includes this comment:

# TODO: Finish the rest of this port. This platform port is incomplete.

(definitely a bad sign).

In general, there seems to be a mixup in the DLL and import library names. Strange suffixes are certainly used, e.g., .dll.a. I added a couple of config lines to intl/icu/source/config/mh-mingw, which seemed to help:

LIBICUDT=$(top_builddir)/stubdata/$(LIBICU)$(DATA_STUBNAME)$(ICULIBSUFFIX)$(IMPORT_LIB_EXT)
LIBICUUC=$(LIBDIR)/$(LIBICU)$(COMMON_STUBNAME)$(ICULIBSUFFIX)$(IMPORT_LIB_EXT) $(LIBICUDT)

But now I get the following error:

/home/ubuntu/install/mingw-w64/lib/gcc/i686-w64-mingw32/5.1.0/../../../../i686-w64-mingw32/lib/../lib/libssp.a(ssp.o):ssp.c:(.text+0x239): multiple definition of `__stack_chk_fail'
../lib/icuuc.dll.a(d001684.o):(.text+0x0): first defined here
collect2: error: ld returned 1 exit status
make[7]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/intl/icu/target/i18n'
make[7]: *** [../lib/icuin52.dll] Error 1
make[6]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/intl/icu/target'
make[6]: *** [all-recursive] Error 2
make[5]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/config/external/icu'
make[5]: *** [buildicu] Error 2
make[4]: *** [config/external/icu/target] Error 2

comment:6 in reply to:  5 ; Changed 4 years ago by gk

Replying to mcs:

Since adding a simple C++ implementation of the Intl API seems like a big task (please discuss that idea in #16874), I spent a little time trying to learn about and fix the cross-compile problems that are the subject of this ticket. I think I made a little progress but am out of time for today. Probably Georg got further than I did in the past, but I might as well record here what I learned.

Our build is using intl/icu/source/config/mh-mingw, which includes this comment:

# TODO: Finish the rest of this port. This platform port is incomplete.

(definitely a bad sign).

In general, there seems to be a mixup in the DLL and import library names. Strange suffixes are certainly used, e.g., .dll.a. I added a couple of config lines to intl/icu/source/config/mh-mingw, which seemed to help:

LIBICUDT=$(top_builddir)/stubdata/$(LIBICU)$(DATA_STUBNAME)$(ICULIBSUFFIX)$(IMPORT_LIB_EXT)
LIBICUUC=$(LIBDIR)/$(LIBICU)$(COMMON_STUBNAME)$(ICULIBSUFFIX)$(IMPORT_LIB_EXT) $(LIBICUDT)

There are interesting things going on here. First I tried to tackle this from a different angle: I cross-compiled the ICU lib alone which is surprisingly working out of the box (at least the compilation does not break) and tried to determine what is wrong with Mozilla's one. I took the same version (52.1) and used the same toolchain but I am still puzzled.

I then applied the lines above to a plain ESR 38 build and used the toolchain I built for it. Somehow I am getting quite far. I am probably missing the libssp bits (have not checked yet) but still the build is failing close to the end while linking libxul:

    INPUT("../../modules/zlib/src/zutil.o")
    INPUT("StaticXULComponentsEnd/StaticXULComponentsEnd.o")

i686-w64-mingw32-g++: error: ../../intl/icu/target/lib/libicuin.a: Datei oder Verzeichnis nicht gefunden
i686-w64-mingw32-g++: error: ../../intl/icu/target/lib/libicuuc.a: Datei oder Verzeichnis nicht gefunden
i686-w64-mingw32-g++: error: ../../intl/icu/target/lib/libicudt.a: Datei oder Verzeichnis nicht gefunden
make[5]: *** [xul.dll] Fehler 1

Then I applied the two lines to a gitian build and this failed quite early indicating that both lines are not enough. I am fixing that right now I hope.

But now I get the following error:

/home/ubuntu/install/mingw-w64/lib/gcc/i686-w64-mingw32/5.1.0/../../../../i686-w64-mingw32/lib/../lib/libssp.a(ssp.o):ssp.c:(.text+0x239): multiple definition of `__stack_chk_fail'
../lib/icuuc.dll.a(d001684.o):(.text+0x0): first defined here
collect2: error: ld returned 1 exit status
make[7]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/intl/icu/target/i18n'
make[7]: *** [../lib/icuin52.dll] Error 1
make[6]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/intl/icu/target'
make[6]: *** [all-recursive] Error 2
make[5]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/config/external/icu'
make[5]: *** [buildicu] Error 2
make[4]: *** [config/external/icu/target] Error 2

Do you have a bit more context? When does this happen?

comment:7 in reply to:  6 Changed 4 years ago by mcs

Replying to gk:

Do you have a bit more context? When does this happen?

It happens while building under /home/ubuntu/build/tor-browser/obj-mingw/intl/icu/target/i18n/, while trying to link ../lib/icuin52.dll. I ssh'd into the VM and grabbed the full command:

i686-w64-mingw32-g++ -mwindows -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Wno-invalid-offsetof -Wcast-align -Wno-format -fno-exceptions -fno-strict-aliasing -mms-bitfields -mstackrealign -fno-keep-inline-dllexport -frtti -fno-exceptions -fno-math-errno -std=gnu++0x -pipe -g -UDEBUG -DNDEBUG -O -W -Wall -pedantic -Wpointer-arith -Wwrite-strings -Wno-long-long -Wno-unused -Wno-unused-parameter -mthreads   -specs=/home/ubuntu/build/msvcr100.spec -static -Wl,--enable-stdcall-fixup -Wl,--large-address-aware   -shared -Wl,-Bsymbolic -Wl,--enable-auto-import -Wl,--out-implib=../lib/icuin.dll.a  -o ../lib/icuin52.dll ucln_in.o fmtable.o format.o msgfmt.o umsg.o numfmt.o unum.o decimfmt.o dcfmtsym.o ucurr.o digitlst.o fmtable_cnv.o choicfmt.o datefmt.o smpdtfmt.o reldtfmt.o dtfmtsym.o udat.o dtptngen.o udatpg.o nfrs.o nfrule.o nfsubs.o rbnf.o numsys.o unumsys.o ucsdet.o ucal.o calendar.o gregocal.o timezone.o simpletz.o olsontz.o astro.o taiwncal.o buddhcal.o persncal.o islamcal.o japancal.o gregoimp.o hebrwcal.o indiancal.o chnsecal.o cecal.o coptccal.o dangical.o ethpccal.o coleitr.o coll.o tblcoll.o sortkey.o bocsu.o ucoleitr.o ucol.o ucol_res.o ucol_bld.o ucol_sit.o ucol_tok.o ucol_wgt.o ucol_cnt.o ucol_elm.o strmatch.o usearch.o search.o stsearch.o translit.o utrans.o esctrn.o unesctrn.o funcrepl.o strrepl.o tridpars.o cpdtrans.o rbt.o rbt_data.o rbt_pars.o rbt_rule.o rbt_set.o nultrans.o remtrans.o casetrn.o titletrn.o tolowtrn.o toupptrn.o anytrans.o name2uni.o uni2name.o nortrans.o quant.o transreg.o brktrans.o regexcmp.o rematch.o repattrn.o regexst.o regextxt.o regeximp.o uregex.o uregexc.o ulocdata.o measfmt.o currfmt.o curramt.o currunit.o measure.o utmscale.o csdetect.o csmatch.o csr2022.o csrecog.o csrmbcs.o csrsbcs.o csrucode.o csrutf8.o inputext.o wintzimpl.o windtfmt.o winnmfmt.o basictz.o dtrule.o rbtz.o tzrule.o tztrans.o vtzone.o zonemeta.o upluralrules.o plurrule.o plurfmt.o selfmt.o dtitvfmt.o dtitvinf.o udateintervalformat.o tmunit.o tmutamt.o tmutfmt.o currpinf.o uspoof.o uspoof_impl.o uspoof_build.o uspoof_conf.o uspoof_wsconf.o decfmtst.o smpdtfst.o ztrans.o zrule.o vzone.o fphdlimp.o fpositer.o locdspnm.o decNumber.o decContext.o alphaindex.o tznames.o tznames_impl.o tzgnames.o tzfmt.o compactdecimalformat.o gender.o region.o scriptset.o identifier_info.o uregion.o ../lib/icuuc.dll.a ../stubdata/icudt.dll.a -lm

After a little more poking around this morning, I think the above command should be linking with the DLL (../lib/icuuc52.dll) instead of the static lib (../lib/icuuc.dll.a). I am going to make that change to the lines I suggested adding in comment:5 and see what happens. My revised additions to intl/icu/source/config/mh-mingw are:

# Use correct names for import libraries.
# See https://trac.torproject.org/projects/tor/ticket/13419
LIBICUDT= $(top_builddir)/stubdata/$(LIBICU)$(DATA_STUBNAME)$(ICULIBSUFFIX)$(IMPORT_LIB_EXT)
LIBICUUC= $(LIBDIR)/$(LIBICU)$(COMMON_STUBNAME)$(ICULIBSUFFIX)$(SO_TARGET_VERSION_MAJOR).$(SO) $(LIBICUDT)

comment:8 Changed 4 years ago by mcs

And now I get the same error as gk did (when linking xul.dll):

i686-w64-mingw32-g++: error: ../../intl/icu/target/lib/libicuin.a: No such file or directory
i686-w64-mingw32-g++: error: ../../intl/icu/target/lib/libicuuc.a: No such file or directory
i686-w64-mingw32-g++: error: ../../intl/icu/target/lib/libicudt.a: No such file or directory

The contents of ../../intl/icu/target/lib are:

icudt52.dll icuin52.dll icuin.dll.a icuuc52.dll icuuc.dll.a

I am out of time for now, but I guess things are still messed up with respect to import library names, DLL names, etc.

comment:9 Changed 4 years ago by gk

Keywords: TorBrowserTeam201510 added; TorBrowserTeam201509 removed

Moving Tor Browser tickets to October 2015.

comment:10 Changed 4 years ago by gk

Keywords: GeorgKoppen201510 added; GeorgKoppen201509 removed

Moving my tickets to October 2015

comment:11 Changed 4 years ago by gk

Keywords: TorBrowserTeam201511 added; TorBrowserTeam201510 removed

comment:12 Changed 4 years ago by gk

Keywords: GeorgKoppen201511 added; GeorgKoppen201510 removed

comment:13 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201512 added; TorBrowserTeam201511 removed

comment:14 Changed 3 years ago by gk

Keywords: TorBrowserTeam201604 ff45-esr tbb-6.0a5 added; GeorgKoppen201511 TorBrowserTeam201512 removed
Priority: MediumHigh
Severity: Major

#18767 shows that the UI of Firefox itself is partly broken now without ICU support. Raising the prio and putting it on the 6.0a5 radar.

comment:15 Changed 3 years ago by gk

Keywords: GeorgKoppen201604 added

comment:16 Changed 3 years ago by gk

Mozilla seems to go own ways starting with Firefox 48: https://bugzilla.mozilla.org/show_bug.cgi?id=1239083 I wonder whether we'd need to fix the cross-compilation setup twice now.

Last edited 3 years ago by gk (previous) (diff)

comment:17 in reply to:  6 Changed 3 years ago by gk

Status: assignedneeds_review

Replying to gk:

There are interesting things going on here. First I tried to tackle this from a different angle: I cross-compiled the ICU lib alone which is surprisingly working out of the box (at least the compilation does not break) and tried to determine what is wrong with Mozilla's one. I took the same version (52.1) and used the same toolchain but I am still puzzled.

I thought it might be a good idea to start from here again trying to find out what the issue is. It turns out. ICU does not like the -static LDFLAG if it is configured with --disable-static --enable-shared as it is in a default setup. We have the former requirement due to bug 1116777. The patch in icu.m4 takes care of this.

The next issue was that all libraries have the Windows static library suffix .lib while .a is expected. This is addressed in the Makefile.in patch.

The final problem was that not all libraries are generated with the proper prefix lib. Changing that in the ICU mingw config and the Makefile.in file solves this.

This patch is only meant to get Tor Browser based on ESR45 into shape. Creating upstreamable ones (both to ICU and Mozilla) will be separate tickets. In fact, as I mentioned in my previous comment I guess we'd need a heavily modified one anyway.

Testing a resulting build solves both this bug, #16874 and #18767 without noticeable downsides for me.

Please review the attached patch.

Last edited 3 years ago by gk (previous) (diff)

comment:18 Changed 3 years ago by gk

Keywords: TorBrowserTeam201604R added; TorBrowserTeam201604 removed

comment:19 Changed 3 years ago by mcs

r=mcs
Good work! The changes look OK and your explanations make sense, although I did not try to build with them.

comment:20 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

This is fixed with commit 76b0760ee1315c1d726673d99ecd3f277671132b on tor-browser-45.0.2esr-6.x-1. The upstreaming ticket is #18847.

comment:21 Changed 3 years ago by bugzilla

Mozilla has started to go the right direction of integration its components with the mozbuild system. Unfortunately, they aren't going to do this for non tier 1 platforms. But for ICU they've done, as they stated, separate mozbuild "path" which is suitable for all platforms and cross-compilation. It can be easily back-ported.

comment:22 Changed 3 years ago by msdobrescu

Hi, I came across this by searching to fix a SpiderMonkey build under Windows with MinGW64 using mozilla build tools.
It still doesn't build as it creates libs like 'libX.dll.a'. I've managed to find where to name them 'libX.a', but still 'libX56.a' is needed, where 56 is the major version of ICU.
IMHO, it is not yet fixed in this case.

Note: See TracTickets for help on using tickets.