diff --git a/.github/workflows/sanitizer-debug.yaml b/.github/workflows/sanitizer-debug.yaml new file mode 100644 index 0000000000..3e4075a434 --- /dev/null +++ b/.github/workflows/sanitizer-debug.yaml @@ -0,0 +1,130 @@ +name: Debug sanitizers + +on: + push: + branches: + - main + - fix-sanitizer-issues + pull_request: + branches: + - main + +jobs: + build_dependencies: + name: Build deps [ubuntu] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Get dependencies hash + id: get-hash + run: echo "deps_hash=`cat docker/build_deps.sh | shasum`" >> $GITHUB_OUTPUT + - name: Cache lookup + uses: actions/cache/restore@v4 + id: cache-lookup + with: + path: deps + key: deps-${{ steps.get-hash.outputs.deps_hash }} + lookup-only: true + - name: Set up dependencies + if: steps.cache-lookup.outputs.cache-hit != 'true' + run: | + sudo apt-get update + sudo apt-get install -qy build-essential \ + gdb \ + curl \ + python3.10 \ + python3-pip \ + cmake \ + ninja-build \ + pkg-config \ + bison \ + libfl-dev \ + libbenchmark-dev \ + libgmock-dev \ + libz-dev + - name: Fetch & Build non packaged dependencies + if: steps.cache-lookup.outputs.cache-hit != 'true' + run: | + mkdir -p deps + cd deps + ../docker/build_deps.sh + - name: Cache save + if: steps.cache-lookup.outputs.cache-hit != 'true' + uses: actions/cache/save@v4 + with: + path: deps + key: deps-${{ steps.get-hash.outputs.deps_hash }} + + unit_tests_cxx: + name: UT [c++] + runs-on: ubuntu-latest + needs: build_dependencies + steps: + - uses: actions/checkout@v4 + - name: Get dependencies hash + id: get-hash + run: echo "deps_hash=`cat docker/build_deps.sh | shasum`" >> $GITHUB_OUTPUT + - uses: actions/cache/restore@v4 + with: + path: deps + key: deps-${{ steps.get-hash.outputs.deps_hash }} + - name: Set up dependencies + run: | + sudo apt-get update + sudo apt-get install -qy build-essential \ + gdb \ + curl \ + python3.10 \ + python3-pip \ + cmake \ + ninja-build \ + pkg-config \ + bison \ + libfl-dev \ + libbenchmark-dev \ + libgmock-dev \ + libz-dev + - name: Install cached non packaged dependencies + working-directory: deps + run: ../docker/build_deps.sh + - name: Build BlazingMQ UTs + env: + PKG_CONFIG_PATH: /usr/lib/x86_64-linux-gnu/pkgconfig:/opt/bb/lib64/pkgconfig + run: | + cmake -S . -B build/blazingmq -G Ninja \ + -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/deps/srcs/bde-tools/BdeBuildSystem/toolchains/linux/gcc-default.cmake \ + -DCMAKE_BUILD_TYPE=Debug \ + -DBDE_BUILD_TARGET_SAFE=ON \ + -DBDE_BUILD_TARGET_64=ON \ + -DBDE_BUILD_TARGET_CPP17=ON \ + -DCMAKE_PREFIX_PATH=${{ github.workspace }}/deps/srcs/bde-tools/BdeBuildSystem \ + -DCMAKE_INSTALL_LIBDIR=lib64 + cmake --build build/blazingmq --parallel 8 --target all.t + - name: Run C++ Unit Tests + run: | + cd ${{ github.workspace }}/build/blazingmq + ctest -E mwcsys_executil.t --output-on-failure + + run_asan: + needs: build_dependencies + uses: ./.github/workflows/sanitizer-check.yaml + with: + sanitizer-name: 'asan' + + run_msan: + needs: build_dependencies + uses: ./.github/workflows/sanitizer-check.yaml + with: + sanitizer-name: 'msan' + + run_tsan: + needs: build_dependencies + uses: ./.github/workflows/sanitizer-check.yaml + with: + sanitizer-name: 'tsan' + + run_ubsan: + needs: build_dependencies + uses: ./.github/workflows/sanitizer-check.yaml + with: + sanitizer-name: 'ubsan' diff --git a/etc/tsansup.txt b/etc/tsansup.txt index 4056b7e308..ff480e7825 100644 --- a/etc/tsansup.txt +++ b/etc/tsansup.txt @@ -15,7 +15,7 @@ race:BloombergLP::bcema_Pool<*>::allocate # Not sure what the problem is here, but tsan can't show the other stack, so # there's nothing to look into -race:__tsan_atomic32_fetch_add +#race:__tsan_atomic32_fetch_add # Don't warn about using cout from multiple threads race:std::basic_ostream& bsl::operator<< <*>(std::basic_ostream<*>&, bsl::basic_string<*> const&) @@ -27,8 +27,8 @@ race:BloombergLP::ball::LoggerManager::isInitialized() # Suppress TSan report in a routine used in bmqimp::Brokersession test driver. # It is a benign race in the test driver, but should be looked into at some # point. -race:TestSession::waitForChannelClose -race:TestSession::arriveAtStepWithCfgs +#race:TestSession::waitForChannelClose +#race:TestSession::arriveAtStepWithCfgs # Since we use mqbmock::Dispatcher in unit tests, this method does not get # enqueued correctly back to cluster-dispatcher thread, causing potential diff --git a/src/groups/bmq/bmqimp/bmqimp_brokersession.t.cpp b/src/groups/bmq/bmqimp/bmqimp_brokersession.t.cpp index 3205e5d2f6..139cebe5cb 100644 --- a/src/groups/bmq/bmqimp/bmqimp_brokersession.t.cpp +++ b/src/groups/bmq/bmqimp/bmqimp_brokersession.t.cpp @@ -2196,12 +2196,12 @@ bool TestSession::waitForChannelClose(const bsls::TimeInterval& timeout) // Wait for the close to be called on the base channel const bsls::TimeInterval expireAfter = bsls::SystemTime::nowRealtimeClock() + timeout; - while (d_testChannel.closeCalls().empty() && + while (d_testChannel.closeCallsEmpty() && bsls::SystemTime::nowRealtimeClock() < expireAfter) { bslmt::ThreadUtil::microSleep(k_TIME_SOURCE_STEP.totalMicroseconds()); } - if (d_testChannel.closeCalls().empty()) { + if (d_testChannel.closeCallsEmpty()) { return false; // RETURN } diff --git a/src/groups/mwc/mwcio/mwcio_ntcchannelfactory.t.cpp b/src/groups/mwc/mwcio/mwcio_ntcchannelfactory.t.cpp index 69e28b282d..45d5ee0825 100644 --- a/src/groups/mwc/mwcio/mwcio_ntcchannelfactory.t.cpp +++ b/src/groups/mwc/mwcio/mwcio_ntcchannelfactory.t.cpp @@ -98,19 +98,22 @@ static const ChannelWatermarkType::Enum WAT_HIGH = static const ChannelWatermarkType::Enum WAT_LOW = ChannelWatermarkType::e_LOW_WATERMARK; -#ifdef BSLS_PLATFORM_OS_SOLARIS -static const bool skipTest = true; +// Adjust attempt interval depending on the platform +#if defined(BSLS_PLATFORM_OS_SOLARIS) || defined(BSLS_PLATFORM_OS_AIX) +static const int k_ATTEMPT_INTERVAL_MS = 300; #elif defined( \ __has_feature) // Clang-supported method for checking sanitizers. -static const bool skipTest = __has_feature(memory_sanitizer) || - __has_feature(thread_sanitizer) || - __has_feature(undefined_behavior_sanitizer); +static const int k_ATTEMPT_INTERVAL_MS = + (__has_feature(memory_sanitizer) || __has_feature(thread_sanitizer) || + __has_feature(undefined_behavior_sanitizer)) + ? 300 + : 1; #elif defined(__SANITIZE_MEMORY__) || defined(__SANITIZE_THREAD__) || \ defined(__SANITIZE_UNDEFINED__) // GCC-supported macros for checking MSAN, TSAN and UBSAN. -static const bool skipTest = true; +static const int k_ATTEMPT_INTERVAL_MS = 300; #else -static const bool skipTest = false; // Default to running the test. +static const int k_ATTEMPT_INTERVAL_MS = 1; #endif // ======================== @@ -658,10 +661,9 @@ void Tester::connect(int line, ConnectOptions reqOptions(options, s_allocator_p); - reqOptions.setAttemptInterval( - bsls::TimeInterval(0, 10 * bdlt::TimeUnitRatio::k_NS_PER_MS)); - - reqOptions.setAttemptInterval(bsls::TimeInterval(0, 1)); + reqOptions.setAttemptInterval(bsls::TimeInterval( + 0, + k_ATTEMPT_INTERVAL_MS * bdlt::TimeUnitRatio::k_NS_PER_MS)); HandleMap::iterator serverIter = d_handleMap.find(endpointOrServer); if (serverIter != d_handleMap.end()) { @@ -1055,22 +1057,6 @@ static void test6_preCreationCbTest() { mwctst::TestHelper::printTestName("Pre Creation Cb Test"); - if (skipTest) { - // This test has been disabled for Solaris/MSan/TSan/UBSan build. This - // test relies on the timings of certain callbacks being fired before - // or after certain operations. Normally this timing is always - // observed, but in msan/tsan/ubsan enabled build, the timing gets - // changed, leading to test failure. Of course, the right fix is to - // not rely on these timings, which can be worked on if the test starts - // failing in non-instrumented builds. Additionally, we could try to - // enable this test in MSan/TSan/UBSan build once all MSan/TSan/UBSan - // reports have been fixed to see if that helps (see `msansup.txt`, - // `tsansup.txt` and `ubsansup.txt`). - bsl::cout << "Test skipped (running on Solaris or under sanitizer)" - << bsl::endl; - return; // RETURN - } - Tester t(s_allocator_p); // Concern 'a' @@ -1106,22 +1092,6 @@ static void test5_visitChannelsTest() { mwctst::TestHelper::printTestName("Cancel Handle Test"); - if (skipTest) { - // This test has been disabled for Solaris/MSan/TSan/UBSan build. This - // test relies on the timings of certain callbacks being fired before - // or after certain operations. Normally this timing is always - // observed, but in msan/tsan/ubsan enabled build, the timing gets - // changed, leading to test failure. Of course, the right fix is to - // not rely on these timings, which can be worked on if the test starts - // failing in non-instrumented builds. Additionally, we could try to - // enable this test in MSan/TSan/UBSan build once all MSan/TSan/UBSan - // reports have been fixed to see if that helps (see `msansup.txt`, - // `tsansup.txt` and `ubsansup.txt`). - bsl::cout << "Test skipped (running on Solaris or under sanitizer)" - << bsl::endl; - return; // RETURN - } - Tester t(s_allocator_p); // Concerns 'a' @@ -1162,22 +1132,6 @@ static void test4_cancelHandleTest() { mwctst::TestHelper::printTestName("Cancel Handle Test"); - if (skipTest) { - // This test has been disabled for Solaris/MSan/TSan/UBSan build. This - // test relies on the timings of certain callbacks being fired before - // or after certain operations. Normally this timing is always - // observed, but in msan/tsan/ubsan enabled build, the timing gets - // changed, leading to test failure. Of course, the right fix is to - // not rely on these timings, which can be worked on if the test starts - // failing in non-instrumented builds. Additionally, we could try to - // enable this test in MSan/TSan/UBSan build once all MSan/TSan/UBSan - // reports have been fixed to see if that helps (see `msansup.txt`, - // `tsansup.txt` and `ubsansup.txt`). - bsl::cout << "Test skipped (running on Solaris or under sanitizer)" - << bsl::endl; - return; // RETURN - } - Tester t(s_allocator_p); // Concerns 'a' @@ -1232,22 +1186,6 @@ static void test3_watermarkTest() { mwctst::TestHelper::printTestName("Watermark Test"); - if (skipTest) { - // This test has been disabled for Solaris/MSan/TSan/UBSan build. This - // test relies on the timings of certain callbacks being fired before - // or after certain operations. Normally this timing is always - // observed, but in msan/tsan/ubsan enabled build, the timing gets - // changed, leading to test failure. Of course, the right fix is to - // not rely on these timings, which can be worked on if the test starts - // failing in non-instrumented builds. Additionally, we could try to - // enable this test in MSan/TSan/UBSan build once all MSan/TSan/UBSan - // reports have been fixed to see if that helps (see `msansup.txt`, - // `tsansup.txt` and `ubsansup.txt`). - bsl::cout << "Test skipped (running on Solaris or under sanitizer)" - << bsl::endl; - return; // RETURN - } - Tester t(s_allocator_p); // Concern 'a' @@ -1337,22 +1275,6 @@ static void test1_breathingTest() { mwctst::TestHelper::printTestName("Breathing Test"); - if (skipTest) { - // This test has been disabled for Solaris/MSan/TSan/UBSan build. This - // test relies on the timings of certain callbacks being fired before - // or after certain operations. Normally this timing is always - // observed, but in msan/tsan/ubsan enabled build, the timing gets - // changed, leading to test failure. Of course, the right fix is to - // not rely on these timings, which can be worked on if the test starts - // failing in non-instrumented builds. Additionally, we could try to - // enable this test in MSan/TSan/UBSan build once all MSan/TSan/UBSan - // reports have been fixed to see if that helps (see `msansup.txt`, - // `tsansup.txt` and `ubsansup.txt`). - bsl::cout << "Test skipped (running on Solaris or under sanitizer)" - << bsl::endl; - return; // RETURN - } - Tester t(s_allocator_p); t.init(L_); diff --git a/src/groups/mwc/mwcio/mwcio_testchannel.cpp b/src/groups/mwc/mwcio/mwcio_testchannel.cpp index 446ffb9ebf..1b41dd9aab 100644 --- a/src/groups/mwc/mwcio/mwcio_testchannel.cpp +++ b/src/groups/mwc/mwcio/mwcio_testchannel.cpp @@ -272,6 +272,12 @@ TestChannel::CloseCall TestChannel::popCloseCall() return result; } +bool TestChannel::closeCallsEmpty() +{ + bslmt::LockGuard guard(&d_mutex); // LOCK + return d_closeCalls.empty(); +} + TestChannel::CloseSignaler& TestChannel::closeSignaler() { return d_closeSignaler; diff --git a/src/groups/mwc/mwcio/mwcio_testchannel.h b/src/groups/mwc/mwcio/mwcio_testchannel.h index 3efd2ce60f..49357b5618 100644 --- a/src/groups/mwc/mwcio/mwcio_testchannel.h +++ b/src/groups/mwc/mwcio/mwcio_testchannel.h @@ -248,6 +248,8 @@ class TestChannel : public Channel { /// Pops a close-call from those written to the channel (FIFO ordering). CloseCall popCloseCall(); + bool closeCallsEmpty(); + CloseSignaler& closeSignaler(); /// Return a reference providing modifiable access to the corresponding diff --git a/src/groups/mwc/mwcma/mwcma_countingallocator.t.cpp b/src/groups/mwc/mwcma/mwcma_countingallocator.t.cpp index 2285f880ef..d5df7f7f5e 100644 --- a/src/groups/mwc/mwcma/mwcma_countingallocator.t.cpp +++ b/src/groups/mwc/mwcma/mwcma_countingallocator.t.cpp @@ -191,23 +191,24 @@ static void test3_deallocate() mwctst::TestHelper::printTestName("DEALLOCATE"); // This test twice-deallocates the same block of memory, to verify that - // such an operation fails. If we're running under MemorySanitizer or - // AddressSanitizer, we must skip this test to avoid detecting the issue - // and aborting. + // such an operation fails. If we're running under MemorySanitizer, + // AddressSanitizer or ThreadSanitizer, we must skip this test to avoid + // detecting the issue and aborting. // // Under MSan, we would instead try to explicitly "unpoison" the memory, // but CountingAllocator keeps a hidden "header" block, which we have no // good way of accessing to unpoison. // // Under ASan, we might be able to use the `no_sanitize` attribute, but - // GCC doesn't support it before versio 8.0 - so for now, better just to + // GCC doesn't support it before version 8.0 - so for now, better just to // skip the testcase. #if defined(__has_feature) // Clang-supported method for checking sanitizers. const bool skipTestForSanitizers = __has_feature(address_sanitizer) || __has_feature(memory_sanitizer) || __has_feature(thread_sanitizer); -#elif defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_THREAD__) - // GCC-supported macros for checking ASAN and TSAN. +#elif defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_MEMORY__) || \ + defined(__SANITIZE_THREAD__) + // GCC-supported macros for checking ASAN, MSAN and TSAN. const bool skipTestForSanitizers = true; #else const bool skipTestForSanitizers = false; // Default to running the test.