Enable sanitizers in tests (#465)

* Enable sanitizers in tests * Enable sanitizers in workflows * Fix undefined behavior in mpi test * Add undefined behavior in non-mpi test * Remove undefined behavior previously introduced * Trigger asan in non-mpi test * Undo last commit * Explain why no asan in mpi tests * Add help text * Enable address sanitizer for mpi tests but not leak sanitizer * Trigger asan in mpi test * Undo last commit * Fix typo
kamping-site · Dec 18, 2022 · 4f5b1e9 · 4f5b1e9
1 parent ddccf19
commit 4f5b1e9
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 3 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -71,7 +71,7 @@ jobs:
     - name: print-mpicxx-flags
       run: mpicxx -show
     - name: cmake
-      run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build-mode }} -DCMAKE_CXX_COMPILER=${{ matrix.compiler.cxx }} -DCMAKE_C_COMPILER=${{ matrix.compiler.cc }} -DKAMPING_WARNINGS_ARE_ERRORS=ON -DKAMPING_EXCEPTION_MODE=${{ matrix.exception-mode }} -DKAMPING_ASSERTION_LEVEL=${{ matrix.assertion-level }} -DKAMPING_TESTS_DISCOVER=OFF
+      run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build-mode }} -DCMAKE_CXX_COMPILER=${{ matrix.compiler.cxx }} -DCMAKE_C_COMPILER=${{ matrix.compiler.cc }} -DKAMPING_WARNINGS_ARE_ERRORS=ON -DKAMPING_EXCEPTION_MODE=${{ matrix.exception-mode }} -DKAMPING_ASSERTION_LEVEL=${{ matrix.assertion-level }} -DKAMPING_TESTS_DISCOVER=OFF -DKAMPING_TEST_ENABLE_SANITIZERS=ON
     - name: build
       run: cmake --build build/ --parallel
     - name: Allow-running-mpi

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -17,6 +17,8 @@ if (NOT N EQUAL 0)
     )
 endif ()
 
+option(KAMPING_TEST_ENABLE_SANITIZERS "Enable undefined behavior sanitizer and address sanitizer." ON)
+
 # Registering tests without MPI:
 kamping_register_test(test_checking_casts FILES checking_casts_test.cpp)
 kamping_register_test(test_mpi_function_wrapper_helpers FILES mpi_function_wrapper_helpers_test.cpp)

diff --git a/tests/cmake/KaTestrophe.cmake b/tests/cmake/KaTestrophe.cmake
@@ -87,13 +87,18 @@ if (NOT DEFINED KATESTROPHE_INCLUDED)
                     ${MPI}
                     MPI_EXEC_COMMAND
                     "${MPI_EXEC_COMMAND}"
+                    PROPERTIES
+                    ENVIRONMENT
+                    "ASAN_OPTIONS=detect_leaks=0" # Prevent memory leaks in OpenMPI from making the test fail.
                 )
             else ()
                 add_test(
                     NAME "${TEST_NAME}"
                     COMMAND ${MPI_EXEC_COMMAND} $<TARGET_FILE:${KATESTROPHE_TEST_TARGET}>
                     WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
                 )
+                # Prevent memory leaks in OpenMPI from making the test fail.
+                set_property(TEST ${TEST_NAME} PROPERTY ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0")
             endif ()
             # TODO: Do not rely on the return value of mpiexec to check if a test succeeded, as this does not work for
             # ULFM.

diff --git a/tests/cmake/KampingTestHelper.cmake b/tests/cmake/KampingTestHelper.cmake
@@ -28,6 +28,15 @@ function (kamping_register_test KAMPING_TARGET_NAME)
     kamping_set_kassert_flags(${KAMPING_TARGET_NAME} ${ARGN})
     target_compile_definitions(${KAMPING_TARGET_NAME} PRIVATE -D_GLIBCXX_DEBUG)
     target_compile_definitions(${KAMPING_TARGET_NAME} PRIVATE -D_GLIBCXX_DEBUG_PEDANTIC)
+
+    if (KAMPING_TEST_ENABLE_SANITIZERS)
+        target_compile_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=address)
+        target_link_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=address)
+        target_compile_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=undefined)
+        target_link_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=undefined)
+        target_compile_options(${KAMPING_TARGET_NAME} PRIVATE -fno-sanitize-recover=all)
+        target_link_options(${KAMPING_TARGET_NAME} PRIVATE -fno-sanitize-recover=all)
+    endif ()
 endfunction ()
 
 # Convenience wrapper for adding tests for KaMPIng which rely on MPI this creates the target, links googletest, kamping
