1490568 - MinGW Clang x86 build fails with error: invalid variant '16'

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

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

See also Bug 1470993

Failing build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3c8caa3d0e0b79f2e68dad717b438d4ec06c0bf&selectedJob=198793599

> /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/i686-w64-mingw32-clang++ -mwindows -o xptcstubs.o -c  -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/win32 -I/builds/worker/workspace/build/src/obj-firefox/xpcom/reflect/xptcall/md/win32 -I/builds/worker/workspace/build/src/xpcom/reflect/xptcall -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -Qunused-arguments <<-W options omitted>> -fno-sized-deallocation -fms-extensions -fno-exceptions -fno-strict-aliasing -mms-bitfields -mstackrealign -fno-rtti -ffunction-sections -fdata-sections -Wa,-mbig-obj -fno-exceptions -fno-math-errno -pipe -g -fno-omit-frame-pointer  -MD -MP -MF .deps/xptcstubs.o.pp   /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/win32/xptcstubs.cpp
> <inline asm>:14:33: error: invalid variant '16'
>         call       _PrepareAndDispatch@16


Jacek this is definitely one I will need your help on...

Comment 1

[Nathan Froyd [:froydnj]]

I think this is a difference in mangling between win32 and mingw32 (compare the !GNUC case in that file, above), and this could possibly be a legitimate bug in clang's asm parser.

Comment 2

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

Jacek suggested removing the @16 and that eems to have fixed it; although we won't know for sure until we get a successull link

Comment 3

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

Attachment: Bug 1490568 Remove the @16 from _PrepareAndDispatch in reflect asm to fix MinGW build r=Alex_Gaynor

The error is:

> <inline asm>:14:33: error: invalid variant '16'
>         call       _PrepareAndDispatch@16

The linker should be able to fix it up itself.

Comment 4

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

Got to the linking step: https://treeherder.mozilla.org/#/jobs?repo=try&revision=16dc1afd48571cbcb5c19756c8a087536756f440

> [task 2018-09-13T17:46:30.712Z] 17:46:30     INFO -  lld-link: error: undefined symbol: __ZN14nsXPTCStubBase5Stub3Ev@4
> [task 2018-09-13T17:46:30.712Z] 17:46:30     INFO -  >>> referenced by ../../xpcom/reflect/xptcall/xptcall.o:(__ZTV14nsXPTCStubBase)
> [task 2018-09-13T17:46:30.712Z] 17:46:30     INFO -  lld-link: error: undefined symbol: __ZN14nsXPTCStubBase5Stub4Ev@4
> [task 2018-09-13T17:46:30.712Z] 17:46:30     INFO -  >>> referenced by ../../xpcom/reflect/xptcall/xptcall.o:(__ZTV14nsXPTCStubBase)
> etc

Comment 5

[Jacek Caban]

Attachment: scl_fix.diff

It seems similar to bug 1470993, the attached patch should help.

Comment 6

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

It did: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4823e97cbec78f153e4e312f6b283cb84d8c1056&selectedJob=199149334

There are a bunch of unrelated symbol errors for me to track down, but the most relevant one for this bug is:

> [task 2018-09-13T18:58:00.164Z] 18:58:00     INFO -  lld-link: error: undefined symbol: _PrepareAndDispatch
> [task 2018-09-13T18:58:00.164Z] 18:58:00     INFO -  >>> referenced by ../../xpcom/reflect/xptcall/md/win32/xptcstubs.o:(SharedStub)

Comment 7

[Jacek Caban]

I didn't look at Mozilla source code side of linking problems yet, but missing DhcpRequestParams and _fstat should be fixed in mingw-w64 now.

Comment 8

[Jacek Caban]

Attachment: hacks.diff

I looked at this and _PrepareAndDispatch was easy to fix by quitting the symbol.

I was able to get it linking (although I have a crash on startup). There were two more problems:

- GetFrameLoader could inside child objects vtbls could not be resolved by linker. It looks like a clang bug, but requires closer look. I worked around it by having explicit forwards.

- libyuv is confused. It some pretty stupid #ifdefs, making a lot of assumptions it shouldn't do. It hurts our clang build, because it did assumptions that we're clang-cl or clang GCC randomly. Now that we use -fms-extensions, we can actually use any version of assemblies - both MSVC and GCC variants should work, but we  need libyuv to be consequent. I hacked it to use MSVC versions consequently. I'm tempted to try using -fms-compatibility and see if we can get it working this way.

Comment 9

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

Attachment: Bug 1490568 Quote the _PrepareAndDispatch symbol to fix the mingw-clang x86 build r=dmajor

Comment 10

[(Away)]

Comment on attachment 9009637 [details]
Bug 1490568 Quote the _PrepareAndDispatch symbol to fix the mingw-clang x86 build r=dmajor

David Major [:dmajor] has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8719cf957ca5
Quote the _PrepareAndDispatch symbol to fix the mingw-clang x86 build r=dmajor

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8719cf957ca5

Comment 13

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

Comment on attachment 9009637 [details]
Bug 1490568 Quote the _PrepareAndDispatch symbol to fix the mingw-clang x86 build r=dmajor

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed to fix the mingw-clang build for esr60

User impact if declined: Tor will have to carry this patch themselves, and we won't be able to run mingw-clang in automation.

Fix Landed on Version: 64.0a1 / 20180918075510

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): We're adding quotes to a symbol. This hasn't impacted anything for the 1.5 months it's been in Nightly/Beta.

String or UUID changes made by this patch:

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9009637 [details]
Bug 1490568 Quote the _PrepareAndDispatch symbol to fix the mingw-clang x86 build r=dmajor

Better support for build automation, let's uplift.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/5e6ad4f72b5d