[CMake][MSVC] Add include path to crt headers when enabling sanitizers.

[pgousseau]

Summary

This change is to fix build errors “Cannot open include file: 'sanitizer/asan_interface.h'” when building LLVM with MSVC and LLVM_USE_SANITIZER=Address.

I have used the VCToolsInstallDir environment variable to generate the path, let me know if there is a better alternative.

Tested with visual studio 2019 v16.9.6

[thakis]

(removing myself as reviewer; i don't know anything about using address sanitizer with msvc's cl.exe)

[rnk]

Can we use __has_include instead? I think that would be simpler, which I'd prefer. MSVC claims to support it:
https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-170

[rnk]

llvm/cmake/modules/HandleLLVMOptions.cmake

818

I sent the previous comment too early, I see this now.

@cbezault , is there a reason MSVC doesn't ship asan_interface.h with the other includes? I imagine there could be some bad interaction if clang-cl finds the wrong asan_interface.h (MSVC's instead its own from compiler-rt), but clang puts its resource directory onto the search path first, so this should work.

If the usage if asan_interface.h is to call the LSan APIs, I think it would be reasonable to still use __has_include, since LSan support is new and somewhat optional. I would rather not add all internal crt sources to the header search paths used for all compiles in LLVM.

[cbezault]

llvm/cmake/modules/HandleLLVMOptions.cmake

818

This is not something that I've looked at. @stwish_msft if you want to comment.

[pgousseau]

llvm/cmake/modules/HandleLLVMOptions.cmake

818

Thank you for reviewing!
We seem to include asan_interface.h to use asan manual poisoning in Allocator.h

So, it seems we would lose some Asan coverage if not including asan_interface.h

With this change I mean to only affect cl.exe builds and not clang-cl, let me know if this is not the case.

I have a separate patch for an lsan api msvc issue at https://reviews.llvm.org/D118162

[pgousseau]

I have made an alternative patch at https://reviews.llvm.org/D118624 to avoid adding all internal crt sources to the header search paths. Not ideal either, let me know what you think, thanks!

[pgousseau]

Abandoned in favour of https://reviews.llvm.org/D118624

llvm/cmake/modules/HandleLLVMOptions.cmake

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)
     # 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)
+  elseif (MSVC)
+    check_cxx_source_compiles(
+      "#include <sanitizer/asan_interface.h> int main() { return 0; }"
+      msvc_sanitizer_include_enabled)
+    if(NOT msvc_sanitizer_include_enabled)
+      file(TO_CMAKE_PATH "$ENV{VCToolsInstallDir}/crt/src" MSVC_CRT_SRC_DIR)
+      if(NOT EXISTS "${MSVC_CRT_SRC_DIR}/sanitizer")
+        message(WARNING "Sanitizer include directory not found in ${MSVC_CRT_SRC_DIR}")
+      endif()
+      include_directories(SYSTEM ${MSVC_CRT_SRC_DIR})
+    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()