[PATCH] [lsan] Convert all LSan tests to LIT tests. Remove the old testing interface.

Kostya Serebryany kcc at google.com
Fri May 24 02:01:24 PDT 2013


  Add one-line comments on the top of lit tests explaining what the test is doing.


================
Comment at: lib/lsan/lit_tests/large_allocation_leak.cc:3
@@ +2,3 @@
+// RUN: %clangxx_lsan %s -o %t
+// RUN: LSAN_OPTIONS=$LSAN_BASE:"use_globals=0" %t 2>&1 | FileCheck --check-prefix=CHECK-leak %s
+// RUN: LSAN_OPTIONS=$LSAN_BASE:"use_globals=1" %t 2>&1 | FileCheck --check-prefix=CHECK-noleak %s
----------------
I think you should also run with default options and check for leaks, 
i.e. add 3-rd line to ever test (where applicable) with empty LSAN_OPTIONS

================
Comment at: lib/lsan/lit_tests/stale_stack_leak.cc:30
@@ +29,3 @@
+
+// Hopefully this will run after LSan.
+__attribute__((destructor))
----------------
Hm?


================
Comment at: lib/lsan/lit_tests/stale_stack_leak.cc:15
@@ +14,3 @@
+// Hopefully the argument p will be passed on a register, saving us from false
+// positives.
+__attribute__((noinline))
----------------
I don't understand this comment? 
did you mean False negatives? 

================
Comment at: lib/lsan/lit_tests/test_defs.h:2
@@ +1,3 @@
+#ifndef TEST_DEFS_H
+#define TEST_DEFS_H
+
----------------
I'd really prefer not to have this common file. 
Standalone .cc test files have their own coolness. 

================
Comment at: lib/lsan/lit_tests/use_globals_initialized.cc:20
@@ +19,2 @@
+// CHECK-leak: Directly leaked 1337 byte block at [[ADDR]]
+// CHECK-noleak-NOT: block at [[ADDR]]
----------------
here and in all other tests: 
the test should not have *any* leak, not just a leak on ADDR, so 
replace this with something like 
  CHECK-noleak-NOT: block at
or  
  CHECK-noleak-NOT: LeakSanitizer

================
Comment at: lib/lsan/lit_tests/use_stacks_threaded.cc:30
@@ +29,3 @@
+    pthread_yield();
+  exit(0);
+  return 0;
----------------
exit(0) followed by return 0 ? 

================
Comment at: lib/lsan/lit_tests/use_tls_dynamic.cc:2
@@ +1,3 @@
+// RUN: LSAN_BASE="report_blocks=1:use_stacks=0:use_registers=0"
+// RUN: %clangxx_lsan -fno-sanitize=leak -fno-sanitize=address %p/SharedLibs/huge_tls_lib_so.cc -fPIC -shared -o %t-so.so
+// RUN: %clangxx_lsan %s -o %t
----------------
Should this be something similar, like clangxx ? 


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



More information about the llvm-commits mailing list