Merge pull request #3321 from eseiler/fix/tsan

[INFRA] Sanitizer CI

.clang-format

@@ -274,4 +274,5 @@ WhitespaceSensitiveMacros:
   - STRINGIZE
   - SEQAN3_WORKAROUND_GCC_BOGUS_MEMCPY_START
   - SEQAN3_WORKAROUND_GCC_BOGUS_MEMCPY_STOP
+  - reduction
 ...

.github/workflows/ci_misc.yml

@@ -60,7 +60,7 @@ jobs:
           mkdir build && cd build
           cmake ../test/${{ matrix.build }} -DCMAKE_BUILD_TYPE=Release \
                                             -DCMAKE_CXX_FLAGS="${{ matrix.cxx_flags }}" \
-                                            -DSEQAN3_BENCHMARK_MIN_TIME=0.01
+                                            -DSEQAN3_BENCHMARK_MIN_TIME=0.01s
           case "${{ matrix.build }}" in
             snippet) make gtest_main;;
             performance) make benchmark_main;;

.github/workflows/ci_sanitizer.yml

@@ -0,0 +1,113 @@
+# SPDX-FileCopyrightText: 2006-2024, Knut Reinert & Freie Universität Berlin
+# SPDX-FileCopyrightText: 2016-2024, Knut Reinert & MPI für molekulare Genetik
+# SPDX-License-Identifier: CC0-1.0
+
+name: Sanitizer
+
+on:
+  schedule:
+    - cron: "0 6 * * SAT"
+  workflow_dispatch:
+
+concurrency:
+  group: sanitizer-actions
+  cancel-in-progress: true
+
+env:
+  SEQAN3_NO_VERSION_CHECK: 1
+  TZ: Europe/Berlin
+  TSAN_OPTIONS: ignore_noninstrumented_modules=1
+  UBSAN_OPTIONS: print_stacktrace=1
+
+defaults:
+  run:
+    shell: bash -Eeuxo pipefail {0}
+
+jobs:
+  build:
+    name: ${{ matrix.name }} ${{ matrix.build_type }} ${{ matrix.os }}
+    runs-on: ${{ matrix.os }}
+    if: github.repository_owner == 'seqan' || github.event_name == 'workflow_dispatch'
+    env:
+      ASAN_OPTIONS: strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:detect_leaks=${{ contains(matrix.os, 'macos') && '0' || '1' }}
+    strategy:
+      fail-fast: false
+      matrix:
+        name: [ASan, TSan, UBSan]
+        os: [ubuntu-latest, macos-14]
+        build_type: [Release, RelWithDebInfo, Debug]
+        exclude:
+          # macOS llvm packages do not contain libarcher, which is required for TSan to handle OpenMP.
+          # TSan runs on ubuntu with clang. Packages there contain libarcher.
+          - name: "TSan"
+            os: macos-14
+
+        include:
+          - os: macos-14
+            compiler: clang-19
+          - os: ubuntu-latest
+            compiler: gcc-14
+            image: ghcr.io/seqan/gcc-14
+
+          - name: "TSan"
+            os: ubuntu-latest
+            compiler: clang-19
+            image: ghcr.io/seqan/clang-19
+            cxx_flags: "-fsanitize=thread"
+            ctest_excludes: "-E async_input_buffer_snippet"
+
+          - name: "ASan"
+            os: ubuntu-latest
+            cxx_flags: "-fsanitize=address -Wno-maybe-uninitialized"
+          - name: "ASan"
+            os: macos-14
+            cxx_flags: "-fsanitize=address"
+
+          - name: "UBSan"
+            os: ubuntu-latest
+            cxx_flags: "-fsanitize=undefined,float-divide-by-zero -Wno-maybe-uninitialized -Wno-stringop-overflow"
+          - name: "UBSan"
+            os: macos-14
+            cxx_flags: "-fsanitize=undefined,float-divide-by-zero,local-bounds,nullability"
+            ctest_excludes: "-E tmp_directory_snippet_cmp_output"
+
+    container:
+      # If an image is defined for a matrix entry, use it.
+      # Otherwise, use the "empty"/'' image which means do not use a container at all.
+      image: ${{ matrix.image || '' }}
+      volumes:
+        - /home/runner:/home/runner
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Setup compiler
+        if: contains(matrix.os, 'macos')
+        uses: seqan/actions/setup-compiler@main
+        with:
+          compiler: ${{ matrix.compiler }}
+
+      - name: Configure tests
+        run: |
+          mkdir build && cd build
+          cmake ../test/analyse -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} \
+                                -DCMAKE_CXX_FLAGS="-fno-omit-frame-pointer ${{ matrix.cxx_flags }} -fno-sanitize-recover=all" \
+                                -DSEQAN3_BENCHMARK_MIN_TIME=0.01s \
+                                -DSEQAN3_WITH_SEQAN2_CI=OFF
+          make gtest_main benchmark_main
+
+      - name: Build tests
+        working-directory: build
+        run: make -k
+
+      - name: Run tests
+        working-directory: build
+        continue-on-error: true
+        id: test
+        run: ctest . -j --output-on-failure --no-tests=error ${{ matrix.ctest_excludes }}
+
+      # Rerun failed tests with **one** thread. Some snippets touch the same file and fail in parallel.
+      - name: Rerun failed tests
+        if: steps.test.outcome == 'failure'
+        working-directory: build
+        run: ctest . -j1 --output-on-failure --no-tests=error --rerun-failed

