[PATCH] D15626: Add iOS/watchOS/tvOS support for ASan (compiler-rt part)

[Alexey Samsonov via llvm-commits]

samsonov added a comment.

Looks pretty good.


================
Comment at: cmake/config-ix.cmake:313
@@ -312,3 +312,3 @@
 
-  option(COMPILER_RT_ENABLE_IOS "Enable building for iOS - Experimental" Off)
+  option(COMPILER_RT_ENABLE_IOS "Enable building for iOS" On)
   option(COMPILER_RT_ENABLE_WATCHOS "Enable building for watchOS - Experimental" Off)
----------------
Please submit this small change (switching the default) as a separate commit, so that it would be easy revert if iOS build won't work on some setups.

================
Comment at: lib/asan/asan_internal.h:39
@@ -38,3 +38,3 @@
 #ifndef ASAN_LOW_MEMORY
-#if SANITIZER_WORDSIZE == 32
+# if SANITIZER_IOS
 #  define ASAN_LOW_MEMORY 1
----------------
```
# if SANITIZER_IOS || (SANITIZER_WORDSIZE == 32)
```

================
Comment at: lib/asan/asan_mapping.h:148
@@ -147,3 +147,3 @@
 #  endif
-#else
-#  if defined(__aarch64__)
+# else
+#  if SANITIZER_IOS
----------------
Please restore the indentation.

================
Comment at: lib/asan/asan_mapping.h:150
@@ +149,3 @@
+#  if SANITIZER_IOS
+#    if SANITIZER_IOSSIM
+#      define SHADOW_OFFSET kIosSimShadowOffset64
----------------
Makes sense, let's fix 32-bit variant accordingly then.

================
Comment at: lib/asan/asan_mapping.h:165
@@ -158,3 +164,3 @@
 #   define SHADOW_OFFSET kMIPS64_ShadowOffset64
 #  elif SANITIZER_IOSSIM
 #    define SHADOW_OFFSET kIosSimShadowOffset64
----------------
Do you need to remove these now?

================
Comment at: lib/sanitizer_common/sanitizer_mac.cc:361
@@ +360,3 @@
+bool IsHandledDeadlySignal(int signum) {
+#if (SANITIZER_WATCHOS || SANITIZER_TVOS) && !SANITIZER_IOSSIM
+  // Handling fatal signals on watchOS and tvOS devices is disallowed.
----------------
Please make it a plain `if()` instead of `#if`?

================
Comment at: lib/sanitizer_common/sanitizer_platform.h:57
@@ -44,3 +56,3 @@
 #  define SANITIZER_IOSSIM 0
 # endif
 #else
----------------
Suggestion: write this as
# if TARGET_OS_IPHONE && TARGET_OS_WATCH
#  define SANITIZER_WATCHOS 1
# else
#  define SANITIZER_WATCHOS 0
# endif
and the same for SANITIZER_TVOS, to minimize the places we define this macro at.


http://reviews.llvm.org/D15626