857450 - expandlibs_exec.py loses most contents of ICU libraries

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Norbert Lindenberg]

Attachment: Build desktop Firefox with ICU-based ECMAScript Internationalization API enabled

The attached patch is an adaptation of two patches in bug 724533 and bug 853301 which, according to various comments on bug 724533, should be able to build Firefox on Windows, Mac, and Linux with the ICU-based ECMAScript Internationalization API enabled. Compared to the patches in bug 724533 and bug 853301, the attached patch:
- Does not provide ICU_LIBS when linking XUL.
- Therefore configures only js/src, not the top-level, to build with ICU.
- Makes other simplification within the js/src build made possible by not having to provide ICU_LIBS to the XUL link step. In particular, the build uses $(DEPTH)/intl/icu/lib) rather than $(DIST)/lib) as the final location for the ICU libraries.
- Adds diagnostic output to a few build steps.
The patch can be applied to mozilla-central tip (I used -r e53044984f1b).

The build succeeds on Windows, but fails on Mac and Linux.
https://tbpl.mozilla.org/?tree=Try&rev=8eb8c759c16b

On both Mac and Linux, the build fails when linking XUL because a number of ICU functions cannot be found. See
https://tbpl.mozilla.org/php/getParsedLog.php?id=21370111&tree=Try&full=1 (Mac)
https://tbpl.mozilla.org/php/getParsedLog.php?id=21371342&tree=Try&full=1 (Linux)

The ICU functions are supposed to be found in libjs_static.a. In the step linking that library, the logs show that first a script expandlibs_exec.py is called, and is provided a list of files that includes the three ICU libraries ./intl/icu/lib/libicui18n.a, ./intl/icu/lib/libicuuc.a, and ./intl/icu/lib/libicudata.a. Then ar is called with a list of files that does not include these three libraries, but instead includes the name tmpz0lH7z/icudt50l_dat.o. icudt50l_dat.o is the name of the single object file that went into the creation of libicudata.a. The list does not include any of the contents of libicui18n.a and libicuuc.a.

Using the nm tool on Mac, I can see that libicui18n.a and libicuuc.a contain numerous entries that would satisfy the missing references that made the XUL link step fail, e.g.:

js/src/intl/icu/lib/libicui18n.a(ucal.ao):
0000000000001ad0 T _ucal_getKeywordValuesForLocale_50
000000000001a4f0 S _ucal_getKeywordValuesForLocale_50.eh
js/src/intl/icu/lib/libicui18n.a(udat.ao):
0000000000000170 T _udat_open_50
0000000000020bc8 S _udat_open_50.eh
js/src/intl/icu/lib/libicui18n.a(numsys.ao):
0000000000000d80 T __ZN6icu_5015NumberingSystem7getNameEv
00000000000123c0 S __ZN6icu_5015NumberingSystem7getNameEv.eh
js/src/intl/icu/lib/libicui18n.a(ucol.ao):
0000000000011c80 T _ucol_strcoll_50
0000000000045a18 S _ucol_strcoll_50.eh

js/src/intl/icu/lib/libicuuc.a(uenum.ao):
0000000000000000 T _uenum_close_50
0000000000002668 S _uenum_close_50.eh

Apparently the expandlibs_exec.py omits these when munging the input file list for ar.

Comment 1

[Mike Hommey [:glandium]]

(In reply to Norbert Lindenberg from comment #0)
> js/src/intl/icu/lib/libicui18n.a(ucal.ao):
> js/src/intl/icu/lib/libicui18n.a(udat.ao):
> js/src/intl/icu/lib/libicui18n.a(numsys.ao):
> js/src/intl/icu/lib/libicui18n.a(ucol.ao):
> js/src/intl/icu/lib/libicuuc.a(uenum.ao):
> 
> Apparently the expandlibs_exec.py omits these when munging the input file
> list for ar.

... because the files contained in these libs are *.ao instead of *.o.

Comment 2

[Mike Hommey [:glandium]]

... which unveils a *big* problem with bug 724533: the static libs are built entirely as static, without -fPIC. And that's really really bad.

Comment 3

[Norbert Lindenberg]

I'm clearly out of my depth here, but would like to understand:

> ... because the files contained in these libs are *.ao instead of *.o.

The file suffix doesn't seem to stop the linker from building functional binaries using the contents of these files, so why should expandlibs_exec.py throw them away, without any warning?

> ... which unveils a *big* problem with bug 724533: the static libs are built
> entirely as static, without -fPIC. And that's really really bad.

As far as I could find out, -fPIC is a gcc option, and it's the default for Darwin and Mac OS X according to the gcc man page. clang doesn't have the option at all, according to its man page. So what's amiss here, what's the actual damage, and what should I do about it?

Comment 4

[Mike Hommey [:glandium]]

(In reply to Norbert Lindenberg from comment #3)
> I'm clearly out of my depth here, but would like to understand:
> 
> > ... because the files contained in these libs are *.ao instead of *.o.
> 
> The file suffix doesn't seem to stop the linker from building functional
> binaries using the contents of these files, so why should expandlibs_exec.py
> throw them away, without any warning?

Because expandlibs assumes it only has to handle OBJ_SUFFIX.

> > ... which unveils a *big* problem with bug 724533: the static libs are built
> > entirely as static, without -fPIC. And that's really really bad.
> 
> As far as I could find out, -fPIC is a gcc option, and it's the default for
> Darwin and Mac OS X according to the gcc man page. clang doesn't have the
> option at all, according to its man page. So what's amiss here, what's the
> actual damage, and what should I do about it?

What this does on linux is that it creates binaries with relocations in the code, and we don't want that. What you need is to make ICU create a static library of the objects it would create for a shared library. The easiest is probably to make it add what we define in DSO_PIC_CFLAGS in its compile flags (CFLAGS="$DSO_PIC_CFLAGS" CXXFLAGS="$DSO_PIC_CFLAGS" intl/icu/configure from the top level configure should do), and to override STATIC_O when invoking make. ($(MAKE) -C intl/icu STATIC_O=$(OBJ_SUFFIX))

Comment 5

[Mike Hommey [:glandium]]

Anyways, I'm not going to change expandlibs here.

Comment 6

[Norbert Lindenberg]

Thanks for the clarifications and advice in comment 4.