.github/workflows/cron_avx2.yml

@@ -51,7 +51,7 @@ jobs:
           mkdir build && cd build
           cmake ../test/${{ matrix.build }} -DCMAKE_BUILD_TYPE=Release \
                                             -DCMAKE_CXX_FLAGS="-mavx2 ${{ matrix.cxx_flags }}" \
-                                            -DSEQAN3_BENCHMARK_MIN_TIME=0.01
+                                            -DSEQAN3_BENCHMARK_MIN_TIME=0.01s
           case "${{ matrix.build }}" in
             unit) make gtest_main;;
             snippet) make gtest_main;;

.github/workflows/cron_latest_libraries.yml

@@ -59,7 +59,7 @@ jobs:
           mkdir build && cd build
           cmake ../test/${{ matrix.build }} -DCMAKE_BUILD_TYPE=Release \
                                             -DCMAKE_CXX_FLAGS="${{ matrix.cxx_flags }}" \
-                                            -DSEQAN3_BENCHMARK_MIN_TIME=0.01
+                                            -DSEQAN3_BENCHMARK_MIN_TIME=0.01s
           case "${{ matrix.build }}" in
             unit) make gtest_main;;
             snippet) make gtest_main;;

include/seqan3/core/debug_stream/debug_stream_type.hpp

@@ -28,7 +28,7 @@ namespace seqan3
 //!\ingroup core_debug_stream
 //!\implements seqan3::enum_bitwise_operators
 //!\sa seqan3::enum_bitwise_operators enables combining enum values.
