[PATCH] Android test runner for ASan lit tests

[Evgeniy Stepanov]

================
Comment at: CMakeLists.txt:99
@@ +98,3 @@
+  test_target_arch(arm_android ${TARGET_ARM_ANDROID_CFLAGS})
+else()
+  if("${LLVM_NATIVE_ARCH}" STREQUAL "X86")
----------------
Alexey Samsonov wrote:
> Why do you need "else" here? i.e. why can't we target x86_64 and arm_android simultaneously?
Because we are not ready to handle the situation where we target both x86 and arm in one run. See SANITIZER_COMMON_LIT_TEST_DEPS for example. And we don't need to, in the long term.

================
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()
----------------
Alexey Samsonov wrote:
> 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.
done

================
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)
----------------
Alexey Samsonov wrote:
> 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.
We also would need it in lib/asan/tests/CMakeLists.txt.
And this leaves only sanitizer_common tests, because everything else is excluded on Android. I'd prefer to keep it here.

================
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
----------------
Alexey Samsonov wrote:
> Please explain why you are doing this in a comment.
done

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

================
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")
----------------
Alexey Samsonov wrote:
> s/TEST_CONFIG_NAME/TEST_CONFIG_SUFFIX
done

================
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:
> Alexey Samsonov wrote:
> > I think for now it's better to s/SANITIZER_/ASAN_ in variable names to avoid confusion.
> config.name_suffix = "..."
done

================
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@"
----------------
Evgeniy Stepanov wrote:
> Alexey Samsonov wrote:
> > Alexey Samsonov wrote:
> > > I think for now it's better to s/SANITIZER_/ASAN_ in variable names to avoid confusion.
> > config.name_suffix = "..."
> done
done

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


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