1299159 - Stop exporting _NSModule modules from xul.dll

Status: RESOLVED DUPLICATE of bug 1541792

(Firefox Build System :: General, defect)

Description

[Benjamin Smedberg]

Currently xul.dll exports a bunch of symbols like this:

struct mozilla::Module const * const Apprunner_NSModule
struct mozilla::Module const * const Browser_Embedding_Module_NSModule

I'm pretty sure that we don't need to export these modules. I'd like to try not doing it and see what happens.

Comment 1

[Mike Hommey [:glandium]]

See bug 938437 comment 5 why the symbols are exported. The good news is it's not a problem for Windows. And things may have changed on other platforms.

Comment 2

[Benjamin Smedberg]

Attachment: Bug 1299159 - Stop exporting *_NSModule from xul.dll,

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

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8786838 [details]
Bug 1299159 - Stop exporting *_NSModule from xul.dll,

https://reviewboard.mozilla.org/r/75726/#review73756

If you're not going to do it in this bug, please file a followup to at least try to do the same on other platforms.

::: toolkit/library/libxul.mk:56
(Diff revision 1)
>  endif
>  endif
>  endif
>  
>  ifdef _MSC_VER
> -get_first_and_last = dumpbin -exports $1 | grep _NSModule@@ | sort -k 3 | sed -n 's/^.*?\([^@]*\)@@.*$$/\1/;1p;$$p'
> +get_first_and_last = dumpbin -section:rodata -map $1 | grep _NSModule@@ | sort -k 2 | sed -n 's/^.*?\([^@]*\)@@.*$$/\1/;1p;$$p'

It's funny how -map is not documented on MSDN. Interestingly, -section:rodata doesn't make much difference. Also interestingly, this requires the .pdb file to work, and when the .pdb file is not there, the output format is different (and doesn't include symbols). I hope nobody is building without debugging info.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58b07057fe25
Stop exporting *_NSModule from xul.dll, r=glandium

Comment 5

[Phil Ringnalda (:philor)]

First guess on what happens is that it breaks Windows PGO builds in startup cache precompilation (https://treeherder.mozilla.org/logviewer.html#?job_id=35164615&repo=mozilla-inbound, not that there's ever anything in the log from that bustage).

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/9b827786ca68a923bacef503c9a52369381b3170 to see whether I'm right that it was this one.

Comment 6

[Mike Hommey [:glandium]]

From a local PGO build:

$ dumpbin -map obj-i686-pc-mingw32/toolkit/library/xul.dll |grep _NSModule | sort -k2

Dump of file obj-i686-pc-mingw32/toolkit/library/xul.dll
  0002:00002AB4 05847AB4 40302040          4  ?start_kPStaticModules_NSModule@@3QBUModule@mozilla@@B (struct mozilla::Module const * const start_kPStaticModules_NSModule)
  0002:00002AB8 05847AB8 40302040          4  ?end_kPStaticModules_NSModule@@3QBUModule@mozilla@@B (struct mozilla::Module const * const end_kPStaticModules_NSModule)

So the LOCAL_CHECK still passes because the first and last are still start and end, but... they are adjacent and there are no other NSModules in there, so no component registration, so no component initialization, and kaboom. Thanks PGO.

Comment 7

[Benjamin Smedberg]

Ugh. Are these symbols being excluded by opt:ref (removing unreferenced symbols)?

Other than explicitly referencing symbols in toolkit/library (which takes us almost back to nsDllDeps.cpp) do you have other tips how I might achieve this?

Comment 8

[Mike Hommey [:glandium]]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #7)
> Ugh. Are these symbols being excluded by opt:ref (removing unreferenced
> symbols)?

We're passing -OPT:REF on both normal and PGO builds. I'd expect it to have the same outcome in both cases if it was responsible for it. But yeah, it looks a bit like it.

> Other than explicitly referencing symbols in toolkit/library (which takes us
> almost back to nsDllDeps.cpp) do you have other tips how I might achieve
> this?

Unfortunately, it doesn't seem there is an easy way out:
https://connect.microsoft.com/VisualStudio/Feedback/Details/1587892
https://connect.microsoft.com/VisualStudio/feedback/details/1314631/c-lost-global-variables-with-wpo-ltcg

It would be sad to go back to the old days of nsStaticXULComponents.cpp.

Does it hurt to leave them exported? Not all XPCOM components are equal, and I suspect that if we need to hide some, we can probably get away with not hiding most. In that case, maybe we can look into deCOMtaminating them? Or to manually register them first in nsComponentManagerImpl::InitializeStaticModules? Better to have a small list of hardcoded central components that are always there than to have a list of abount a hundred components with plenty of #ifdefs like we used to have.

Comment 9

[Masatoshi Kimura [:emk]]

Will be fixed by bug 1541792.