[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