Closed Bug 1481633 Opened Last year Closed Last year

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

Categories

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

enhancement

Tracking

(firefox-esr60 fixed, firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- fixed
firefox64 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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?
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(jacek)
What works with mingw-gcc?
Flags: needinfo?(mh+mozilla)
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.
Flags: needinfo?(jacek)
Depends on: 1475566
Attached patch Proof of conceptSplinter Review
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.
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.
Wanted to flag you on this; this bug and Bug 1489205 are the only things blocking landing the mingw job.
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(mh+mozilla)
Comment on attachment 9001682 [details]
Bug 1481633 Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium

Mike Hommey [:glandium] has approved the revision.
Attachment #9001682 - Flags: review+
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c73d63442c8d
Resolve kPStaticModules undefined symbols in MinGW Clang r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c73d63442c8d
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

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

Attachment #9035858 - Flags: approval-mozilla-esr60?

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

approved for 60.5esr

Attachment #9035858 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.