[llvm-dev] de-posixifying list tests?

[Rafael Espíndola via llvm-dev]

On 30 March 2017 at 11:38, Kostya Serebryany <kcc at google.com> wrote:
> Rafael, Filipe,
>
> I am looking at the fixes you apply to sanitizer tests and they worry me.
> (e.g. https://reviews.llvm.org/D31498)
> The fixes are mostly mechanical and thus every single change looks safe,
> but given the amount of changes there is large risk to cripple some of the
> tests
> in a way that they will stop detecting failures.
>
> When I write a test for new functionality, I always verify that the test
> fails w/o the implementation
> of that functionality (i.e. that the test catches the failure). I hope most
> of the tests are developed this way too. But when you modify the tests, I
> bet you only verify that the passing test is still passing. Am I right? If
> yes, this is not the way to modify tests, unfortunately.

It is probably not as comprehensive and testing with the functionality
disabled, but I do tent to try to break the test locally. For example,
when changing from [] to count I will check that the test does catch
having the wrong count.

> So, my question is: did you consider modifying the environment to closer
> match what already works well on posix, instead of massively changing tests?

Yes, unfortunately there is no good way to do that as far as I can
tell. Different posix shells on windows have different oddities and
they break from time to time (which is probably why we have an
internal shell in the first place).

Currently the only big feature that I am changing tests for is
avoiding a subshell. The rest I seem to be able to implement in the
internal shell. I have just started sending the shell patches up for
review.

The difference to the posix tests are fairly small. I have attached a
diff showing all that is left from current trunk.

Cheers,
Rafael
-------------- next part --------------
diff --git a/compiler-rt/test/asan/TestCases/Posix/closed-fds.cc b/compiler-rt/test/asan/TestCases/Posix/closed-fds.cc
index b7bca26..75e3216a 100644
--- a/compiler-rt/test/asan/TestCases/Posix/closed-fds.cc
+++ b/compiler-rt/test/asan/TestCases/Posix/closed-fds.cc
@@ -2,7 +2,8 @@
 // symbolizer still works.
 
 // RUN: rm -f %t.log.*
-// RUN: %clangxx_asan -O0 %s -o %t 2>&1 && %env_asan_opts=log_path='"%t.log"':verbosity=2 not %run %t 2>&1
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %env_asan_opts=log_path='"%t.log"':verbosity=2 not %run %t
 // RUN: FileCheck %s --check-prefix=CHECK-FILE < %t.log.*
 
 // FIXME: copy %t.log back from the device and re-enable on Android.
diff --git a/compiler-rt/test/asan/TestCases/Posix/coverage-fork-direct.cc b/compiler-rt/test/asan/TestCases/Posix/coverage-fork-direct.cc
index c196719..dec7825 100644
--- a/compiler-rt/test/asan/TestCases/Posix/coverage-fork-direct.cc
+++ b/compiler-rt/test/asan/TestCases/Posix/coverage-fork-direct.cc
@@ -1,8 +1,10 @@
 // RUN: %clangxx_asan -fsanitize-coverage=func %s -o %t
 // RUN: rm -rf %T/coverage-fork-direct
 // RUN: mkdir -p %T/coverage-fork-direct && cd %T/coverage-fork-direct
-// RUN: (%env_asan_opts=coverage=1:coverage_direct=1:verbosity=1 %run %t; \
-// RUN:  %sancov rawunpack *.sancov.raw; %sancov print *.sancov) 2>&1
+// RUN: %env_asan_opts=coverage=1:coverage_direct=1:verbosity=1 %run %t > %t.log 2>&1
+// RUN: %sancov rawunpack *.sancov.raw
+// RUN: %sancov print *.sancov >> %t.log 2>&1
+// RUN: FileCheck %s < %t.log
 //
 // XFAIL: android
 
@@ -34,5 +36,4 @@ int main(int argc, char **argv) {
 
 // CHECK-DAG: Child PID: [[ChildPID:[0-9]+]]
 // CHECK-DAG: Parent PID: [[ParentPID:[0-9]+]]
-// CHECK-DAG: read 3 PCs from {{.*}}.[[ParentPID]].sancov
-// CHECK-DAG: read 1 PCs from {{.*}}.[[ChildPID]].sancov
+// CHECK: read 3 64-bit PCs from {{.*}}.[[ParentPID]].sancov
diff --git a/compiler-rt/test/asan/TestCases/Posix/coverage-sandboxing.cc b/compiler-rt/test/asan/TestCases/Posix/coverage-sandboxing.cc
index c4e6bc7..431dce1 100644
--- a/compiler-rt/test/asan/TestCases/Posix/coverage-sandboxing.cc
+++ b/compiler-rt/test/asan/TestCases/Posix/coverage-sandboxing.cc
@@ -12,9 +12,9 @@
 // RUN: %env_asan_opts=coverage=1:verbosity=1 %run %t a b 2>&1 | FileCheck %s --check-prefix=CHECK-sandbox
 // RUN: %sancov unpack coverage_sandboxing_test.sancov.packed
 // RUN: cd ..
-// RUN: %sancov print vanilla/`basename %dynamiclib`*.sancov > vanilla.txt
-// RUN: %sancov print sandbox1/`basename %dynamiclib`*.sancov > sandbox1.txt
-// RUN: %sancov print sandbox2/`basename %dynamiclib`*.sancov > sandbox2.txt
+// RUN: %sancov print vanilla/%xdynamiclib_filename*.sancov > vanilla.txt
+// RUN: %sancov print sandbox1/%xdynamiclib_filename*.sancov > sandbox1.txt
+// RUN: %sancov print sandbox2/%xdynamiclib_filename*.sancov > sandbox2.txt
 // RUN: diff vanilla.txt sandbox1.txt
 // RUN: diff vanilla.txt sandbox2.txt
 // RUN: rm -r %T/coverage_sandboxing_test
diff --git a/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cc b/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cc
index 18ba563..2d2b056 100644
--- a/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cc
+++ b/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cc
@@ -1,6 +1,7 @@
 // Check that UAR mode can handle very deep recusrion.
-// RUN: %clangxx_asan -O2 %s -o %t && \
-// RUN:   (ulimit -s 4096; %env_asan_opts=detect_stack_use_after_return=1 %run %t) 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -O2 %s -o %t
+// RUN: ulimit -s 4096
+// RUN: %env_asan_opts=detect_stack_use_after_return=1 %run %t 2>&1 | FileCheck %s
 
 // Also check that use_sigaltstack+verbosity doesn't crash.
 // RUN: %env_asan_opts=verbosity=1:use_sigaltstack=1:detect_stack_use_after_return=1 %run %t  | FileCheck %s