[PATCH] [lsan] Run the leak checking tests under both ASan and LSan.

[Sergey Matveev]

================
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()
----------------
Alexey Samsonov wrote:
> No, this is Linux-only. I think we had some variable like ASAN_RUNTIME_LIBRARIES.
We have a variable like that in ASan cmakelists, but it's undefined in this scope. But we don't need this on non-Linux targets anyway, so fixed.



================
Comment at: lib/lsan/lit_tests/ASan/lit.cfg:1
@@ +1,2 @@
+# -*- Python -*-
+
----------------
Alexey Samsonov wrote:
> Let's stick to lowercase directory names. ASan->asan
There seems to be a convention of camelcasing lit_test subdirectories in sanitizer tools. Do you propose to put everything in lower case, or just asan/lsan/common?

================
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)
----------------
Alexey Samsonov wrote:
> lsan_lit_cfg can't be None here, I presume
fixed

================
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)
----------------
Alexey Samsonov wrote:
> lsan_lit_cfg is not None here
fixed

================
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')
 
----------------
Alexey Samsonov wrote:
> WDYT of s/Tests/common ?
I've thought about that and I have an issue with it. It gives the impression that tool-specific tests go under asan/ or lsan/, and common tests go under common/. But that is false - you can't put tests under asan/ or lsan/. If we want some tests to run under standalone LSan only (and we do), we have two options. One is to do a bit of config magic and put them under a subdirectory such as common/lsan/. But then common is not common anymore. Another is to have a separate test suite in lit_tests/. But there's already a lit_tests/lsan/, so how do we come up with a name that means basically the same thing but is different?

================
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 "
----------------
Alexey Samsonov wrote:
> remove "*"
fixed


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