[PATCH] [lsan] CMakeLists and lit test configs for LeakSanitizer.

Sergey Matveev earthdok at google.com
Mon May 20 02:48:48 PDT 2013



================
Comment at: lib/lsan/CMakeLists.txt:6
@@ +5,3 @@
+  -I${COMPILER_RT_SOURCE_DIR}/lib
+  ${COMPILER_RT_GTEST_INCLUDE_CFLAGS}
+  )
----------------
Alexey Samsonov wrote:
> This and the previous compile flag should not be needed for building LSan.
The previous one is needed to include stuff from sanitizer_common.
Moved this one to tests/CMakeLists.txt

================
Comment at: lib/lsan/CMakeLists.txt:16
@@ +15,3 @@
+  lsan_common_linux.cc
+  )
+
----------------
Alexey Samsonov wrote:
> nit: I prefer
> 
> set(VAR
>   a
>   b)
> 
> to
> 
> set(VAR
>   a
>   b
>   )
> 
> I know it's not consistent in compiler-rt rules, but let's stick to the former in the new files.
> 
fixed

================
Comment at: lib/lsan/CMakeLists.txt:21
@@ +20,3 @@
+set(LSAN_RUNTIME_LIBRARIES)
+  set(arch "x86_64")
+  add_compiler_rt_static_runtime(clang_rt.lsan-${arch} ${arch}
----------------
Alexey Samsonov wrote:
> weird indentation
fixed

================
Comment at: lib/lsan/CMakeLists.txt:25
@@ +24,3 @@
+            $<TARGET_OBJECTS:RTInterception.${arch}>
+            $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
+    CFLAGS ${LSAN_CFLAGS}
----------------
Alexey Samsonov wrote:
> check if you need new weird RTSanitizerCommonLibc stuff.
> I haven't looked at this part yet.
added, since that is where stoptheworld will end up

================
Comment at: lib/lsan/tests/CMakeLists.txt:25
@@ +24,3 @@
+    clang_compile(${output_obj} ${source}
+                        CFLAGS ${ARGN} ${TARGET_CFLAGS}
+                        DEPS gtest)
----------------
Alexey Samsonov wrote:
> weird indentation here and below
fixed

================
Comment at: lib/lsan/tests/CMakeLists.txt:49
@@ +48,3 @@
+  add_dependencies(Lsan-${arch}-Test  lsan_tls_loadable-${arch})
+  message(${CMAKE_CURRENT_BINARY_DIR})
+endmacro()
----------------
Alexey Samsonov wrote:
> debug print?
removed

================
Comment at: lib/lsan/tests/CMakeLists.txt:52
@@ +51,3 @@
+
+# Build on 64-bit Linux.
+if(UNIX AND NOT APPLE
----------------
Alexey Samsonov wrote:
> Change comment to "Build tests for 64-bit Linux only"
done

================
Comment at: lib/lsan/tests/CMakeLists.txt:55
@@ +54,3 @@
+    AND CAN_TARGET_x86_64
+    AND CMAKE_SIZEOF_VOID_P EQUAL 8
+    AND NOT LLVM_BUILD_32_BITS)
----------------
Alexey Samsonov wrote:
> remove this and the following line
done

================
Comment at: lib/lsan/tests/CMakeLists.txt:69
@@ +68,3 @@
+    ${LSAN_RUNTIME_LIBRARIES}
+    )
+  set(LSAN_TEST_PARAMS
----------------
Alexey Samsonov wrote:
> see nit about closing brace above
fixed everywhere

================
Comment at: lib/lsan/tests/CMakeLists.txt:36
@@ +35,3 @@
+  set_target_compile_flags(${testname_arch} ${LSAN_CFLAGS})
+  set_property(TARGET ${testname_arch} APPEND_STRING
+    PROPERTY LINK_FLAGS " -pie")
----------------
Alexey Samsonov wrote:
> you may want to use set_target_link_flags
fixed (didn't even need to set -pie manually after all)


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



More information about the llvm-commits mailing list