[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