Enable builds of darwin lsan by default

[fjricci]

Summary

Testing and asan leak detection are disabled by default.

[kubamracek]

lib/asan/asan_flags.cc

64

asan_flags.cc currently doesn't have any platform-specific code. Could this be moved to some _mac.cc file? Or perhaps into sanitizer_flags.inc?

If not, then this at least deserves a comment explaining that we're bringing up LSan and it doesn't fully work yet on Darwin.

[fjricci]

Account for default detect_leaks value when setting asan default value

[fjricci]

lib/asan/asan_flags.cc

64

I think the correct behavior here would be to account for the default value of cf.detect_leaks, as set in sanitizer_flags.inc when determining the default value for asan. However, this appears to be causing some problems with 32-bit lsan tests, where things should also be disabled by default. I'm looking into those test failures now.

64

I think the correct behavior here would be to account for the default value of cf.detect_leaks, as set in sanitizer_flags.inc when determining the default value for asan. However, this appears to be causing some problems with 32-bit lsan tests, where things should also be disabled by default. I'm looking into those test failures now.

[fjricci]

Ok, pending D31430, this now passes check-asan tests. Working on Linux check-lsan now, and running into more issues with i386 builds.

[fjricci]

Updated that diff, and now check-all passes locally on darwin and linux.

[fjricci]

Rebase

[fjricci]

Ping - I think @kubamracek was already mostly fine with this, except the comment I addressed.

[kubamracek]

A patch that removes more lines than adds? LGTM!

cmake/config-ix.cmake

 
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND LSAN_SUPPORTED_ARCH AND
     OS_NAME MATCHES "Linux|FreeBSD")
   set(COMPILER_RT_HAS_LSAN TRUE)
 else()
   set(COMPILER_RT_HAS_LSAN FALSE)
 endif()
 
-if(APPLE)
-  option(COMPILER_RT_ENABLE_LSAN_OSX "Enable building LSan for OS X - Experimental" Off)
-  if(COMPILER_RT_ENABLE_LSAN_OSX)
-    set(COMPILER_RT_HAS_LSAN TRUE)
-  endif()
-endif()
-
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND MSAN_SUPPORTED_ARCH AND
     OS_NAME MATCHES "Linux")
   set(COMPILER_RT_HAS_MSAN TRUE)
 else()
   set(COMPILER_RT_HAS_MSAN FALSE)
 endif()
 
 if (PROFILE_SUPPORTED_ARCH AND NOT LLVM_USE_SANITIZER AND

lib/asan/CMakeLists.txt

 
 set(ASAN_PREINIT_SOURCES
   asan_preinit.cc)
 
 include_directories(..)
 
 set(ASAN_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 
-# FIXME(fjricci) - remove this once lsan for darwin is fully enabled
-if(APPLE AND COMPILER_RT_HAS_LSAN)
-  set(ASAN_CFLAGS ${ASAN_CFLAGS} -DCAN_SANITIZE_LEAKS_MAC=1)
-endif()
 append_rtti_flag(OFF ASAN_CFLAGS)
 
 set(ASAN_DYNAMIC_LINK_FLAGS)
 
 if(ANDROID)
 # On Android, -z global does not do what it is documented to do.
 # On Android, -z global moves the library ahead in the lookup order,
 # placing it right after the LD_PRELOADs. This is used to compensate for the fact

lib/asan/asan_flags.cc

 }
 
 void InitializeFlags() {
   // Set the default values and prepare for parsing ASan and common flags.
   SetCommonFlagsDefaults();
   {
     CommonFlags cf;
     cf.CopyFrom(*common_flags());
-    cf.detect_leaks = CAN_SANITIZE_LEAKS;
+    cf.detect_leaks = cf.detect_leaks && CAN_SANITIZE_LEAKS;
     cf.external_symbolizer_path = GetEnv("ASAN_SYMBOLIZER_PATH");
     cf.malloc_context_size = kDefaultMallocContextSize;
     cf.intercept_tls_get_addr = true;
     cf.exitcode = 1;
     OverrideCommonFlags(cf);
   }
   Flags *f = flags();
   f->SetDefaults();

lib/lsan/CMakeLists.txt

set(LSAN_SOURCES
   lsan_allocator.cc
   lsan_linux.cc
   lsan_interceptors.cc
   lsan_preinit.cc
   lsan_thread.cc)
 
 set(LSAN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
-# FIXME(fjricci) - remove this once lsan for darwin is fully enabled
-if(APPLE AND COMPILER_RT_HAS_LSAN)
-  set(LSAN_CFLAGS ${LSAN_CFLAGS} -DCAN_SANITIZE_LEAKS_MAC=1)
-endif()
 add_compiler_rt_object_libraries(RTLSanCommon
     OS ${SANITIZER_COMMON_SUPPORTED_OS}
     ARCHS ${LSAN_COMMON_SUPPORTED_ARCH}
     SOURCES ${LSAN_COMMON_SOURCES}
     CFLAGS ${LSAN_CFLAGS})
 
 if(COMPILER_RT_HAS_LSAN)
   add_compiler_rt_component(lsan)

lib/lsan/lsan_common.h

 // LeakSanitizer relies on some Glibc's internals (e.g. TLS machinery) thus
 // supported for Linux only. Also, LSan doesn't like 32 bit architectures
 // because of "small" (4 bytes) pointer size that leads to high false negative
 // ratio on large leaks. But we still want to have it for some 32 bit arches
 // (e.g. x86), see https://github.com/google/sanitizers/issues/403.
 // To enable LeakSanitizer on new architecture, one need to implement
 // internal_clone function as well as (probably) adjust TLS machinery for
 // new architecture inside sanitizer library.
-#if (SANITIZER_LINUX && !SANITIZER_ANDROID) && (SANITIZER_WORDSIZE == 64) && \
+#if (SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC) && \
+    (SANITIZER_WORDSIZE == 64) &&                               \
     (defined(__x86_64__) || defined(__mips64) || defined(__aarch64__))
 #define CAN_SANITIZE_LEAKS 1
-#elif SANITIZER_LINUX && !SANITIZER_ANDROID && defined(__i386__)
-#define CAN_SANITIZE_LEAKS 1
-#elif CAN_SANITIZE_LEAKS_MAC && (defined(__x86_64__) ||  defined(__mips64) || \
-      defined(__aarch64__) || defined(__i386__))
+#elif defined(__i386__) && \
+    (SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC)
 #define CAN_SANITIZE_LEAKS 1
 #else
 #define CAN_SANITIZE_LEAKS 0
 #endif
 
 namespace __sanitizer {
 class FlagParser;
 struct DTLS;

lib/sanitizer_common/sanitizer_flags.inc

COMMON_FLAG(
     "append executable name to logs (as in \"log_path.exe_name.pid\").")
 COMMON_FLAG(
     bool, log_to_syslog, SANITIZER_ANDROID || SANITIZER_MAC,
     "Write all sanitizer output to syslog in addition to other means of "
     "logging.")
 COMMON_FLAG(
     int, verbosity, 0,
     "Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).")
-COMMON_FLAG(bool, detect_leaks, true, "Enable memory leak detection.")
+COMMON_FLAG(bool, detect_leaks, !SANITIZER_MAC, "Enable memory leak detection.")
 COMMON_FLAG(
     bool, leak_check_at_exit, true,
     "Invoke leak checking in an atexit handler. Has no effect if "
     "detect_leaks=false, or if __lsan_do_leak_check() is called before the "
     "handler has a chance to run.")
 COMMON_FLAG(bool, allocator_may_return_null, false,
             "If false, the allocator will crash instead of returning 0 on "
             "out-of-memory.")