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

[Andrew Ng via Phabricator via llvm-commits]

andrewng 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)
----------------
zero9178 wrote:
> 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. 
The addition of the ASAN libraries was never setup before when `lld-link` is being used. Are you saying that there are configurations that don't require this and therefore by adding these libraries, it would break those configurations? Could you shed some light on such configurations? If there's another way to make ASAN work without modifying the CMake, then perhaps that's the way to go.


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

https://reviews.llvm.org/D126706