[PATCH] Optional support for dynamic Asan runtime

[Alexey Samsonov]

================
Comment at: cmake/Modules/AddCompilerRT.cmake:57
@@ -55,2 +56,3 @@
     # Setup correct output directory in the build tree.
     set_target_properties(${name} PROPERTIES
+      LIBRARY_OUTPUT_DIRECTORY ${COMPILER_RT_LIBRARY_OUTPUT_DIR})
----------------
Use a single call:
  set_target_properties(${name} PROPERTIES
    ARCHIVE_OUTPUT_DIRECTORY ...
    LIBRARY_OUTPUT_DIRECTORY ...)

================
Comment at: cmake/Modules/AddCompilerRT.cmake:75
@@ +74,3 @@
+  # Add installation command.
+  install(TARGETS ${name}
+    ARCHIVE DESTINATION ${COMPILER_RT_LIBRARY_INSTALL_DIR})
----------------
Is it possible to use
  install(TARGETS ${name}
    ARCHIVE DESTINATION ...
    LIBRARY DESTINATION ...)
in the add_compiler_rt_runtime above and get rid of add_compiler_rt_{static,shared}_runtime rules?

================
Comment at: lib/asan/CMakeLists.txt:57
@@ +56,3 @@
+
+set(ASAN_DYNAMIC_LIBS stdc++)
+append_if(COMPILER_RT_HAS_LIBPTHREAD pthread ASAN_DYNAMIC_LIBS)
----------------
  set(ASAN_DYNAMIC_LIBS stdc++ c m)

================
Comment at: lib/asan/asan_linux.cc:92
@@ +91,3 @@
+  }
+
+#endif
----------------
remove newline

================
Comment at: lib/asan/asan_linux.cc:67
@@ -65,1 +66,3 @@
 
+static int DsoCallback(struct dl_phdr_info *info, size_t size,
+                       void *data) {
----------------
Use more specific name (FindFirstDynamicLibraryCallback or some such).

================
Comment at: lib/asan/asan_linux.cc:100
@@ +99,3 @@
+#if ASAN_DYNAMIC
+    badsym = "__asan_static";
+#else
----------------
Weird indentation.

================
Comment at: lib/asan/asan_linux.cc:105
@@ +104,3 @@
+  if (dlsym(RTLD_DEFAULT, badsym)) {
+    Report("Your application is linked against "
+      "incompatible ASan runtimes.\n");
----------------
Consider running tools/clang/tools/clang-format/clang-format-diff.py on your patch.

================
Comment at: lib/asan/asan_malloc_linux.cc:66
@@ +65,3 @@
+static const uptr kCallocPoolSize = 1 << kCallocPoolSizeLog;
+static u8 calloc_memory_for_dlsym[kCallocPoolSize] ALIGNED(kCallocPoolSize);
+
----------------
Use ALIGNED before the type (see comments in sanitizer_internal_defs.h)

================
Comment at: lib/asan/asan_malloc_linux.cc:73
@@ +72,3 @@
+  static uptr allocated;
+  uptr size_aligned = (size + kWordSize - 1) & ~(kWordSize - 1);
+  void *mem = (void*)&calloc_memory_for_dlsym[allocated];
----------------
  uptr size_aligned = RoundUpTo(size, kWordSize);

================
Comment at: lib/asan/asan_malloc_linux.cc:76
@@ +75,3 @@
+  allocated += size_aligned;
+  CHECK(allocated < kCallocPoolSize);
+  return mem;
----------------
CHECK_LT

================
Comment at: lib/asan/asan_malloc_linux.cc:64
@@ -63,1 +63,3 @@
 
+static const uptr kCallocPoolSizeLog = 12;
+static const uptr kCallocPoolSize = 1 << kCallocPoolSizeLog;
----------------
Rename the variables - they are used not only in calloc now.

================
Comment at: lib/asan/asan_malloc_linux.cc:115
@@ +114,3 @@
+  if (!asan_inited)
+    return AsanPreinitAlloc(size);
+  if (AsanIsPreinitAllocated(ptr))
----------------
This is just wrong. Is realloc() called before asan_inited or on PreinitAllocated pointers?

================
Comment at: test/asan/lit.cfg:43
@@ +42,3 @@
+if config.asan_dynamic:
+  clang_asan_static_cflags = list(clang_asan_cflags)
+  clang_asan_cflags.append('-shared-libasan')
----------------
Why do you need this?

================
Comment at: test/asan/lit.cfg:45
@@ +44,3 @@
+  clang_asan_cflags.append('-shared-libasan')
+  clang_asan_static_cxxflags = list(clang_asan_cxxflags)
+  clang_asan_cxxflags.append('-shared-libasan')
----------------
and this?

================
Comment at: test/asan/TestCases/Linux/interception_malloc_test.cc:13
@@ +12,3 @@
+extern "C"
+__attribute__((no_sanitize_address))  // Malloc may be called from dlsym in __asan_init
+void *malloc(size_t size) {
----------------
So, how does __attribute__((no_sanitize_address)) help here?

================
Comment at: test/asan/CMakeLists.txt:44
@@ -35,1 +43,3 @@
+  set(ASAN_TEST_DYNAMIC False)
+  set(ASAN_RT_SUFFIX "i386")
   configure_lit_site_cfg(
----------------
Can you instead just unconditionally provide the basename of dynamic ASan runtime (for i386, x86_64, powerpc64) when configuring a dynamic test suite (and not deal with this asan_rt_suffix)?


http://llvm-reviews.chandlerc.com/D3042