Closed Bug 1429549 Opened 6 years ago Closed 6 years ago

Build clang-plugin for the host (not target) bitness on Windows

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1469091

People

(Reporter: away, Unassigned)

References

(Depends on 1 open bug)

Details

Attempting to change win32-st-an builders from the win32-clang-cl toolchain to the win64-clang-cl toolchain breaks with:

18:33:28     INFO -  clang.lib(clang.exe) : fatal error LNK1112: module machine type 'x64' conflicts with target machine type 'x86'

AIUI, clang-plugin.dll will be loaded by the compiler running on the host, so the bitness needs to match that used by the host toolchain.
Blocks: 1414287
Ted, do you have any advice for this?
Flags: needinfo?(ted)
I have neglected this, but we discussed on IRC, and I think the fix here will be to set `CC = $(HOST_CC)` for Windows builds like we do for cross-mac builds:
https://dxr.mozilla.org/mozilla-central/rev/994a8d6eccbcdc6106794705bd77e3ac5f031be2/build/clang-plugin/Makefile.in#13

That should always be OK, because currently all Windows builds use the same compiler for the host and the target, and if that changes (as dmajor is attempting to do), using the host compiler to build the clang plugin is almost certainly the right thing to do.
Flags: needinfo?(ted)
I tried setting CC to HOST_CC, as well as adding ac_add_options --host=x86_64-pc-mingw32 -- but it still fails with the same error: https://treeherder.mozilla.org/#/jobs?repo=try&revision=417a5a6fe3baf14c3349f0b8f6467f575b4dbc9d
Product: Core → Firefox Build System
See Also: → 1450699
OK, so I looked at this briefly today, going from the patches in dmajor's try push from comment 3.  That try push is actually failing during configure, since we're trying to use a 64-bit clang-cl, but wanting to compile for 32-bit.  The try push has a patch to set --host to 64-bit, but we actually want to set --target to 32-bit; --host will get set to 64-bit on its own.

So, we try setting --target=i686-pc-mingw32.  That gets us past configure, into actually building the plugin:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c228ea85cd4d4d9660d7bd9d48adfe1d3cd080f3

which fails, because even though we've set CXX to HOST_CXX, CXX and HOST_CXX accept exactly the same set of options...and we're passing -m32 to force 32-bit target compilation, which, through the wonder that is the clang-plugin build, we are using to build the plugin as well.  So again, we're mixing 32-bit and 64-bit code, which fails to link.

I think the next step is to strip -m32 (which I imagine we can do unconditionally?) on Windows to try and make the plugin compile as 64-bit.  I'm not sure how to do that, though, since -m32 is baked into CXX/HOST_CXX; I guess add -m64 somewhere in *CXXFLAGS and expect that to override the earlier -m32?
Ugh, no, we shouldn't need to strip -m32, because HOST_CXX shouldn't have -m32.  Apparently that try push somehow though --host should be 32-bit; let's try to see if explicitly setting --host and --target does the trick:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3dd222260b613713055d9eecfa669f35980e713
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfbbed4d9df9ba56377301a079ae34b77d41b8ba

Needed to set --host and  --target, *and* set HOST_{CC,CXX}, because automatic compiler detection wasn't working properly.  Still erroring out somewhere in JS configure, which is a regression from actually being able to build the plugin. :(

Not sure what's going wrong here, will investigate more later.
Got a little bit farther; for whatever reason the JS configure wasn't finding the correct AS:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=998ac4f224bbc92dc8e8330924b05992ee9b8c51

The top of the patch stack hacks around this in the mozconfig.  I think the right answer is to export AS from the Gecko configure so the JS configure will find it (like we do for any number of other variables).  The error is now:

21:22:13     INFO -  z:/build/build/src/vs2017_15.6.0/VC/bin/Hostx64/x86/link.exe -NOLOGO -DLL -OUT:clang-plugin.dll -PDB:clang-plugin.pdb -SUBSYSTEM:WINDOWS,6.01 -MACHINE:X86 ThirdPartyPaths.obj Unified_cpp_build_clang-plugin0.obj Unified_cpp_build_clang-plugin1.obj ./module.res -LIBPATH:z:/build/build/src/clang/lib clang.lib clangASTMatchers.lib        kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib secur32.lib
21:22:13     INFO -  ThirdPartyPaths.obj : fatal error LNK1112: module machine type 'x64' conflicts with target machine type 'x86'

And ThirdPartyPaths.obj is definitely *not* being built for 32-bit (hooray!):

