Warn on .lib files with the wrong arch

[nico]

Bugzilla Link	38965
Version	unspecified
OS	All
Depends On	#38130
Blocks	#38315

Extended Description

clang-cl defaults to 64-bit builds. If you accidentally have your env set up to point to the 32-bit .lib paths, you get these cryptic diags:

$ ./clang-cl.sh foo.cc -fuse-ld=lld
lld-link: error: : undefined symbol: mainCRTStartup
$ ./clang-cl.sh -m32 foo.cc -fuse-ld=lld # works

link.exe is much better, it has this warning telling you what the actual issue is:

C:\Program Files (x86)\Windows Kits\10\lib\10.0.17134.0\um\x64\advapi32.lib : warning LNK4272: library machine type 'x64' conflicts with target machine type 'x86'

(copied from a different bug; it probably complains about a different .lib for a trivial foo.cc with just main() -- but the warning is the same otherwise)

lld-link should warn on this too.

Hans says: """
.lib files themselves don't have a machine type.

However, the import lib pseudo-files inside them do. lld currently
doesn't check the machine type of those (it would if ImportFile
overrode getMachineType; it probably should).

But, lld will only look at the members of a .lib file lazily, i.e. if
it seems something that it needs in the .lib file's symbol table.
Because of this, I'm not sure that implementing
ImportFile::getMachineType is enough to produce a warning in Bruce's
case. Probably no symbols from advapi32.lib's symbol table are being
referenced, and so we would never parse the .lib file members :-/
"""

Zach says: """
It's pretty rare for a .lib file to have mixed arch object files. You would have to go out of your way to make it happen. Given that, can we just open the first object file, parse just enough of it to get the object file, and early-warn if it mismatches on the assumption that all other objects in the archive will have the same architecture?
"""

We should find some way to make this work.

[nico]

This isn't a problem for C++ symbols, since they "usually" have the same mangling in 32-bit and 64-bit. So there you get a nice error already:

$ cat test.cc
void f() {}
$ cat main.cc
void f();
int main() { f(); }
$ ./clang-cl.sh -m32 /cc test.cc
$ out/gn/bin/llvm-lib /out:test.lib test.obj
$ ./clang-cl.sh main.cc test.lib -fuse-ld=lld
lld-link: error: test.lib(test.obj): machine type x86 conflicts with x64

This is true for normal C++ functions, but not e.g for __stdcall annotated functions, which get mangled as __cdecl in 64-bit. But the main use of __stdcall is for Windows SDK functions, which are C functions, so I'm not sure how important that is.

So another approach would be to do:

