[llvm-commits] [PATCH] [msan] MemorySanitizer unit tests.
Alexey Samsonov
samsonov at google.com
Mon Dec 24 07:46:47 PST 2012
Didn't look at unit test itself yet.
================
Comment at: cmake/Modules/CompilerRTCompile.cmake:18
@@ +17,3 @@
+
+# Link a shared library with just-built Clang.
+# clang_link_shared(<output.so>
----------------
Maybe you can move this to CompilerRTLink.cmake
================
Comment at: cmake/Modules/CompilerRTCompile.cmake:21
@@ +20,3 @@
+# OBJECTS <list of input objects>
+# LINKFLAGS <list of link flags>
+# DEPS <list of dependencies>)
----------------
I think for clarity you can use format
clang_link_shared(<output.so> OBJECTS <list of inputs> FLAGS <link flags> DEPS <deps>),
and then
parse_arguments(LINK "OBJECS;FLAGS;DEPS" "" ${ARGN})
so you'll have reasonable variables
LINK_OBJECTS, LINK_FLAGS and LINK_DEPS.
================
Comment at: lib/msan/CMakeLists.txt:1
@@ -1,2 +1,2 @@
# Build for the MemorySanitizer runtime support library.
----------------
+1 for moving tests into separate directory
================
Comment at: lib/msan/CMakeLists.txt:21
@@ -11,1 +20,3 @@
+ -fPIE
+ -ffreestanding)
----------------
Add a comment describing why we need it.
================
Comment at: lib/msan/CMakeLists.txt:30
@@ +29,3 @@
+set(MSAN_LIBCXX_CFLAGS
+ -I${MSAN_LIBCXX_PATH}/include
+ -fsanitize=memory
----------------
You have MSAN_LIBCXX_INCLUDE_CFLAGS
================
Comment at: lib/msan/CMakeLists.txt:24
@@ +23,3 @@
+# Instrumented libcxx sources and build flags.
+set(MSAN_LIBCXX_PATH ${LLVM_MAIN_SRC_DIR}/projects/libcxx)
+file(GLOB MSAN_LIBCXX_SOURCES ${MSAN_LIBCXX_PATH}/src/*.cpp)
----------------
What would happen if libcxx is not checked out? I think we need to just not build the unit tests in this case. We may need a variable defined in somewhere like projects/compiler-rt/CMakeLists.txt (or in modules) that would tell us if we have libcxx. This can be done in later commits, but please add a FIXME in this case.
================
Comment at: lib/msan/CMakeLists.txt:55
@@ +54,3 @@
+set(MSAN_UNITTEST_COMMON_CFLAGS
+ -I${MSAN_LIBCXX_PATH}/include
+ ${COMPILER_RT_GTEST_INCLUDE_CFLAGS}
----------------
MSAN_LIBCXX_INCLUDE_CFLAGS
================
Comment at: lib/msan/CMakeLists.txt:102
@@ +101,3 @@
+ CFLAGS ${ARGN} ${TARGET_CFLAGS}
+ DEPS gtest ${MSAN_RUNTIME_LIBRARIES})
+ list(APPEND ${obj_list} ${output_obj})
----------------
You need to explicitly depend on unittest header files (see my recent changes in ASan)
================
Comment at: lib/msan/CMakeLists.txt:107
@@ +106,3 @@
+macro(msan_link_shared so_list so_name arch)
+ parse_arguments(SOURCE "OBJECTS;LINKFLAGS;DEPS" "" ${ARGN})
+ get_unittest_directory(OUTPUT_DIR)
----------------
Same here, SOURCE seems pretty much irrelevant here.
http://llvm-reviews.chandlerc.com/D236
More information about the llvm-commits
mailing list