1481633 - lld-link: error: undefined symbol: __start_kPStaticModules in mingw-clang

Status: RESOLVED FIXED

(Firefox Build System :: General: Unsupported Platforms, enhancement, P5)

Description

[Tom Ritter [:tjr]]

It appears that the refactoring in Bug 1471132 does not support a MinGW clang build.

> INFO -  lld-link: error: undefined symbol: __start_kPStaticModules
> INFO -  >>> referenced by ../../xpcom/components/Unified_cpp_xpcom_components0.o:(_Z5beginR16AllStaticModules)
> INFO -  lld-link: error: undefined symbol: __stop_kPStaticModules
> INFO -  >>> referenced by ../../xpcom/components/Unified_cpp_xpcom_components0.o:(_Z3endR16AllStaticModules)

I've confirmed that we use the __ELF__ condition here: https://searchfox.org/mozilla-central/rev/51268dcbdff0f6f4a5cff7986df0f616efc5bcfd/xpcom/components/nsComponentManager.cpp#301

And that we do _not_ add in a linker argument for StaticXULComponents.ld



If I try to add the linker script in (with -T) I get the same error, as seen in this try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d2a4b44408fa23838bdac37ec7b428e332c76c5&selectedJob=192627905

And if I try to use the msvc version in nsComponentManager I get an error about not being able to correctly process the the __declspec attribute: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5afd312c7288de35e5d2225b866dcc35cffde999&selectedJob=192620182

(The issue with __declspec appears to be a MinGW limitation though.)

glandium, Jacek, do you have any thoughts about which approach should be taken here?

Comment 1

[Mike Hommey [:glandium]]

What works with mingw-gcc?

Comment 2

[Jacek Caban]

It's hard to test mingw-gcc, it can't build m-c anymore. Now that we use clang, we should be able to use MSVC codepath in more cases. I think this one should work, but it needs -fms-extensions (bug 1475566). I could get it to build with -fms-extensions (the browser crashed, but I didn't look deeper and it's likely to be unrelated). I will update bug 1475566 with a workaround and attach required changes here.

Comment 3

[Jacek Caban]

Attachment: Proof of concept

With this patch, mingw build uses the same code as MSVC. I had to relax symbol order checks, like for incremental MSVC build. It needs closer look if it's the right thing to do and if yes, replace the hack with proper condition.

Comment 4

[Tom Ritter [:tjr]]

Attachment: Bug 1481633 Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium

clang can handle MSVC-like codepaths generally, so we want to use those
when building with clang for Windows. So we switch _MSC_VER over to _WIN32
to pick up those codepaths when compiling for Windows with clang.

Additionally, we relax the ordering of sections for the same scenario.

Note that we do need to tell clang to use -fms-extensions with the MSVC code,
we do that in the mingw clang build job patch.

Comment 5

[Tom Ritter [:tjr]]

Wanted to flag you on this; this bug and Bug 1489205 are the only things blocking landing the mingw job.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 9001682 [details]
Bug 1481633 Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium

Mike Hommey [:glandium] has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c73d63442c8d
Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c73d63442c8d

Comment 9

[Tom Ritter [:tjr]]

Attachment: Bug 1481633 Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium (esr60)

Comment 10

[Tom Ritter [:tjr]]

Comment on attachment 9035858 [details] [diff] [review]
Bug 1481633 Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium (esr60)

This is one of series of patches I am requesting uplift to esr60. Please don't uplift any if the entire series won't go. The whole series will need to go in one push.

This try run (applied on tip-of-esr60 as of an hour ago; and beginning with 'Bug 1491901 - move MK*SHLIB to moz.configure') represents the patch series. It must be applied in that order: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0659d6e265f3b624ad6fbac0c9cd7ce246094596 (If this try run doesn't complete successfully, I will investigate and figure out why)

The uplift request form is the same for all of the patch series; see https://bugzilla.mozilla.org/show_bug.cgi?id=1491901#c10

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9035858 [details] [diff] [review]
Bug 1481633 Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium (esr60)

approved for 60.5esr

Comment 12

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/750ac3b5b740