[PATCH] Add CMake option LLVM_USE_SANITIZER to allow bootstrap of LLVM/Clang under ASan/MSan.

[Alexey Samsonov]

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:228-230
@@ +227,5 @@
+function(append_common_sanitizer_flags)
+  if(NOT LLVM_ENABLE_PIC)
+    message(WARNING "LLVM_USE_SANITIZER can only be used with LLVM_ENABLE_PIC")
+  endif()
+  # Append -fno-omit-frame-pointer and -g to get better stack traces.
----------------
Chandler Carruth wrote:
> Shouldn't the sanitizer flag imply this?
I won't submit this patch before D502 comes in (which should take care of this). Removed this check.

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:245-246
@@ +244,4 @@
+    add_flag_or_print_warning("-fsanitize=memory")
+    # -pie is required for MSan.
+    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -pie")
+    if(uppercase_LLVM_USE_SANITIZER STREQUAL "MEMORYWITHORIGINS")
----------------
Chandler Carruth wrote:
> MSan should handle this automatically then. Users shouldn't have to remember to set it.
D502 will take care of this.

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:239
@@ +238,3 @@
+  string(TOUPPER "${LLVM_USE_SANITIZER}" uppercase_LLVM_USE_SANITIZER)
+  if (uppercase_LLVM_USE_SANITIZER STREQUAL "ADDRESS")
+    append_common_sanitizer_flags()
----------------
Chandler Carruth wrote:
> Ugh, this seems a weird way to check the option. Is there not a multiple-choice option mechanism that makes this easier?
Kinda ugly. I looked at how CMAKE_BUILD_TYPE var is handled. I think I will just fix a single capitalization.

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:247-249
@@ +246,5 @@
+    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -pie")
+    if(uppercase_LLVM_USE_SANITIZER STREQUAL "MEMORYWITHORIGINS")
+      add_flag_or_print_warning("-fsanitize-memory-track-origins")
+    endif()
+  endif()
----------------
Chandler Carruth wrote:
> I would just make this a separate option at the CMake level: LLVM_SANITIZE_MEMORY_TRACK_ORIGINS or some such.
Hm, then this option would make no sense if LLVM_USE_SANITIZER is not provided or isn't equal to "Memory". I'd prefer to fix a few predefined configurations and name them like we do with CMAKE_BUILD_TYPE (Release, Debug, RelWithDebInfo, MinSizeRel).

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:236-237
@@ +235,4 @@
+
+if (LLVM_ON_UNIX)
+  # Turn on sanitizers if necessary.
+  string(TOUPPER "${LLVM_USE_SANITIZER}" uppercase_LLVM_USE_SANITIZER)
----------------
Chandler Carruth wrote:
> My inclination is to warn if the sanitizer string isn't empty and we're on a non-unix platform. Better to tell the user they aren't getting the sanitizer they asked for...
Done

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:231-233
@@ +230,5 @@
+  endif()
+  # Append -fno-omit-frame-pointer and -g to get better stack traces.
+  add_flag_if_supported("-fno-omit-frame-pointer")
+  add_flag_if_supported("-g")
+endfunction()
----------------
Chandler Carruth wrote:
> It would seem better to set the baseline at '-gline-tables-only'?
> 
> Is there a way to ensure this doesn't conflict with '-g' options passed later by the user?
> 
> Finally, shouldn't -fsanitize=... imply these by default? (They could still be disabled explicitly if they cause problems for some users...)
1. Done
2. Well, we can consult CMAKE_BUILD_TYPE to check if debug info will be enabled automatically. I'd like to avoid parsing CMAKE_CXX_FLAGS here. There are not only -g or -g0 but stuff like -ggdb here, gr-r.
3. Hm, these flags are not necessary for sanitizer functionality, and increase the binary size... IIRC some tools have problems parsing debug info produced by -gline-tables-only, so I'd avoid always turning them on for now.


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