The build uses other mechanism to select the runtime.
Fixes #62719
Differential D150688
[cmake] Set CMP0091 to fix Windows builds after the cmake_minimum_required bump hans on May 16 2023, 9:22 AM. Authored by
Details The build uses other mechanism to select the runtime. Fixes #62719
Diff Detail Event TimelineComment Actions Thanks for the patch! When this fixes the CI I'm happy with it.
Comment Actions
I think it's not just compiler-rt, but all of llvm that's affected. Agreed, this seems like a larger project that should be done separately by a cmake enthusiast :) Comment Actions Out of curiosity - what are the symptoms that this change fixes, and which build configurations does it affect? (I would expect that regular builds with just llvm+clang would pass at least?) But I do agree that the new behaviour of CMP0091 probably does conflict with ChooseMSVCCRT.cmake. Tangentially; for libc++ builds, ChooseMSVCCRT.cmake isn't ever included and used so far. So for the sake of libc++ at the moment, I'd go with keeping CMP0091 set to the new behaviour - which probably is the more convenient way of interacting with the choice of CRT going forward. I do see that CMakePolicy.cmake doesn't get included in libc++ builds either, so I think that should all be fine. Comment Actions CMakePolicy.cmake is also not included in compiler-rt/CMakeLists.txt or runtimes/CMakeLists.txt, so this doesn't help with the issue when building compiler-rt standalone. Comment Actions I've seen two issues so far, but there could also be others.
> cmake -GNinja -DLLVM_ENABLE_PROJECTS="llvm;clang;compiler-rt" -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_TARGETS_TO_BUILD=X86 ..\llvm && ninja check-fuzzer [...] [2985/2987] Generating Fuzzer-x86_64-Test.exe FAILED: projects/compiler-rt/lib/fuzzer/tests/Fuzzer-x86_64-Test.exe cmd.exe /C "cd /D C:\src\llvm-project\build.nana\projects\compiler-rt\lib\fuzzer\tests && C:\src\llvm-project\build.nana\.\bin\clang++.exe FuzzerTestObjects.FuzzerUnittest.cpp.x86_64.o FuzzerTestObjects.gtest-all.cc.x86_64.o C:/src/llvm-project/build.nana/projects/compiler-rt/lib/fuzzer/tests/RTFuzzerTest.x86_64.lib -o C:/src/llvm-project/build.nana/projects/compiler-rt/lib/fuzzer/tests/./Fuzzer-x86_64-Test.exe --driver-mode=g++ -Wl,-defaultlib:libcmt,-defaultlib:oldnames" RTFuzzerTest.x86_64.lib(FuzzerIOWindows.cpp.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MD_DynamicRelease' doesn't match value 'MT_StaticRelease' in FuzzerTestObjects.FuzzerUnittest.cpp.x86_64.o It seems the test object ended up with /MT but the library got /MD for some reason.
lld-link: error: duplicate symbol: calloc >>> defined at LLVMSupport.lib(rpmalloc.c.obj) >>> defined at api-ms-win-crt-heap-l1-1-0.dll It seems we're still seeing some crt related link errors (https://bugs.chromium.org/p/chromium/issues/detail?id=1445671#c7) so maybe this wasn't enough.
Does libc++ do anything else to choose crt? compiler-rt has seems to have its own thing in https://github.com/llvm/llvm-project/blob/llvmorg-16.0.3/compiler-rt/CMakeLists.txt#L398 Comment Actions Actually, I was mistaken, ChooseMSVCCRT.cmake does get included there too. However only the default of /MD is supported anyway, at the moment.
Oh, I see. This seems to look for the literal form /MD, but after the CMake update, CMake seems to set the option in the form -MD, so if this pattern is updated to check for both / and -, this might start working as expected. Comment Actions Although, the new policy does place the flag in a different separate cmake variable, so it’s quite possible that it isn’t enough. In any case, with the new policy, the right thing to do is to set the variable (or target property) for deciding what CRT to use, not to manually exchange flags in the variables. Comment Actions Yes, I tried changing those regexes but it didn't help (https://github.com/llvm/llvm-project/issues/62719#issuecomment-1551493063). Comment Actions Reverted this and follow-ups in d763c6e5e2d0a6b34097aa7dabca31e9aff9b0b6 for now. Sorry this is such a pain to land :( Comment Actions …that comment was for D144509, but this was one of the follow-ups reverted in that rev. Comment Actions Since you're mentioning /MD, I spotted this: https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/orc/CMakeLists.txt#L134, which ends up *not* adding /MD on the command line because of the quotes (ORC_CFLAGS containing -Isomething, this ends up adding an invalid directory that ends with /MD to the include list). This is yet another thing that should be done through setting the runtime properly. |
Maybe put this on top so the policies are ordered by the CMake version introducing them.