@@ -52,6 +61,14 @@ function (kamping_register_mpi_test KAMPING_TARGET_NAME)
     kamping_set_kassert_flags(${KAMPING_TARGET_NAME} ${ARGN})
     target_compile_definitions(${KAMPING_TARGET_NAME} PRIVATE -D_GLIBCXX_DEBUG)
     target_compile_definitions(${KAMPING_TARGET_NAME} PRIVATE -D_GLIBCXX_DEBUG_PEDANTIC)
+    if (KAMPING_TEST_ENABLE_SANITIZERS)
+        target_compile_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=undefined)
+        target_link_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=undefined)
+        target_compile_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=address)
+        target_link_options(${KAMPING_TARGET_NAME} PRIVATE -fsanitize=address)
+        target_compile_options(${KAMPING_TARGET_NAME} PRIVATE -fno-sanitize-recover=all)
+        target_link_options(${KAMPING_TARGET_NAME} PRIVATE -fno-sanitize-recover=all)
+    endif ()
 endfunction ()
 
 # Convenience wrapper for registering a set of tests that should fail to compile and require KaMPIng to be linked.

diff --git a/tests/cmake/MPIGoogleTestAddTests.cmake b/tests/cmake/MPIGoogleTestAddTests.cmake
@@ -70,13 +70,17 @@ function (gtest_discover_tests_impl)
     if (NOT EXISTS "${_TEST_EXECUTABLE}")
         message(FATAL_ERROR "Specified test executable does not exist.\n" "  Path: '${_TEST_EXECUTABLE}'")
     endif ()
+    # Prevent OpenMPI memory leaks from making the next command fail.
+    set(ENV{ASAN_OPTIONS} detect_leaks=0)
     execute_process(
         COMMAND ${_TEST_EXECUTOR} "${_TEST_EXECUTABLE}" --gtest_list_tests ${filter}
         WORKING_DIRECTORY "${_TEST_WORKING_DIR}"
         TIMEOUT ${_TEST_DISCOVERY_TIMEOUT}
         OUTPUT_VARIABLE output
         RESULT_VARIABLE result
     )
+    # Undo previous set.
+    unset(ENV{ASAN_OPTIONS})
     if (NOT ${result} EQUAL 0)
         string(REPLACE "\n" "\n    " output "${output}")
         message(FATAL_ERROR "Error running test executable.\n" "  Path: '${_TEST_EXECUTABLE}'\n"

diff --git a/tests/mpi_communicator_test.cpp b/tests/mpi_communicator_test.cpp
@@ -11,6 +11,8 @@
 // You should have received a copy of the GNU Lesser General Public License along with KaMPIng.  If not, see
 // <https://www.gnu.org/licenses/>.
 
+#include <limits>
+
 #include <gtest/gtest.h>
 #include <kassert/kassert.hpp>
 #include <mpi.h>
@@ -130,7 +132,11 @@ TEST_F(CommunicatorTest, is_valid_tag) {
     ASSERT_TRUE(comm.is_valid_tag(0));
     ASSERT_TRUE(comm.is_valid_tag(42));
     ASSERT_TRUE(comm.is_valid_tag(mpi_tag_ub));
-    ASSERT_FALSE(comm.is_valid_tag(mpi_tag_ub + 1));
+    // Avoid signed integer overflow
+    if (mpi_tag_ub < std::numeric_limits<decltype(mpi_tag_ub)>::max()) {
+        ASSERT_FALSE(comm.is_valid_tag(mpi_tag_ub + 1));
+    }
+
     if (mpi_tag_ub == std::numeric_limits<int>::max()) {
         ASSERT_TRUE(comm.is_valid_tag(std::numeric_limits<int>::max()));
     } else {
@@ -150,7 +156,10 @@ TEST_F(CommunicatorTest, set_default_tag) {
     ASSERT_EQ(comm.default_tag(), 23);
     comm.default_tag(mpi_tag_ub);
     ASSERT_EQ(comm.default_tag(), mpi_tag_ub);
-    EXPECT_THROW(comm.default_tag(mpi_tag_ub + 1), kassert::KassertException);
+    // Avoid signed integer overflow
+    if (mpi_tag_ub < std::numeric_limits<decltype(mpi_tag_ub)>::max()) {
+        EXPECT_THROW(comm.default_tag(mpi_tag_ub + 1), kassert::KassertException);
+    }
     EXPECT_THROW(comm.default_tag(-1), kassert::KassertException);
 }