• If we're about to emit an undefined symbol error:
  -- Check if the symbol is a C symbol (doesn't start with '?')
  --- If so, do a look up for the same symbol with _ prepended if there's no leading _, or do one with _ removed if a _ is there
  ---- If that succeeds, either diag "wrong bitness for lib" directly, or load the .obj file that defines it and check that bitness is wrong and warn then

This is a bit of a heuristic that would be incorrect e.g. for function names that start with an underscore, and the CRT has many functions starting with an underscore (_free_locale, __iscsym, _isatty, _fwrite_nolock, etc etc etc). But as long as at least one of the referenced functions doesn't start with an underscore, we'd still emit a decent diag, and mainCRTStartup doesn't have a leading underscore, so this approach would fix the main motivation for this bug.

It has the advantage of putting the actionable bit at the error diagnostic instead of adding a warning, and it only requires work in the error path, while "read first obj file in lib" always requires (minor) work.

STDC (/Za)

[nico]

Yet another idea would be to do a lookup for WinMainCRTStartup / wWinMainCRTStartup / mainCRTStartup / wmainCRTStartup (and I suppose _DllMainCRTStartup for dlls). Driver.cpp already special-cases those, and it would also fix the main motivation here (linking for the wrong bitness).

This would allow us to emit the nicest diagnostic of the approaches so far. It wouldn't fire with /noentry or custom /entry: flags, but that's maybe not a common use case.

Or we could do this approach as a first line of defense to get the nice diagnostic in the common case, and then do more complicated things later.

[nico]

Here's the original report for this. The third suggested approach wouldn't work for it.

d:\src\chromium\src>ninja -v -C out\debug_component embedder_switches.dll
ninja.exe -v -C out\debug_component embedder_switches.dll -j 640 -l 32
ninja: Entering directory `out\debug_component'
[1 processes, 1/1 @ 4.3/s : 0.231s ] d:/src/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False ../../third_party/llvm-build/Release+Asserts/bin/lld-link.exe /nologo /IMPLIB:./embedder_switches.dll.lib /DLL /OUT:./embedder_switches.dll /PDB:./embedder_switches.dll.pdb @./embedder_switches.dll.rsp
FAILED: embedder_switches.dll embedder_switches.dll.lib embedder_switches.dll.pdb
d:/src/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False ../../third_party/llvm-build/Release+Asserts/bin/lld-link.exe /nologo /IMPLIB:./embedder_switches.dll.lib /DLL /OUT:./embedder_switches.dll /PDB:./embedder_switches.dll.pdb @./embedder_switches.dll.rsp
d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__QueryPerformanceCounter@4

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_support.c:113
msvcrtd.lib(gs_support.obj):(___get_entropy)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__GetCurrentProcessId@0

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_support.c:104
msvcrtd.lib(gs_support.obj):(___get_entropy)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__GetCurrentThreadId@0

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_support.c:103
msvcrtd.lib(gs_support.obj):(___get_entropy)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__GetSystemTimeAsFileTime@4

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_support.c:95
msvcrtd.lib(gs_support.obj):(___get_entropy)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__DisableThreadLibraryCalls@4

referenced by f:\dd\vctools\crt\vcstartup\src\startup\dll_dllmain_stub.cpp:34
msvcrtd.lib(dll_dllmain_stub.obj):(_DllMain@12)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__InitializeSListHead@4

referenced by f:\dd\vctools\crt\vcstartup\src\eh\tncleanup.cpp:17
msvcrtd.lib(tncleanup.obj):(?__scrt_initialize_type_info@@yaxxz)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__IsDebuggerPresent@0

referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:205
msvcrtd.lib(utility_desktop.obj):(___scrt_fastfail)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__UnhandledExceptionFilter@4

referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:211
msvcrtd.lib(utility_desktop.obj):(___scrt_fastfail)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__SetUnhandledExceptionFilter@4

referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:83
msvcrtd.lib(utility_desktop.obj):(___scrt_set_unhandled_exception_filter)
referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:210
msvcrtd.lib(utility_desktop.obj):(___scrt_fastfail)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__GetStartupInfoW@4

referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:24
msvcrtd.lib(utility_desktop.obj):(___scrt_get_show_window_mode)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: _IsProcessorFeaturePresent@4

referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:134
msvcrtd.lib(utility_desktop.obj):(___scrt_fastfail)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__GetModuleHandleW@4

referenced by f:\dd\vctools\crt\vcstartup\src\utility\utility_desktop.cpp:33
msvcrtd.lib(utility_desktop.obj):(___scrt_is_managed_app)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: _IsProcessorFeaturePresent@4

referenced by msvcrtd.lib(cpu_disp.obj):(___isa_available_init)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__UnhandledExceptionFilter@4

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:147
msvcrtd.lib(gs_report.obj):(___raise_securityfailure)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__SetUnhandledExceptionFilter@4

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:146
msvcrtd.lib(gs_report.obj):(___raise_securityfailure)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__GetCurrentProcess@0

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:160
msvcrtd.lib(gs_report.obj):(___raise_securityfailure)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: __imp__TerminateProcess@8

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:160
msvcrtd.lib(gs_report.obj):(___raise_securityfailure)

d:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe: error: undefined symbol: _IsProcessorFeaturePresent@4

referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:463
msvcrtd.lib(gs_report.obj):(___report_securityfailure)
referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:339
msvcrtd.lib(gs_report.obj):(___report_securityfailureEx)
referenced by f:\dd\vctools\crt\vcstartup\src\gs\gs_report.c:218
msvcrtd.lib(gs_report.obj):(___report_gsfailure)

ninja: build stopped: subcommand failed.

d:\src\chromium\src>gn args out\debug_component
Waiting for editor on "d:\src\chromium\src\out\debug_component\args.gn"...
Generating files...
Done. Made 8508 targets from 1500 files in 8872ms
d:\src\chromium\src>ninja -C out\debug_component embedder_switches.dll
ninja.exe -C out\debug_component embedder_switches.dll -j 640 -l 32
ninja: Entering directory `out\debug_component'
[1 processes, 1/1 @ 0.1/s : 8.719s ] Regenerating ninja files
[1 processes, 2/2 @ 1.0/s : 2.075s ] LINK(DLL) embedder_switches.dll embedder_switches.dll.lib embedder_switches.dll.pdb
FAILED: embedder_switches.dll embedder_switches.dll.lib embedder_switches.dll.pdb
d:/src/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False link.exe /nologo /IMPLIB:./embedder_switches.dll.lib /DLL /OUT:./embedder_switches.dll /PDB:./embedder_switches.dll.pdb @./embedder_switches.dll.rsp
msvcrtd.lib(gs_support.obj) : error LNK2019: unresolved external symbol __imp__QueryPerformanceCounter@4 referenced in function ___get_entropy
msvcrtd.lib(gs_support.obj) : error LNK2019: unresolved external symbol __imp__GetCurrentProcessId@0 referenced in function ___get_entropy
msvcrtd.lib(gs_support.obj) : error LNK2019: unresolved external symbol __imp__GetCurrentThreadId@0 referenced in function ___get_entropy
msvcrtd.lib(gs_support.obj) : error LNK2019: unresolved external symbol __imp__GetSystemTimeAsFileTime@4 referenced in function ___get_entropy
msvcrtd.lib(dll_dllmain_stub.obj) : error LNK2019: unresolved external symbol __imp__DisableThreadLibraryCalls@4 referenced in function _DllMain@12
msvcrtd.lib(tncleanup.obj) : error LNK2019: unresolved external symbol __imp__InitializeSListHead@4 referenced in function "void __cdecl __scrt_initialize_type_info(void)" (?__scrt_initialize_type_info@@yaxxz)
msvcrtd.lib(utility_desktop.obj) : error LNK2019: unresolved external symbol __imp__IsDebuggerPresent@0 referenced in function ___scrt_fastfail
msvcrtd.lib(utility_desktop.obj) : error LNK2019: unresolved external symbol __imp__UnhandledExceptionFilter@4 referenced in function ___scrt_fastfail
msvcrtd.lib(gs_report.obj) : error LNK2001: unresolved external symbol __imp__UnhandledExceptionFilter@4
msvcrtd.lib(utility_desktop.obj) : error LNK2019: unresolved external symbol __imp__SetUnhandledExceptionFilter@4 referenced in function ___scrt_fastfail
msvcrtd.lib(gs_report.obj) : error LNK2001: unresolved external symbol __imp__SetUnhandledExceptionFilter@4
msvcrtd.lib(utility_desktop.obj) : error LNK2019: unresolved external symbol __imp__GetStartupInfoW@4 referenced in function ___scrt_get_show_window_mode
msvcrtd.lib(utility_desktop.obj) : error LNK2019: unresolved external symbol _IsProcessorFeaturePresent@4 referenced in function ___scrt_fastfail
msvcrtd.lib(cpu_disp.obj) : error LNK2001: unresolved external symbol _IsProcessorFeaturePresent@4
msvcrtd.lib(gs_report.obj) : error LNK2001: unresolved external symbol _IsProcessorFeaturePresent@4
msvcrtd.lib(utility_desktop.obj) : error LNK2019: unresolved external symbol __imp__GetModuleHandleW@4 referenced in function ___scrt_is_managed_app
msvcrtd.lib(gs_report.obj) : error LNK2019: unresolved external symbol __imp__GetCurrentProcess@0 referenced in function ___raise_securityfailure
msvcrtd.lib(gs_report.obj) : error LNK2019: unresolved external symbol __imp__TerminateProcess@8 referenced in function ___raise_securityfailure
C:\Program Files (x86)\Windows Kits\10\lib\10.0.17134.0\um\x64\advapi32.lib : warning LNK4272: library machine type 'x64' conflicts with target machine type 'x86'
LINK : error LNK1218: warning treated as error; no output file generated
ninja: build stopped: subcommand failed.

[nico]

half a patch
not ready, but has some interesting notes

i wonder if we could make this an error, not a warning

[nico]

rebased half a patch

[nico]

symbol-table based patch
link.exe seems to only emit this warning if the link fails, else it doesn't bother.

Here's a slightly different approach: It gets the .lib's arch off the .obj file belonging to the first symbol in the .lib's symbol table instead of walking .obj files in the .lib until one with an arch is hit.

[nico]

...and link.exe seems to use the symbol table too:

C:\src\llvm-project>type one.cc
void f() {}

C:\src\llvm-project>type empty.cc

C:\src\llvm-project>type test.cc
void g();
int main() { g(); }

C:\src\llvm-project>out\gn\bin\clang-cl /c -m32 empty.cc /Foempty32.obj
C:\src\llvm-project>out\gn\bin\clang-cl /c -m64 empty.cc /Foempty64.obj
C:\src\llvm-project>out\gn\bin\clang-cl /c -m32 one.cc /Foone32.obj
C:\src\llvm-project>out\gn\bin\clang-cl /c -m64 one.cc /Foone64.obj

C:\src\llvm-project>out\gn\bin\clang-cl /c -m64 test.cc

C:\src\llvm-project>out\gn\bin\lld-link.exe /lib /out:mixed.lib empty32.obj one64.obj /allowMixedArchForTesting

C:\src\llvm-project>link test.obj mixed.lib
test.obj : error LNK2019: unresolved external symbol "void __cdecl g(void)" (?g@@yaxxz) referenced in function main
test.exe : fatal error LNK1120: 1 unresolved externals

C:\src\llvm-project>out\gn\bin\lld-link.exe /lib /out:mixed.lib one32.obj empty64.obj /allowMixedArchForTesting

C:\src\llvm-project>link test.obj mixed.lib
test.obj : error LNK2019: unresolved external symbol "void __cdecl g(void)" (?g@@yaxxz) referenced in function main
mixed.lib : warning LNK4272: library machine type 'x86' conflicts with target machine type 'x64'
test.exe : fatal error LNK1120: 1 unresolved externals

C:\src\llvm-project>out\gn\bin\lld-link.exe /lib /out:mixed.lib empty64.obj one32.obj /allowMixedArchForTesting

C:\src\llvm-project>link test.obj mixed.lib
test.obj : error LNK2019: unresolved external symbol "void __cdecl g(void)" (?g@@yaxxz) referenced in function main
mixed.lib : warning LNK4272: library machine type 'x86' conflicts with target machine type 'x64'
test.exe : fatal error LNK1120: 1 unresolved externals

[nico]

better symbol-table based patch
I dusted off my local branch and cleaned up a bit and wrote a few tests.

Before:

C:\src\llvm-project>out\gn\bin\clang-cl -m32 main.cc -fuse-ld=lld
lld-link: error: : undefined symbol: _mainCRTStartup
clang-cl: error: linker command failed with exit code 1 (use -v to see invocation)

After:

C:\src\llvm-project>out\gn\bin\clang-cl -m32 main.cc -fuse-ld=lld
lld-link: error: C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.25.28610\lib\x64\libcmt.lib: machine type x64 conflicts with x86
lld-link: error: HUGE in d:\agent_work\4\s\Intermediate\vctools\oldnames.nativeproj__391811071\objr\amd64\oldnames\HUGE.obj: unknown machine: 0
clang-cl: error: linker command failed with exit code 1 (use -v to see invocation)

So it's good that we get the machine type diag for libcmt.lib. That does seem much more understandable than the _mainCRTStartup diag.

However, oldnames.lib exclusively contains object files with IMAGE_FILE_MACHINE_UNKNOWN that are all in the symbol table. So maybe we should just ignore static libraries where the first symbol is in such an obj file. XXX: Check what link.exe does if the first symbol is such a symbol but the next one isn't.

These obj files are a bit mysterious:

C:\src\llvm-project>out\gn\bin\llvm-nm.exe "C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.25.28610\lib\x64\oldnames.lib"
...
d:\agent_work\4\s\Intermediate\vctools\oldnames.nativeproj__391811071\objr\amd64\oldnames\utime.obj:
00c7a09e a @​comp.id
00000011 a @​feat.00
U _utime32
00000000 W utime

C:\src\llvm-project>lib /extract:d:\agent_work\4\s\Intermediate\vctools\oldnames.nativeproj__391811071\objr\amd64\oldnames\cgets.obj "C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.25.28610\lib\x64\oldnames.lib"

C:\src\llvm-project>out\gn\bin\obj2yaml.exe cgets.obj
--- !COFF
header:
Machine: IMAGE_FILE_MACHINE_UNKNOWN
Characteristics: [ ]
sections:

• Name: .drectve
  Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
  SectionData: ''
  symbols:
• Name: '@comp.id'
  Value: 13082782
  SectionNumber: -1
  SimpleType: IMAGE_SYM_TYPE_NULL
  ComplexType: IMAGE_SYM_DTYPE_NULL
  StorageClass: IMAGE_SYM_CLASS_STATIC
• Name: '@feat.00'
  Value: 17
  SectionNumber: -1
  SimpleType: IMAGE_SYM_TYPE_NULL
  ComplexType: IMAGE_SYM_DTYPE_NULL
  StorageClass: IMAGE_SYM_CLASS_STATIC
• Name: _cgets
  Value: 0
  SectionNumber: 0
  SimpleType: IMAGE_SYM_TYPE_NULL
  ComplexType: IMAGE_SYM_DTYPE_NULL
  StorageClass: IMAGE_SYM_CLASS_EXTERNAL
• Name: cgets
  Value: 0
  SectionNumber: 0
  SimpleType: IMAGE_SYM_TYPE_NULL
  ComplexType: IMAGE_SYM_DTYPE_NULL
  StorageClass: IMAGE_SYM_CLASS_WEAK_EXTERNAL
  WeakExternal:
  TagIndex: 2
  Characteristics: IMAGE_WEAK_EXTERN_SEARCH_ALIAS
  ...

I don't know if cl.exe can create these IMAGE_SYM_CLASS_WEAK_EXTERNAL symbols (but lld handles them, see isWeakExternal calls in lld\COFF). (See also eg https://reviews.llvm.org/D33520)

This is for the case where no .obj file is pulled in from a .obj file (the C case). If an .obj file from such a .lib is loaded (the C++ case), this currently happens:

C:\src\llvm-project>"c:\src\llvm-project\out\gn\bin\lld-link.exe" "C:\src\llvm-project\out\gn\obj\lld\test\COFF\Output\mixed-arch-lib.s.tmp-main-64.obj" "C:\src\llvm-project\out\gn\obj\lld\test\COFF\Output\mixed-arch-lib.s.tmp-f-32.lib"
lld-link: error: C:\src\llvm-project\out\gn\obj\lld\test\COFF\Output\mixed-arch-lib.s.tmp-f-32.lib: machine type x86 conflicts with x64
lld-link: error: mixed-arch-lib.s.tmp-f-32.lib(mixed-arch-lib.s.tmp-f-32.obj): machine type x86 conflicts with x64

That is, both the .lib and the .obj emit a diag.

That's kind of lame, and with C++ lib files it's not a super contrived example either.

Two ways to think about a fix here:

1. don't emit the .obj file diag for .obj files from .lib files -- if the .lib already errors, we don't need to deal with .obj files from it. The root cause in this view is that ArchiveFile::parse() (and in general InputFile::parse()) should be broken into two phases: The basic coff file parsing, then the arch check should happen, and only if that passes should the contents of the coff file be acted on (added to symbol table or whatever). This might be nice for better error messages from other InputFiles too.

2. don't emit the .lib file diag for .lib files that have at least one (non-IMAGE_FILE_MACHINE_UNKNOWN?) .obj file loaded. In this view, problems are only .lib files that don't have any of their members loaded (because any of them would cause the diag, under the assumption that they all are of the same bitness). Could even do this only if there are unresolved symbols at the end, but that'd be confusing if we're going with an error instead of a warning.

[nico]

Also, the diag in comment 3 looks like it might be against an import library, which in lld is an ImportFile, not an ArchiveFile.

[nico]

mentioned in issue #38315