[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #595: DISPATCH-1450: enable thread sanitizer (TSAN) run time checking

[GitBox <gi...@apache.org>]

kgiusti commented on a change in pull request #595: DISPATCH-1450: enable thread sanitizer (TSAN) run time checking
URL: https://github.com/apache/qpid-dispatch/pull/595#discussion_r337032177
 
 

 ##########
 File path: cmake/RuntimeChecks.cmake
 ##########
 @@ -17,41 +17,97 @@
 # under the License.
 #
 
-# Configuration for code analysis tools: runtime checking and coverage.
+# Configuration for code analysis tools.
+#
+# The RUNTIME_CHECK variable enables run-time checking when running
+# the CTest test suite. The following tools are supported
+#
+# -DRUNTIME_CHECK=memcheck   # runs qdrouter under valgrind's leak checker
+# -DRUNTIME_CHECK=tsan       # turns on thread sanitizer
+#
+# This file updates the QDROUTERD_RUNNER and CMAKE_C_FLAGS
+# appropriately for use when running the ctest suite.
 
-##
-## Valgrind
-##
+
+# Valid options for RUNTIME_CHECK
+#
+set(runtime_checks OFF tsan memcheck helgrind)
+
+# Valgrind configuration
+#
 find_program(VALGRIND_EXECUTABLE valgrind DOC "Location of the valgrind program")
-mark_as_advanced(VALGRIND_EXECUTABLE)
-find_package_handle_standard_args(VALGRIND DEFAULT_MSG VALGRIND_EXECUTABLE)
-option(USE_VALGRIND "Use valgrind when running tests" OFF)
-option(VALGRIND_XML "Write valgrind output as XML" OFF)
-
-if (USE_VALGRIND)
-    if (CMAKE_BUILD_TYPE MATCHES "Coverage")
-        message(WARNING "Building for coverage analysis; disabling valgrind run-time error detection")
-    else ()
-        set(QDROUTERD_RUNNER "${VALGRIND_EXECUTABLE} --quiet --leak-check=full --show-leak-kinds=definite --errors-for-leak-kinds=definite --error-exitcode=42 --suppressions=${CMAKE_SOURCE_DIR}/tests/valgrind.supp")
-        if (VALGRIND_XML)
-            set(QDROUTERD_RUNNER "${QDROUTERD_RUNNER} --xml=yes --xml-file=valgrind-%p.xml")
-        endif()
-    endif ()
+set(VALGRIND_SUPPRESSIONS "${CMAKE_SOURCE_DIR}/tests/valgrind.supp" CACHE STRING "Suppressions file for valgrind")
+set(VALGRIND_COMMON_ARGS "--error-exitcode=42 --xml=yes --xml-file=valgrind-%p.xml --quiet --suppressions=${VALGRIND_SUPPRESSIONS}")
+mark_as_advanced(VALGRIND_EXECUTABLE VALGRIND_SUPPRESSIONS VALGRIND_COMMON_ARGS)
+macro(assert_has_valgrind)
+  if(NOT VALGRIND_EXECUTABLE)
+    message(FATAL_ERROR "valgrind is not available")
+  endif()
+endmacro()
+
+# Check for compiler's support of sanitizers.
+# Currently have tested back to gcc 7.4.0 and clang 6.0.0, older
+# versions may require more work
+#
+if((CMAKE_C_COMPILER_ID MATCHES "GNU"
+      AND (CMAKE_C_COMPILER_VERSION VERSION_GREATER 7.4
+        OR CMAKE_C_COMPILER_VERSION VERSION_EQUAL 7.4))
+    OR (CMAKE_C_COMPILER_ID MATCHES "Clang"
+      AND (CMAKE_C_COMPILER_VERSION VERSION_GREATER 6.0
+        OR CMAKE_C_COMPILER_VERSION VERSION_EQUAL 6.0)))
+  set(HAS_SANITIZERS TRUE)
 endif()
+macro(assert_has_sanitizers)
+  if(NOT HAS_SANITIZERS)
+    message(FATAL_ERROR "compiler sanitizers are not available")
+  endif()
+endmacro()
 
-##
-## Sanitizers
-##
-option(USE_SANITIZERS "Compile with sanitizers (ASan, UBSan, TSan); incompatible with Valgrind" OFF)
-if (USE_SANITIZERS)
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fsanitize=leak -fsanitize=undefined")
-    add_compile_options(-g)
-    add_compile_options(-fno-omit-frame-pointer)
-endif (USE_SANITIZERS)
-
-option(USE_TSAN "Compile with ThreadSanitizer (TSan); incompatible with Valgrind" OFF)
-if (USE_TSAN)
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread")
-    add_compile_options(-g)
-    add_compile_options(-fno-omit-frame-pointer)
-endif (USE_TSAN)
+# Set RUNTIME_CHECK value and deal with the older cmake flags for
+# valgrind and TSAN
+#
+macro(deprecated_enable_check old new doc)
+  if (${old})
+    message("WARNING: option ${old} is deprecated, use -DRUNTIME_CHECK=${new} instead")
+    set(RUNTIME_CHECK_DEFAULT ${new})
+  endif()
+  unset(${old} CACHE)
+endmacro()
+option(VALGRIND_XML "Write valgrind output as XML (DEPRECATED)" OFF)
+deprecated_enable_check(USE_VALGRIND memcheck "Use valgrind to detect run-time problems")
+deprecated_enable_check(USE_TSAN tsan "Compile with thread sanitizer (tsan)")
+
+set(RUNTIME_CHECK ${RUNTIME_CHECK_DEFAULT} CACHE STRING "Enable runtime checks. Valid values: ${runtime_checks}")
+if(CMAKE_BUILD_TYPE MATCHES "Coverage" AND RUNTIME_CHECK)
+  message(FATAL_ERROR "Cannot set RUNTIME_CHECK with CMAKE_BUILD_TYPE=Coverage")
+endif()
+
+if(RUNTIME_CHECK STREQUAL "memcheck")
+  assert_has_valgrind()
+  message(STATUS "Runtime memory checker: valgrind memcheck")
+  set(QDROUTERD_RUNNER "${VALGRIND_EXECUTABLE} --tool=memcheck --leak-check=full --show-leak-kinds=definite --errors-for-leak-kinds=definite ${VALGRIND_COMMON_ARGS}")
+
+elseif(RUNTIME_CHECK STREQUAL "helgrind")
+  assert_has_valgrind()
+  message(STATUS "Runtime race checker: valgrind helgrind")
+  set(QDROUTERD_RUNNER "${VALGRIND_EXECUTABLE} --tool=helgrind ${VALGRIND_COMMON_ARGS}")
+
+#elseif(RUNTIME_CHECK STREQUAL "asan")
+#  assert_has_sanitizers()
+#  message(STATUS "Runtime memory checker: gcc/clang memory sanitizers")
+#  set(SANITIZE_FLAGS "-g -fno-omit-frame-pointer -fsanitize=address,undefined")
+#  set(TEST_WRAP_PREFIX "${CMAKE_SOURCE_DIR}/tests/preload_asan.sh $<TARGET_FILE:qpid-proton-core>")
 
 Review comment:
   Simply due to prioritizing delivery of thread sanitizing as soon as possible.
   We have Valgrind for memory checking already so there is coverage there, but nothing for threading at this point.
   
   Feel free to open a JIRA upstream in dispatch to track implementing address and thread sanitizing as a separate patch.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org