[PATCH] D126706: [CMake] Improve support for ASAN on Windows with MSVC cl & clang-cl

[Markus Böck via Phabricator via llvm-commits]

zero9178 added inline comments.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:897
+        if (${LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE}} MATCHES "^(MT|MTd)$")
+          append("/wholearchive:clang_rt.asan-x86_64.lib /wholearchive:clang_rt.asan_cxx-x86_64.lib"
+            CMAKE_EXE_LINKER_FLAGS)
----------------
andrewng wrote:
> zero9178 wrote:
> > rnk wrote:
> > > This assumes x86_64, while i386 is also a possibility.
> > > 
> > > I wish we hadn't embedded the architecture into our library names. =/
> > They may even possibly not have a suffix at all if the clang toolchain has per target runtime directories. In that case it is eg. called just `clang_rt.asan.lib` on my machine. 
> > 
> > So this seems like if we want it to actually handle all possible options we'll have to loop and check for the existence of those files.
> > One can use either `clang-cl /clang:-print-runtime-dir` to get the directory of the runtime files if the clang version is new enough or otherwise `clang-cl /clang:-print-libgcc-file-name /clang:--rtlib=compiler-rt` to get the (likely non existent) builtins file to extract the directory from. The latter is quite a hack however that has worked well for me so far.
> Thanks for the comments.
> 
> I have assumed `x86_64` as I believe that's the most commonly used. I deliberately titled this patch "improve" as my goal wasn't really to attempt to cope with every possible configuration. Is this acceptable? Perhaps it can be improved incrementally for other desired configurations.
Doing it incrementally sounds good to me, but in the current state it would break existing configurations, so it wouldn't just be an improvement but an regression as well. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126706/new/

https://reviews.llvm.org/D126706