21:21:58     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/clang/bin/clang-cl.exe -FoThirdPartyPaths.obj -c -DDEBUG=1 -Iz:/build/build/src/build/clang-plugin -Iz:/build/build/src/obj-firefox/build/clang-plugin -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -Qunused-arguments -Iz:/build/build/src/clang/include -fms-compatibility-version=19.13.26128 -Xclang -std=c++14 -Zc:inline -Zc:strictStrings -Oi -Zc:rvalueCast -Brepro -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wcovered-switch-default -Wdelete-non-virtual-dtor -Wstring-conversion -MT -O2 -Ob2 -DNDEBUG -EHs-c- -GR- -D_DEBUG_POINTER_IMPL= -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -DUNICODE -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DHAVE_NEW_ASTMATCHER_NAMES -DHAS_ACCEPTS_IGNORINGPARENIMPCASTS -GR- -EHsc -Oy-  -Xclang -MP -Xclang -MG -Xclang -dependency-file -Xclang .deps/ThirdPartyPaths.obj.pp -Xclang -MT -Xclang ThirdPartyPaths.obj    z:/build/build/src/obj-firefox/build/clang-plugin/ThirdPartyPaths.cpp

I think the problem now is the linker:

INFO -  z:/build/build/src/vs2017_15.6.0/VC/bin/Hostx64/x86/link.exe

We're using the target linker to link host binaries, which doesn't really work all that well in this case.  And we don't have HOST_LD or HOST_LINKER because we've never really needed it (?)--which sounds a little odd to me.  Do we really have *no* host binaries that we use on Windows?  Or have we been happy to compile those "host" binaries for "target", silently, and not get too worked up about doing so?

dmajor, would using lld magically fix this, since lld doesn't have separate binaries for 32-bit vs. 64-bit linking?  Microsoft doesn't squirrel away a similar linker somewhere in their SDK, do they?
Flags: needinfo?(dmajor)
Microsoft's linkers can target either architecture too. You just need to pass -MACHINE:{X86,X64}.
Flags: needinfo?(dmajor)
OK, forcing -MACHINE:X64 gets me a *little* farther:

15:45:59     INFO -  z:/build/build/src/vs2017_15.6.0/VC/bin/Hostx64/x86/link.exe -NOLOGO -DLL -OUT:clang-plugin.dll -PDB:clang-plugin.pdb -SUBSYSTEM:WINDOWS,6.01 -MACHINE:X86 ThirdPartyPaths.obj Unified_cpp_build_clang-plugin0.obj Unified_cpp_build_clang-plugin1.obj ./module.res -LIBPATH:z:/build/build/src/clang/lib clang.lib clangASTMatchers.lib      -MACHINE:X64  kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib secur32.lib
15:45:59     INFO -  libcpmt.lib(syserror.obj) : fatal error LNK1112: module machine type 'x86' conflicts with target machine type 'x64'
15:45:59     INFO -  z:/build/build/src/config/rules.mk:679: recipe for target 'clang-plugin.dll' failed

libcpmt.lib seems to be the standard C++ library:

https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

It's not clear to me how that's getting linked in.  Is the linker choosing to resolve symbols in everything prior to -MACHINE:X64 against 32-bit libraries?
We should reevaluate this in light of bug 1254953.
Depends on: 1254953
No longer depends on: 1451275
(In reply to Nathan Froyd [:froydnj] from comment #9)
> OK, forcing -MACHINE:X64 gets me a *little* farther:
> 
> 15:45:59     INFO - 
> z:/build/build/src/vs2017_15.6.0/VC/bin/Hostx64/x86/link.exe -NOLOGO -DLL
> -OUT:clang-plugin.dll -PDB:clang-plugin.pdb -SUBSYSTEM:WINDOWS,6.01
> -MACHINE:X86 ThirdPartyPaths.obj Unified_cpp_build_clang-plugin0.obj
> Unified_cpp_build_clang-plugin1.obj ./module.res
> -LIBPATH:z:/build/build/src/clang/lib clang.lib clangASTMatchers.lib     
> -MACHINE:X64  kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib
> advapi32.lib secur32.lib
> 15:45:59     INFO -  libcpmt.lib(syserror.obj) : fatal error LNK1112: module
> machine type 'x86' conflicts with target machine type 'x64'
> 15:45:59     INFO -  z:/build/build/src/config/rules.mk:679: recipe for
> target 'clang-plugin.dll' failed
> 
> libcpmt.lib seems to be the standard C++ library:
> 
> https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx
> 
> It's not clear to me how that's getting linked in.  Is the linker choosing
> to resolve symbols in everything prior to -MACHINE:X64 against 32-bit
> libraries?

(Unclear if this is still the blocking point with all that has changed in five months, but...) I bet your LIB points at a directory containing 32-bit libcpmt.lib.
See Also: → 1494907
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.