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

[andrewng]

Summary

Tested with MSVC 2019 (19.29) and LLVM 14.0.4.

[rnk]

llvm/cmake/modules/HandleLLVMOptions.cmake

827

This isn't clang-specific, it's x86_32-specific.

897

This assumes x86_64, while i386 is also a possibility.

I wish we hadn't embedded the architecture into our library names. =/

[zero9178]

llvm/cmake/modules/HandleLLVMOptions.cmake

897

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.

[andrewng]

llvm/cmake/modules/HandleLLVMOptions.cmake

827

I just kept this the same as it was but you're right it is x86_32 specific. Do you want me to address this?

897

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.

[zero9178]

llvm/cmake/modules/HandleLLVMOptions.cmake

897

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.

[andrewng]

llvm/cmake/modules/HandleLLVMOptions.cmake

897

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.

[andrewng]

I've added support for x86_32/i686.

[rnk]

lgtm

llvm/cmake/modules/HandleLLVMOptions.cmake

827

There's probably a better way to check the architecture, but I'm not sure what it is and this seems good enough.

llvm/cmake/modules/HandleLLVMOptions.cmake

if (NOT MSVC)
     if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND
         NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO")
       add_flag_if_supported("-gline-tables-only" GLINE_TABLES_ONLY)
     endif()
     # Use -O1 even in debug mode, otherwise sanitizers slowdown is too large.
     if (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND LLVM_OPTIMIZE_SANITIZED_BUILDS)
       add_flag_if_supported("-O1" O1)
     endif()
-  elseif (CLANG_CL)
-    # Keep frame pointers around.
-    append("/Oy-" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+  else()
     # Always ask the linker to produce symbols with asan.
     append("/Z7" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
-    append("-debug" CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
+    append("/debug" CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
+    # Not compatible with /INCREMENTAL link.
+    foreach (flags_opt_to_scrub
+        CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
+      string (REGEX REPLACE "(^| )/INCREMENTAL($| )" " /INCREMENTAL:NO "
+        "${flags_opt_to_scrub}" "${${flags_opt_to_scrub}}")
+    endforeach()
+    if (LLVM_HOST_TRIPLE MATCHES "i[2-6]86-.*")
+      # Keep frame pointers around.
+      append("/Oy-" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+    endif()
   endif()
 endmacro()
 
 # Turn on sanitizers if necessary.
 if(LLVM_USE_SANITIZER)
   if (LLVM_ON_UNIX)
     if (LLVM_USE_SANITIZER STREQUAL "Address")
       append_common_sanitizer_flags()
elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
       append("-fsanitize=address" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
       append("${LLVM_UBSAN_FLAGS}" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
     else()
       message(FATAL_ERROR "This sanitizer not yet supported in a MinGW environment: ${LLVM_USE_SANITIZER}")
     endif()
   elseif(MSVC)
     if (LLVM_USE_SANITIZER STREQUAL "Address")
       append_common_sanitizer_flags()
-      append("-fsanitize=address" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+      append("/fsanitize=address" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+      if (NOT CLANG_CL)
+        # Not compatible with /RTC flags.
+        foreach (flags_opt_to_scrub
+            CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE} CMAKE_C_FLAGS_${uppercase_CMAKE_BUILD_TYPE})
+          string (REGEX REPLACE "(^| )/RTC[1csu]*($| )" " "
+            "${flags_opt_to_scrub}" "${${flags_opt_to_scrub}}")
+        endforeach()
+      endif()
+      if (LINKER_IS_LLD_LINK)
+        if (LLVM_HOST_TRIPLE MATCHES "i[2-6]86-.*")
+          set(arch "i386")
+        else()
+          set(arch "x86_64")
+        endif()
+        if (${LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE}} MATCHES "^(MT|MTd)$")
+          append("/wholearchive:clang_rt.asan-${arch}.lib /wholearchive:clang_rt.asan_cxx-${arch}.lib"
+            CMAKE_EXE_LINKER_FLAGS)
+          append("/wholearchive:clang_rt.asan_dll_thunk-${arch}.lib"
+            CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
+        else()
+          append("clang_rt.asan_dynamic-${arch}.lib /wholearchive:clang_rt.asan_dynamic_runtime_thunk-${arch}.lib"
+            CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
+        endif()
+      endif()
     else()
       message(FATAL_ERROR "This sanitizer not yet supported in the MSVC environment: ${LLVM_USE_SANITIZER}")
     endif()
   else()
     message(FATAL_ERROR "LLVM_USE_SANITIZER is not supported on this platform.")
   endif()
   if (LLVM_USE_SANITIZER MATCHES "(Undefined;)?Address(;Undefined)?")
     add_flag_if_supported("-fsanitize-address-use-after-scope"