Address review comments

Make sanitizer options lower case: none, asan, ubsan, tsan, msan Check if sanitizer option is not valid Remove debug mode restriction for asan Run in RelWithDebInfo mode in GitHub Actions CI Clean up previous LLVM sanitizers implementation
williamfgc · Mar 24, 2021 · 54d3962 · 54d3962
1 parent c7fe1bc
commit 54d3962
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 48 deletions.
diff --git a/.github/workflows/ci-github-actions.yaml b/.github/workflows/ci-github-actions.yaml
@@ -52,25 +52,14 @@ jobs:
       fail-fast: false
       matrix:
         jobname: [
-          clang-openmpi-asan,  # address sanitizer (including leaks)
           clang-openmpi-ubsan, # undefined behavior sanitizer
-          clang-openmpi-tsan,  # thread race sanitizer (expensive)
         ]
         include:
-        - jobname: clang-openmpi-asan
-          container: 
-            image: williamfgc/qmcpack-ci:ubuntu20-openmpi
-            options: -u 1001
 
         - jobname: clang-openmpi-ubsan
           container: 
             image: williamfgc/qmcpack-ci:ubuntu20-openmpi
             options: -u 1001
-
-        - jobname: clang-openmpi-tsan
-          container: 
-            image: williamfgc/qmcpack-ci:ubuntu20-openmpi
-            options: -u 1001
 
     steps:
     - name: Checkout Action

