[PATCH] Adding experimental build support for building compiler-rt for iOS.

[Alexey Samsonov]

I support the direction of this patch, but think it needs to be somewhat polished.
Most importantly, I'd suggest to split it into several smaller patches, that would be either to digest, review, and eventually land.
For example, substantial part of this changes is refactoring (introducing CompilerRTDarwinUtils, moving some functions there,
factoring out common flags to DARWIN_COMMON_CFLAGS, etc.), that can be committed first.

More comments inline.


================
Comment at: CMakeLists.txt:300
@@ +299,3 @@
+  set(DARWIN_COMMON_LINKFLAGS
+    ${DARWIN_COMMON_CFLAGS}
+    -lc++
----------------
This looks weird. It's just a coincidence that -stdlib=libc++ is both linker and compile flag. Not all compiler flags might be passed here.

================
Comment at: CMakeLists.txt:307
@@ -316,1 +306,3 @@
+    -mmacosx-version-min=${SANITIZER_MIN_OSX_VERSION})
   set(DARWIN_osx_LINKFLAGS -mmacosx-version-min=${SANITIZER_MIN_OSX_VERSION}
+    ${DARWIN_COMMON_LINKFLAGS})
----------------
Keep DARWIN_COMMON_LINKFLAGS first for consistency.

================
Comment at: CMakeLists.txt:320
@@ -323,1 +319,3 @@
 
+  set(DARWIN_ios_CFLAGS
+    ${DARWIN_COMMON_CFLAGS}
----------------
Do you actually need to do all this setup if IOSSIM_SDK_DIR is not specified? It would make sense to group flag setup and `darwin_test_archs` invocation together.

================
Comment at: CMakeLists.txt:335
@@ +334,3 @@
+
+  message(STATUS "Toolchain supported arches: ${toolchain_arches}")
+
----------------
Print them after you calculated them?

================
Comment at: CMakeLists.txt:361
@@ +360,3 @@
+
+  set(CMAKE_OSX_DEPLOYMENT_TARGET "") # We're setting the flag manually below.
+
----------------
This is confusing, we don't set anything below.

================
Comment at: cmake/Modules/AddCompilerRT.cmake:23
@@ -22,1 +22,3 @@
+      set(extra_linkflags_${libname} ${DARWIN_${os}_LINKFLAGS})
+      set(LIB_ARCHS_${libname} ${SANITIZER_COMMON_DARWIN_${os}_ARCHES})
     endforeach()
----------------
This is really confusing now - you never use `ARCHS` input list on Darwin at all. The user should have the ability to restrict the set of architectures she targets - this is exactly the reason why we have `ASAN_SUPPORTED_ARCH`, `UBSAN_SUPPORTED_ARCH` and so on. There is no guarantee that we would be able to build ASan runtime for all architectures that our host toolchain can target for OS.

At the very least it means that you should calculate the intersection of `${LIB_ARCHS}` and `${SANITIZER_COMMON_DARWIN_${os}_ARCHES}`. But now all this gets complicated - different architectures are available on different OSes, so it turns out that the input parameters of modified `add_compiler_rt_object_libraries` don't work well. I see no "easy" fix for that - probably we should start to make complex intrusive changes and move from keeping the list of "supported architectures" (COMPILER_RT_SUPPORTED_ARCH) and "supported OSes" (SANITIZER_COMMON_SUPPORTED_OS) to a list of "supported triples"
(well, in fact, OS-arch pairs).






================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:27
@@ +26,3 @@
+function(darwin_get_toolchain_supported_archs output_var)
+  execute_process(
+    COMMAND ld -v
----------------
What if `ld -v` failed?

================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:31
@@ +30,3 @@
+
+  string(FIND ${LINKER_VERSION} "configured to support archs:" LINE_START)
+  string(SUBSTRING ${LINKER_VERSION} ${LINE_START} -1 ARCHES_LINE)
----------------
I think it's easier to scrape this information with regexp matching. For example, see the calculation of `SANITIZER_MIN_OSX_VERSION` from `-mmacosx-version-min=` in CMakeLists.txt. Then you will also be able to detect an error when you're unable to find `configured to support archs:` substring.

================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:33
@@ +32,3 @@
+  string(SUBSTRING ${LINKER_VERSION} ${LINE_START} -1 ARCHES_LINE)
+  string(SUBSTRING ${ARCHES_LINE} 28 -1 ARCHES_LINE)
+  string(FIND ${ARCHES_LINE} "\n" LINE_END)
----------------
Why do you refer to the number of convex uniform honeycombs here?

================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:65
@@ +64,3 @@
+    set(arch_linker_flags "-arch ${arch} ${os_linker_flags}")
+    try_compile(CAN_TARGET_${os}_${arch} ${CMAKE_BINARY_DIR} ${SIMPLE_CPP}
+                COMPILE_DEFINITIONS "-v -arch ${arch}" ${DARWIN_${os}_CFLAGS}
----------------
Some of this code duplicates logic from `cmake/config-ix.cmake` that calculates COMPILER_RT_SUPPORTED_ARCH. I think you'd need to use the latter variable on Darwin (although that's tricky - see rant about triples above).

================
Comment at: lib/asan/CMakeLists.txt:116
@@ -115,3 +115,3 @@
     add_compiler_rt_darwin_dynamic_runtime(clang_rt.asan_${os}_dynamic ${os}
-      ARCHS ${ASAN_SUPPORTED_ARCH}
+      ARCHS ${SANITIZER_COMMON_DARWIN_${os}_ARCHES}
       SOURCES $<TARGET_OBJECTS:RTAsan.${os}>
----------------
See note above.

http://reviews.llvm.org/D10710

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/