[PATCH] [lsan] Run the leak checking tests under both ASan and LSan.
Alexey Samsonov
samsonov at google.com
Thu May 30 12:41:38 PDT 2013
I like the general implementation.
================
Comment at: lib/lsan/lit_tests/ASan/lit.cfg:15
@@ +14,3 @@
+lsan_lit_cfg = os.path.join(lsan_lit_src_root, "lit.common.cfg")
+if (not lsan_lit_cfg) or (not os.path.exists(lsan_lit_cfg)):
+ lit.fatal("Can't find common LSan lit config at: %r" % lsan_lit_cfg)
----------------
lsan_lit_cfg can't be None here, I presume
================
Comment at: lib/lsan/lit_tests/ASan/lit.cfg:5
@@ +4,3 @@
+
+def get_required_attr(config, attr_name):
+ attr_value = getattr(config, attr_name, None)
----------------
Oh, how I hate this copy-paste. We should do a separate change to clean this up, though.
================
Comment at: lib/lsan/lit_tests/ASan/lit.cfg:1
@@ +1,2 @@
+# -*- Python -*-
+
----------------
Let's stick to lowercase directory names. ASan->asan
================
Comment at: lib/lsan/lit_tests/CMakeLists.txt:26
@@ +25,3 @@
+ foreach(arch ${LSAN_SUPPORTED_ARCH})
+ list(APPEND LSAN_TEST_DEPS clang_rt.asan-${arch})
+ endforeach()
----------------
No, this is Linux-only. I think we had some variable like ASAN_RUNTIME_LIBRARIES.
================
Comment at: lib/lsan/lit_tests/CMakeLists.txt:32
@@ -23,3 +31,3 @@
add_lit_testsuite(check-lsan "Running the LeakSanitizer tests"
- ${CMAKE_CURRENT_BINARY_DIR}
- PARAMS ${LSAN_TEST_PARAMS}
+ ${CMAKE_CURRENT_BINARY_DIR}/LSan
+ ${CMAKE_CURRENT_BINARY_DIR}/ASan
----------------
See above, I prefer ASan->asan, LSan->lsan (that's how the folders are called in //projects/compiler-rt/lib).
================
Comment at: lib/lsan/lit_tests/LSan/lit.cfg:15
@@ +14,3 @@
+lsan_lit_cfg = os.path.join(lsan_lit_src_root, "lit.common.cfg")
+if (not lsan_lit_cfg) or (not os.path.exists(lsan_lit_cfg)):
+ lit.fatal("Can't find common LSan lit config at: %r" % lsan_lit_cfg)
----------------
lsan_lit_cfg is not None here
================
Comment at: lib/lsan/lit_tests/Unit/lit.cfg:8
@@ -7,3 +7,3 @@
if not attr_value:
- lit.fatal("No attribute %r in test configuration! You may need to run "
+ lit.fatal("*No attribute %r in test configuration! You may need to run "
"tests from your build directory or add this attribute "
----------------
remove "*"
================
Comment at: lib/lsan/lit_tests/lit.common.cfg:17
@@ -24,1 +16,3 @@
+lsan_lit_src_root = get_required_attr(config, 'lsan_lit_src_root')
+config.test_source_root = os.path.join(lsan_lit_src_root, 'Tests')
----------------
WDYT of s/Tests/common ?
http://llvm-reviews.chandlerc.com/D895
More information about the llvm-commits
mailing list