[PATCH] [sanitizer] Android build cleanup.

[Evgeniy Stepanov]

================
Comment at: cmake/config-ix.cmake:95
@@ +94,3 @@
+
+macro(detect_target_arch)
+  check_symbol_exists(__arm__ "" __ARM)
----------------
glider wrote:
> "detect_target_arch" looks much like "test_target_arch" below, but it populates COMPILER_RT_SUPPORTED_ARCH with several arches instead of only one. I think we need a better name for "detect_target_arch"
Note the elseif()s. It's at most one arch.
Can you suggest a better name? IMO it is quite good - the function examines compiler output with default flags and detect the target architecture.

================
Comment at: cmake/config-ix.cmake:122
@@ -92,1 +121,3 @@
+  detect_target_arch()
+  set(COMPILER_RT_ARCH_SUFFIX "-android")
 else()
----------------
glider wrote:
> Is this an arch suffix or an OS suffix? Should the suffix be present on Linux or other OSes?
It's the suffix in libclang_rt.asan-${arch}-   <<here>> .so
Renamed to OS_SUFFIX.


================
Comment at: lib/asan/scripts/asan_device_setup:110
@@ -109,3 +109,3 @@
     # We could be in the toolchain's base directory.
-    # Consider ../lib, ../lib/asan and ../lib/clang/$VERSION/lib/linux.
-    P=$(ls "$HERE"/../lib/"$ASAN_RT" "$HERE"/../lib/asan/"$ASAN_RT" "$HERE"/../lib/clang/*/lib/linux/"$ASAN_RT" 2>/dev/null | sort | tail -1)
+    # Consider ../lib, ../lib/asan, ../lib/linux and ../lib/clang/$VERSION/lib/linux.
+    P=$(ls "$HERE"/../lib/"$ASAN_RT" "$HERE"/../lib/asan/"$ASAN_RT" "$HERE"/../lib/linux/"$ASAN_RT" "$HERE"/../lib/clang/*/lib/linux/"$ASAN_RT" 2>/dev/null | sort | tail -1)
----------------
glider wrote:
> So the android runtime is in lib/linux? Why?
because it's arm-linux-androideabi, so the runtime goes to lib/linux/libclang_rt.asan-arm-android.so

================
Comment at: lib/asan/tests/CMakeLists.txt:78
@@ -77,4 +77,3 @@
 # on Clang driver behavior, passing -fsanitize=address flag.
-if(NOT ANDROID)
-  list(APPEND ASAN_UNITTEST_INSTRUMENTED_LINKFLAGS -fsanitize=address)
-endif()
+# if(NOT ANDROID)
+list(APPEND ASAN_UNITTEST_INSTRUMENTED_LINKFLAGS -fsanitize=address)
----------------
glider wrote:
> Please remove the commented lines (if you meant to) and fix indentation.
> Do you still need the comment above now?
done
the comment is no longer true, removed

================
Comment at: lib/sanitizer_common/tests/CMakeLists.txt:62
@@ +61,3 @@
+if(ANDROID)
+  # A bug in NDK r10 requires -latomic almost always.
+  list(APPEND SANITIZER_TEST_LINK_LIBS log atomic)
----------------
glider wrote:
> Link to the bug? (and in other places as well)
There is no bug, just a bunch of mail threads.

================
Comment at: test/asan/CMakeLists.txt:10
@@ +9,3 @@
+    set(bits 32)
+  else()
+    set(bits 64)
----------------
glider wrote:
> You'd better list the possible 64-bit arches here to make sure we bail out if an unsupported arch comes around.
done

================
Comment at: test/asan/CMakeLists.txt:34
@@ -21,1 +33,3 @@
+
+else() # Not Android
 
----------------
glider wrote:
> Two spaces before the comment, please
done

================
Comment at: test/asan/CMakeLists.txt:36
@@ -21,3 +35,3 @@
 
 if(CAN_TARGET_arm)
   # This is only true if we are cross-compiling.
----------------
glider wrote:
> Pls indent this block.
done

================
Comment at: test/asan/lit.cfg:58
@@ -57,3 +57,3 @@
 asan_lit_source_dir = get_required_attr(config, "asan_lit_source_dir")
-if config.android == "TRUE":
+if config.android == "1":
   config.available_features.add('android')
----------------
glider wrote:
> Are changes to this file related to the CMake changes? If no, can they be landed separately?
Yes. No.
CAN_TARGET_arm_android used to be "TRUE"
Now ANDROID is "1".

http://reviews.llvm.org/D5467