[PATCH] D29783: Add cmake build support for lsan on OS X

[Francis Ricci via Phabricator via llvm-commits]

fjricci added inline comments.


================
Comment at: cmake/config-ix.cmake:167-171
+if(APPLE)
+  set(ALL_LSAN_SUPPORTED_ARCH ${X86_64} ${ARM64})
+else()
+  set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64})
+endif()
----------------
kubamracek wrote:
> Why exactly do we need this change?  Will the 32-bit x86 slice fail to build?
Yeah, `__thread` is only supported on 64-bit darwin targets.


================
Comment at: lib/lsan/CMakeLists.txt:19-21
+if(APPLE AND COMPILER_RT_HAS_LSAN)
+  set(LSAN_CFLAGS ${LSAN_CFLAGS} -DCAN_SANITIZE_LEAKS_MAC=1)
+endif()
----------------
kubamracek wrote:
> How about we just unconditionally propagate `CAN_SANITIZE_LEAKS` (no `_MAC`) and remove that ugly ifdef in lsan_common.h?
> 
> Doesn't this also need to go to lib/asan/CMakeLists.txt?
Well, the issue with that is that we build LSanCommon for 32-bit architectures (since asan builds for 32-bit), so we still need the architecture check in lsan_common.h to make sure that it's an empty stub if we're building for 32-bit.

This doesn't need to go into lib/asan/CMakeLists.txt because that just pulls in RTLSanCommon, not the individual sources, and RTLsanCommon will already have the flag.


https://reviews.llvm.org/D29783