1457295 - lld does not correctly order NSModules

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

We require NSModules to be ordered correctly:
https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/toolkit/library/libxul.mk#24

lld does not order them correctly, even when the .list of objects is ordered correctly.

Options:

a) There may be something possible with a hack like https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/toolkit/library/StaticXULComponentsEnd/StaticXULComponentsEnd.cpp#6 and using --sort-section=name but experimenting with --sort-section=name didn't seem to produce expected output. Nothing was sorted by anything I could understand (sample: https://pastebin.mozilla.org/9084004).



b) I _can_ pass -Wl,--symbol-ordering-file <file> where <file> is a sorted list of symbols. This does work. The ugly part is I have to generate that file, probably with something like 
> sed '...' libxul.so.list' | nm -g | grep NSModule | grep -v (Start|End) | sort 



c) (Ted's idea) We can do away with this whole .kPStaticModules hack entirely. The whole point of this is to build up an array of modules: https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/xpcom/components/nsComponentManager.cpp#260

If we instead added something like the following to moz.build files:

> XPCOM_STATIC_MODULES += ['<module_variable_name>']

we could generate a source file and make the array that way.  

We can add something in the build system that adds a special define so that NSMODULE_DEFN won't compile without it to avoid someone omitting a XPCOM_STATIC_MODULES line.

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Heh I forgot about Option 4: Just reuse the MSVC hack, which (which when appropriatly converted for clang) orders things correctly: https://pastebin.mozilla.org/9084017

I'm going to attach that to this bug once I clean it up tomorrow.

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

At time of writing, MOZ_LTO is not actually created or defined anywhere yet.

Review commit: https://reviewboard.mozilla.org/r/240356/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240356/

Comment 3

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8971579 [details]
Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240356/diff/1-2/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8971579 [details]
Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

https://reviewboard.mozilla.org/r/240356/#review246614

::: toolkit/library/StaticXULComponentsEnd/StaticXULComponentsEnd.cpp:12
(Diff revision 2)
>   * sections with the same first part are all grouped, and ordered
>   * alphabetically with the second part as sort key. */
>  #  pragma section(".kPStaticModules$Z", read)
>  #  undef NSMODULE_SECTION
>  #  define NSMODULE_SECTION __declspec(allocate(".kPStaticModules$Z"), dllexport)
> +#elif defined(XP_LINUX) && MOZ_LTO

There is no reason to make this linux-only.

Comment 5

[Kim Moir [:kmoir] ET]

assigning to tjr since he attached patches

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8971579 [details]
Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240356/diff/2-3/

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8971579 [details]
Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

https://reviewboard.mozilla.org/r/240356/#review246842

::: toolkit/library/StaticXULComponentsEnd/StaticXULComponentsEnd.cpp:13
(Diff revision 3)
> +/* Clang+lld with LTO does not order modules correctly either, but fortunately
> + * the same trick works. */

I remembered about the hack in toolkit/library/StaticXULComponentsEnd/moz.build. Is StaticXULComponentsEnd built with -fno-lto when that happens?

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8971579 [details]
Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

https://reviewboard.mozilla.org/r/240356/#review250272

::: toolkit/library/StaticXULComponentsEnd/StaticXULComponentsEnd.cpp:13
(Diff revision 3)
> +/* Clang+lld with LTO does not order modules correctly either, but fortunately
> + * the same trick works. */

Yes; even when we compile StaticXULComponentsEnd with -fno-lto stuff doesn't get ordered correctly.

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8971579 [details]
Bug 1457295 Trick clang+lld+lto into ordering NSModules correctly also

https://reviewboard.mozilla.org/r/240356/#review250520

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/813d8cffc9c9
Trick clang+lld+lto into ordering NSModules correctly also r=glandium

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/813d8cffc9c9