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

Chandler Carruth chandlerc at gmail.com
Tue Mar 19 02:45:18 PDT 2013


  Sorry for not seeing this for so long...


================
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.
----------------
Shouldn't the sanitizer flag imply this?

================
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()
----------------
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...)

================
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)
----------------
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...

================
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")
----------------
MSan should handle this automatically then. Users shouldn't have to remember to set it.

================
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()
----------------
I would just make this a separate option at the CMake level: LLVM_SANITIZE_MEMORY_TRACK_ORIGINS or some such.

================
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()
----------------
Ugh, this seems a weird way to check the option. Is there not a multiple-choice option mechanism that makes this easier?


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



More information about the llvm-commits mailing list