[PATCH] Android test runner for ASan lit tests

[Alexey Samsonov]

  What a hack! :) Still, it's pretty cool you've found a reasonable way to run tests on Android device with so little changes, so I support this change (module few nits below).
  In the longer term, we should really add support for special commands we can use in the RUN: lines, like
  "compile" - where we can configure the "compiler" binary or script and provide arguments to it (like target_cflags in your patch).
  "execute" - run a given binary, which may be simply executing a binary, if the host and target arches are the same, or running some script, as you do for Android.


================
Comment at: CMakeLists.txt:99
@@ +98,3 @@
+  test_target_arch(arm_android ${TARGET_ARM_ANDROID_CFLAGS})
+else()
+  if("${LLVM_NATIVE_ARCH}" STREQUAL "X86")
----------------
Why do you need "else" here? i.e. why can't we target x86_64 and arm_android simultaneously?

================
Comment at: CMakeLists.txt:98
@@ +97,3 @@
+  set(TARGET_ARM_ANDROID_CFLAGS "${ANDROID_COMMON_FLAGS}")
+  test_target_arch(arm_android ${TARGET_ARM_ANDROID_CFLAGS})
+else()
----------------
You can avoid creating TARGET_ARM_ANDROID_CFLAGS variable, and instead get it as:
  get_target_flags_for_arch(arm_android, ...)
which would also check that arm_android is indeed supported.

================
Comment at: CMakeLists.txt:115
@@ -111,1 +114,3 @@
+# cross-compiling.
+if(("${CMAKE_HOST_SYSTEM}" STREQUAL "${CMAKE_SYSTEM}" AND UNIX) OR ANDROID)
   option(COMPILER_RT_CAN_EXECUTE_TESTS "Can we execute instrumented tests" ON)
----------------
Can you add instead add this condition to lib/asan/lit_tests/CMakeLists.txt?
I don't think it any other lit tests would work on Android.

================
Comment at: lib/asan/lit_tests/lit.cfg:66
@@ +65,3 @@
+if config.android == "TRUE":
+  clang_wrapper = config.compiler_rt_src_root + "/lib/asan/lit_tests/android_compile.py "
+else:
----------------
use config.asan_source_dir here.

================
Comment at: lib/asan/lit_tests/GenericConfig/lit.site.cfg.in:5
@@ +4,3 @@
+# Tool-specific config options.
+config.name = "@SANITIZER_TEST_CONFIG_NAME@"
+config.asan_source_dir = "@ASAN_SOURCE_DIR@"
----------------
I think for now it's better to s/SANITIZER_/ASAN_ in variable names to avoid confusion.

================
Comment at: lib/asan/lit_tests/CMakeLists.txt:14
@@ +13,3 @@
+
+  if(CAN_TARGET_arm_android)
+    set(SANITIZER_TEST_CONFIG_NAME "-arm-android")
----------------
merge with previous if()-endif() block

================
Comment at: lib/asan/lit_tests/CMakeLists.txt:15
@@ +14,3 @@
+  if(CAN_TARGET_arm_android)
+    set(SANITIZER_TEST_CONFIG_NAME "-arm-android")
+    set(SANITIZER_TEST_BITS "32")
----------------
s/TEST_CONFIG_NAME/TEST_CONFIG_SUFFIX

================
Comment at: lib/asan/lit_tests/GenericConfig/lit.site.cfg.in:5
@@ +4,3 @@
+# Tool-specific config options.
+config.name = "@SANITIZER_TEST_CONFIG_NAME@"
+config.asan_source_dir = "@ASAN_SOURCE_DIR@"
----------------
Alexey Samsonov wrote:
> I think for now it's better to s/SANITIZER_/ASAN_ in variable names to avoid confusion.
config.name_suffix = "..."

================
Comment at: lib/asan/lit_tests/android_common.py:1
@@ +1,2 @@
+import os, subprocess, tempfile
+import time
----------------
Consider moving these files to a subdir like lib/asan/lit_tests/android_commands or some such.

================
Comment at: CMakeLists.txt:230
@@ -224,7 +229,3 @@
 
-set(SANITIZER_COMMON_LIT_TEST_DEPS
-  clang clang-headers FileCheck count not llvm-nm llvm-symbolizer
-  compiler-rt-headers)
-# Check code style when running lit tests for sanitizers.
-if(UNIX)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS SanitizerLintCheck)
+if (NOT ANDROID)
+  set(SANITIZER_COMMON_LIT_TEST_DEPS
----------------
Please explain why you are doing this in a comment.


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