-enum fmtflags2
+enum class fmtflags2 : int8_t
 {
     none = 0,                     //!< No flag is set.
     utf8 = 1,                     //!< Enables use of non-ASCII UTF8 characters in formatted output.

include/seqan3/io/structure_file/format_vienna.hpp

@@ -200,8 +200,14 @@ class format_vienna
                 using alph_type = typename std::ranges::range_value_t<structure_type>::structure_alphabet_type;
                 // We need the structure_length parameter to count the length of the structure while reading
                 // because we cannot infer it from the (already resized) structure_seq object.
-                auto res = std::ranges::copy(read_structure<alph_type>(stream_view), std::ranges::begin(structure));
+                auto range = read_structure<alph_type>(stream_view);
+                // Use std::views::take to avoid going out of bounds if the structure is longer than the sequence.
+                auto res = std::ranges::copy(range | std::views::take(std::ranges::distance(seq)),
+                                             std::ranges::begin(structure));
                 structure_length = std::ranges::distance(std::ranges::begin(structure), res.out);
+                // If the structure is longer than the sequence, there are characters left.
+                // std::ranges::distance will also consume the characters in the stream.
+                structure_length += std::ranges::distance(range);
 
                 if constexpr (!detail::decays_to_ignore_v<bpp_type>)
                     detail::bpp_from_rna_structure<alph_type>(bpp, structure);

include/seqan3/utility/math.hpp

@@ -123,9 +123,25 @@ base_t pow(base_t base, exp_t exp)
     if (base == 0)
         return 0;
 
+    auto check = [base](base_t result)
+    {
+        if (base > 0)
+        {
+            return result > std::numeric_limits<base_t>::max() / base;
+        }
+        else if (result < 0) // and base < 0
+        {
+            return result < std::numeric_limits<base_t>::max() / base;
+        }
+        else // base < 0 and result > 0
+        {
+            return base < std::numeric_limits<base_t>::min() / result;
+        }
+    };
+
     for (exp_t i = 0; i < exp; ++i)
     {
-        if ((base < 0 ? std::numeric_limits<base_t>::min() : std::numeric_limits<base_t>::max()) / base < result)
+        if (check(result))
         {
             std::string error_message{"Calculating " + std::to_string(base) + '^' + std::to_string(exp)
                                       + " will result in an "

test/performance/CMakeLists.txt

@@ -10,7 +10,7 @@ include (../seqan3-test.cmake)
 CPMGetPackage (benchmark)
 
 set (SEQAN3_BENCHMARK_MIN_TIME
-     "1"
+     "1s"
      CACHE STRING "Set --benchmark_min_time= for each bechmark. Timings are unreliable in CI.")
 
 macro (seqan3_benchmark benchmark_cpp)

test/performance/alignment/global_affine_alignment_parallel_benchmark.cpp

@@ -113,7 +113,7 @@ void seqan3_affine_dna4_omp_for(benchmark::State & state)
     int64_t total = 0;
     for (auto _ : state)
     {
-#    pragma omp parallel for num_threads(std::thread::hardware_concurrency()) schedule(guided)
+#    pragma omp parallel for num_threads(std::thread::hardware_concurrency()) schedule(guided) reduction(+:total)
         for (size_t i = 0; i < zip.size(); ++i)
         {
             auto rng = align_pairwise(zip[i], affine_cfg | result_t{});
@@ -182,7 +182,7 @@ void seqan2_affine_dna4_omp_for(benchmark::State & state)
     int64_t total = 0;
     for (auto _ : state)
     {
-#    pragma omp parallel for num_threads(std::thread::hardware_concurrency()) schedule(guided)
+#    pragma omp parallel for num_threads(std::thread::hardware_concurrency()) schedule(guided) reduction(+:total)
         for (size_t i = 0; i < seqan2::length(vec1); ++i)
         {
             if constexpr (score_only)

test/performance/range/container_assignment_benchmark.cpp

@@ -30,8 +30,8 @@ struct assignment_functor
 {
     template <typename container_t>
         requires (id == tag::assignment_operator)
-    static constexpr void call(container_t & to, container_t const & from) noexcept(
-        noexcept(std::is_nothrow_assignable_v<container_t, container_t>))
+    static constexpr void call(container_t & to, container_t const & from)
+        noexcept(noexcept(std::is_nothrow_assignable_v<container_t, container_t>))
     {
         benchmark::DoNotOptimize(to = from);
     }
@@ -53,31 +53,18 @@ struct assignment_functor
     }
 };
 
-template <typename container_t>
-    requires requires (container_t v) { v.clear(); }
-static constexpr void clear(container_t & container) noexcept(noexcept(std::declval<container_t>().clear()))
-{
-    container.clear();
-}
-
 template <typename container_t>
     requires requires (container_t v) { v.resize(1u); }
-static constexpr void resize(container_t & container,
-                             size_t const size) noexcept(noexcept(std::declval<container_t>().resize(1u)))
+static constexpr void resize(container_t & container, size_t const size)
+    noexcept(noexcept(std::declval<container_t>().resize(1u)))
 {
     container.resize(size);
 }
 
 #if SEQAN3_HAS_SEQAN2
 template <typename container_t>
-static constexpr void clear(container_t & container) noexcept(noexcept(seqan2::clear(std::declval<container_t>())))
-{
-    seqan2::clear(container);
-}
-
-template <typename container_t>
-static constexpr void resize(container_t & container,
-                             size_t const size) noexcept(noexcept(seqan2::resize(std::declval<container_t>(), 1u)))
+static constexpr void resize(container_t & container, size_t const size)
+    noexcept(noexcept(seqan2::resize(std::declval<container_t>(), 1u)))
 {
     seqan2::resize(container, size);
 }
@@ -107,8 +94,6 @@ static void assign(benchmark::State & state)
     {
         fn.call(to, from);
         benchmark::ClobberMemory();
-        clear(to);
-        benchmark::ClobberMemory();
     }
 }
 

test/snippet/CMakeLists.txt

@@ -40,7 +40,7 @@ macro (seqan3_snippet test_name_prefix snippet snippet_base_path)
     add_test (NAME "${snippet_compare_test_target}"
               COMMAND ${CMAKE_COMMAND} -DTARGET_FILE=$<TARGET_FILE:${target}>
                       -DSOURCE_FILE=${snippet_base_path}/${snippet} #
-                      -P "${CMAKE_SOURCE_DIR}/compare_snippet_output.cmake")
+                      -P "${CMAKE_CURRENT_SOURCE_DIR}/compare_snippet_output.cmake")
 
     # disable version checker, as it interferes with comparing the snippet output
     set_tests_properties ("${snippet_compare_test_target}" PROPERTIES ENVIRONMENT SEQAN3_NO_VERSION_CHECK=0)

test/unit/alignment/scoring/scoring_scheme_test.cpp

@@ -59,7 +59,7 @@ TEST(nucleotide_scoring_scheme, template_argument_deduction)
     }
 
     {
-        std::array<std::array<int16_t, 15>, 15> m;
+        std::array<std::array<int16_t, 15>, 15> m{};
         seqan3::nucleotide_scoring_scheme scheme{m};
         EXPECT_TRUE((std::is_same_v<decltype(scheme), seqan3::nucleotide_scoring_scheme<int16_t>>));
     }
@@ -83,7 +83,7 @@ TEST(aminoacid_scoring_scheme, template_argument_deduction)
     }
 
     {
-        std::array<std::array<int16_t, 27>, 27> m;
+        std::array<std::array<int16_t, 27>, 27> m{};
         seqan3::aminoacid_scoring_scheme scheme{m};
         EXPECT_TRUE((std::is_same_v<decltype(scheme), seqan3::aminoacid_scoring_scheme<int16_t>>));
     }

test/unit/contrib/parallel/buffer_queue_parallel_test.cpp

@@ -137,8 +137,8 @@ void test_buffer_queue_wait_throw(size_t initialCapacity)
     if constexpr (sequential_pop_t::value)
         thread_count = writer_count + 1;
 
-    // std::cout << "threads: " << thread_count << ‘\n‘;
-    // std::cout << "writers: " << writer_count << ‘\n‘;
+    // std::cout << "threads: " << thread_count << '\n';
+    // std::cout << "writers: " << writer_count << '\n';
 
     ASSERT_GE(thread_count, 2u);
 
@@ -147,8 +147,8 @@ void test_buffer_queue_wait_throw(size_t initialCapacity)
     std::vector<std::thread> workers;
     std::atomic<size_t> registered_writer = 0;
     std::atomic<size_t> registered_reader = 0;
-    seqan3::contrib::queue_op_status push_status = seqan3::contrib::queue_op_status::success;
-    seqan3::contrib::queue_op_status pop_status = seqan3::contrib::queue_op_status::success;
+    std::atomic<seqan3::contrib::queue_op_status> push_status = seqan3::contrib::queue_op_status::success;
+    std::atomic<seqan3::contrib::queue_op_status> pop_status = seqan3::contrib::queue_op_status::success;
     for (size_t tid = 0; tid < thread_count; ++tid)
     {
         workers.push_back(std::thread(

test/unit/range/iterator_test_template.hpp

@@ -499,14 +499,14 @@ inline void jump_backward_test(it_begin_t && it_begin, rng_t && rng)
     // Backward copy it + (-n)
     for (size_t n = 0; n < sz; ++n)
     {
-        expect_iter_equal<test_type>(pre_end_it + (-1 * n), pre_end_rng_it - n);
+        expect_iter_equal<test_type>(pre_end_it + (-static_cast<std::ptrdiff_t>(n)), pre_end_rng_it - n);
         expect_iter_equal<test_type>(pre_end_it, pre_end_rng_it);
     }
 
     // Backward copy friend through (-n) + it
     for (size_t n = 0; n < sz; ++n)
     {
-        expect_iter_equal<test_type>((-1 * n) + pre_end_it, pre_end_rng_it - n);
+        expect_iter_equal<test_type>((-static_cast<std::ptrdiff_t>(n)) + pre_end_it, pre_end_rng_it - n);
         expect_iter_equal<test_type>(pre_end_it, pre_end_rng_it);
     }
 }