diff --git a/CMake/ClangCompilers.cmake b/CMake/ClangCompilers.cmake
@@ -141,23 +141,3 @@ IF(XRAY_PROFILE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${XRAY_FLAGS}")
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${XRAY_FLAGS}")
 ENDIF(XRAY_PROFILE)
-
-SET(LLVM_SANITIZE_ADDRESS FALSE CACHE BOOL "Use llvm address sanitizer library")
-MARK_AS_ADVANCED(LLVM_SANITIZE_ADDRESS)
-IF(LLVM_SANITIZE_ADDRESS)
-  SET(CMAKE_C_FLAGS "-fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope ${CMAKE_C_FLAGS}")
-  SET(CMAKE_CXX_FLAGS "-fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope ${CMAKE_CXX_FLAGS}")
-  SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope")
-  SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope")
-ENDIF(LLVM_SANITIZE_ADDRESS)
-
-# Don't expect this to be useful unless you have msan instrumented all libraries
-SET(LLVM_SANITIZE_MEMORY FALSE CACHE BOOL "Use llvm memory sanitizer library")
-MARK_AS_ADVANCED(LLVM_SANITIZE_MEMORY)
-IF(LLVM_SANITIZE_MEMORY)
-  SET(LLVM_BLACKLIST_SANITIZE_MEMORY "-fsanitize-blacklist=${PROJECT_SOURCE_DIR}/llvm_misc/memory_sanitizer_blacklist.txt")
-  SET(CMAKE_C_FLAGS_DEBUG "-fsanitize=memory ${LLVM_BLACKLIST_SANITIZE_MEMORY} ${CMAKE_C_FLAGS_DEBUG}")
-  SET(CMAKE_CXX_FLAGS_DEBUG "-fsanitize=memory ${LLVM_BLACKLIST_SANITIZE_MEMORY} ${CMAKE_CXX_FLAGS_DEBUG}")
-  SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=memory ${LLVM_BLACKLIST_SANITIZE_MEMORY}")
-  SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=memory ${LLVM_BLACKLIST_SANITIZE_MEMORY}")
-ENDIF(LLVM_SANITIZE_MEMORY)
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -272,16 +272,15 @@ CMAKE_DEPENDENT_OPTION(USE_NVTX_API "Enable/disable NVTX regions in CUDA code."
 ######################################################################
 
 # Add optional sanitizers ASAN, UBSAN, MSAN
-SET(ENABLE_SANITIZER "NONE" CACHE STRING "NONE,ASAN,UBSAN,MSAN")
-SET_PROPERTY(CACHE ENABLE_SANITIZER PROPERTY STRINGS "NONE" "ASAN" "UBSAN" "MSAN")
+SET(VALID_SANITIZERS "none" "asan" "ubsan" "tsan" "msan")
+SET(ENABLE_SANITIZER "none" CACHE STRING "none,asan,ubsan,tsan,msan")
+SET_PROPERTY(CACHE ENABLE_SANITIZER PROPERTY STRINGS ${VALID_SANITIZERS})
 
 # Perform sanitizer option check, only works in debug mode
-IF( NOT "${ENABLE_SANITIZER}" STREQUAL "NONE")
+IF(NOT ENABLE_SANITIZER IN_LIST VALID_SANITIZERS)
+  MESSAGE( FATAL_ERROR "Invalid -DENABLE_SANITIZER=${ENABLE_SANITIZER}, value must be one of ${VALID_SANITIZERS}")
+ELSE()
   MESSAGE( STATUS "Enable sanitizer ENABLE_SANITIZER=${ENABLE_SANITIZER}" )
-  IF( "${ENABLE_SANITIZER}" STREQUAL "ASAN" AND NOT "${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG" )
-    # better to fail than override debug mode
-    MESSAGE( FATAL_ERROR "-DENABLE_SANITIZER=ASAN can only be enabled with -DCMAKE_BUILD_TYPE=Debug" )
-  ENDIF()
 ENDIF()
 
 ######################################################################
@@ -883,21 +882,21 @@ MESSAGE(STATUS "GCOV is enabled")
 ENDIF()
 
 # SETUP SANITIZERS FLAGS
-IF( NOT "${ENABLE_SANITIZER}" STREQUAL "NONE")
+IF( NOT "${ENABLE_SANITIZER}" STREQUAL "none")
   IF( NOT ${COMPILER} MATCHES "GNU" AND NOT ${COMPILER} MATCHES "Clang")
     MESSAGE(FATAL_ERROR "-DENABLE_SANITIZER=${ENABLE_SANITIZER} only works with GNU or Clang compilers")
   ENDIF()
 
-  IF( "${ENABLE_SANITIZER}" STREQUAL "ASAN" )
+  IF( "${ENABLE_SANITIZER}" STREQUAL "asan" )
     SET(CMAKE_CXX_FLAGS_SAN "-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer"
         CACHE STRING "AddressSanitizer C++ compiler builds." FORCE)
-  ELSEIF( "${ENABLE_SANITIZER}" STREQUAL "UBSAN" )
+  ELSEIF( "${ENABLE_SANITIZER}" STREQUAL "ubsan" )
     SET(CMAKE_CXX_FLAGS_SAN "-fsanitize=undefined"
         CACHE STRING "UndefinedBehaviorSanitizer C++ compiler builds." FORCE)
-  ELSEIF( "${ENABLE_SANITIZER}" STREQUAL "MSAN" )
+  ELSEIF( "${ENABLE_SANITIZER}" STREQUAL "msan" )
     SET(CMAKE_CXX_FLAGS_SAN "-fsanitize=memory"
         CACHE STRING "MemorySanitizer C++ compiler builds." FORCE)
-  ELSEIF( "${ENABLE_SANITIZER}" STREQUAL "TSAN" )
+  ELSEIF( "${ENABLE_SANITIZER}" STREQUAL "tsan" )
     SET(CMAKE_CXX_FLAGS_SAN "-fsanitize=thread"
         CACHE STRING "ThreadSanitizer C++ compiler builds." FORCE)
   ENDIF()

diff --git a/tests/test_automation/github-actions/ci/run_step.sh b/tests/test_automation/github-actions/ci/run_step.sh
@@ -15,33 +15,33 @@ case "$1" in
         echo 'Configure for address sanitizer asan including lsan (leaks)'
         CC=clang CXX=clang++ \
         cmake -GNinja -DMPI_C_COMPILER=mpicc -DMPI_CXX_COMPILER=mpicxx \
-                      -DCMAKE_BUILD_TYPE=Debug -DENABLE_SANITIZER=ASAN \
+                      -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_SANITIZER=asan \
                       ${GITHUB_WORKSPACE}
       ;;
       *"ubsan"*)
         echo 'Configure for undefined behavior sanitizer ubsan'
         CC=clang CXX=clang++ \
         cmake -GNinja -DMPI_C_COMPILER=mpicc -DMPI_CXX_COMPILER=mpicxx \
-                      -DCMAKE_BUILD_TYPE=Debug -DENABLE_SANITIZER=UBSAN \
+                      -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_SANITIZER=ubsan \
                       ${GITHUB_WORKSPACE}
       ;;
       *"tsan"*)
         echo 'Configure for thread sanitizer tsan'
         CC=clang CXX=clang++ \
         cmake -GNinja -DMPI_C_COMPILER=mpicc -DMPI_CXX_COMPILER=mpicxx \
-                      -DCMAKE_BUILD_TYPE=Debug -DENABLE_SANITIZER=TSAN \
+                      -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_SANITIZER=tsan \
                       ${GITHUB_WORKSPACE}
       ;;
       *"msan"*)
         echo 'Configure for (uninitialized) memory sanitizer msan'
         CC=clang CXX=clang++ \
         cmake -GNinja -DMPI_C_COMPILER=mpicc -DMPI_CXX_COMPILER=mpicxx \
-                      -DCMAKE_BUILD_TYPE=Debug -DENABLE_SANITIZER=MSAN \
+                      -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_SANITIZER=msan \
                       ${GITHUB_WORKSPACE}
       ;;
       # Configure with default compilers
       *)
-        echo 'Configure for default system compilers'
+        echo 'Configure for default system compilers and options'
         cmake -GNinja -DMPI_C_COMPILER=mpicc -DMPI_CXX_COMPILER=mpicxx \
                       ${GITHUB_WORKSPACE}
       ;;