From 987d32f29d1eb49e26197fa430aa58a38358933e Mon Sep 17 00:00:00 2001 From: RippeR37 Date: Sun, 6 Apr 2025 04:10:39 +0200 Subject: [PATCH 1/3] Add TSAN build option and use ASAN/TSAN in CI builds This commit adds new `LIBBASE_BUILD_TSAN` build option (next to existing `LIBBASE_BUILD_ASAN`) that configures build to use ThreadSanitizer. This also adds new Linux CI jobs that build and run tests and examples with ASAN and TSAN enabled for better testing. --- .github/workflows/ubuntu.yml | 20 +++++++++++++++++++ CMakeLists.txt | 1 + CMakeModules/SetupCompileFlags.cmake | 8 +++++++- src/base/synchronization/waitable_event.cc | 2 +- tests/perf/base/threading/thread_perftests.cc | 4 ++-- .../delayed_task_manager_unittests.cc | 2 +- 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index de1f6ea69b..72d7a68547 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -51,6 +51,18 @@ jobs: cmake_args: [""] code_coverage: [false] include: + - os_compiler: {os: ubuntu-24.04, name: Clang 18 (ASAN), cc: clang-18, cxx: clang++-18, tidy: clang-tidy-18 } + build_type: Debug + cmake_args: "-DLIBBASE_BUILD_ASAN=ON" + code_coverage: false + package: false + run_all: true + - os_compiler: {os: ubuntu-24.04, name: Clang 18 (TSAN), cc: clang-18, cxx: clang++-18, tidy: clang-tidy-18 } + build_type: Debug + cmake_args: "-DLIBBASE_BUILD_TSAN=ON" + code_coverage: false + package: false + run_all: true - os_compiler: {os: ubuntu-24.04, name: CodeCoverage (gcov), cc: gcc-13, cxx: g++-13} build_type: Debug cmake_args: "-DLIBBASE_CODE_COVERAGE=ON" @@ -111,6 +123,14 @@ jobs: - name: Testing run: ctest --test-dir build --output-on-failure --timeout 15 + # (Optional) Run other tests and examples + - name: (Optional) Run performance tests + if: ${{ matrix.run_all }} + run: ./build/tests/perf/libbase_perf_tests + - name: (Optional) Run examples + if: ${{ matrix.run_all }} + run: ./build/examples/simple/simple + # (Optional) Generate code coverage report and upload it # - 'gcov'-based is used with CI # - 'llvm-cov'-based is uploaded as an artifact diff --git a/CMakeLists.txt b/CMakeLists.txt index aa6b6fd1d9..0a3e2f759a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,6 +47,7 @@ else() endif() option(LIBBASE_BUILD_ASAN "Build with Address Sanitizer enabled" OFF) +option(LIBBASE_BUILD_TSAN "Build with Thread Sanitizer enabled" ON) # diff --git a/CMakeModules/SetupCompileFlags.cmake b/CMakeModules/SetupCompileFlags.cmake index 3eee431837..ed92358985 100644 --- a/CMakeModules/SetupCompileFlags.cmake +++ b/CMakeModules/SetupCompileFlags.cmake @@ -50,8 +50,14 @@ function(SETUP_COMPILE_FLAGS) set(CLANG_TIDY_PROPERTIES "CXX_CLANG_TIDY;${CLANG_TIDY_EXE}") endif() + if(LIBBASE_BUILD_ASAN AND LIBBASE_BUILD_TSAN) + message(FATAL_ERROR "Cann't use ASAN and TSAN within same build") + endif() + if(LIBBASE_BUILD_ASAN) - set(SANITIZE_FLAGS ";-fsanitize=address") + set(SANITIZE_FLAGS ";-fsanitize=address,undefined") + elseif(LIBBASE_BUILD_TSAN) + set(SANITIZE_FLAGS ";-fsanitize=thread") endif() if(NOT CONFIGURED_ONCE) diff --git a/src/base/synchronization/waitable_event.cc b/src/base/synchronization/waitable_event.cc index 30c908c685..28b029338c 100644 --- a/src/base/synchronization/waitable_event.cc +++ b/src/base/synchronization/waitable_event.cc @@ -18,8 +18,8 @@ void WaitableEvent::Signal() { { std::lock_guard guard{mutex_}; is_signaled_ = true; + cond_var_.notify_one(); } - cond_var_.notify_one(); } bool WaitableEvent::IsSignaled() { diff --git a/tests/perf/base/threading/thread_perftests.cc b/tests/perf/base/threading/thread_perftests.cc index 8a5ff9d3df..41078f14d4 100644 --- a/tests/perf/base/threading/thread_perftests.cc +++ b/tests/perf/base/threading/thread_perftests.cc @@ -30,9 +30,9 @@ void TestDoubleThreaded(base::SequencedTaskRunner* tr1, } void BM_TestSingleThreaded(benchmark::State& state) { + base::WaitableEvent event{base::WaitableEvent::ResetPolicy::kAutomatic}; base::Thread t1; t1.Start(); - base::WaitableEvent event{base::WaitableEvent::ResetPolicy::kAutomatic}; for (auto _ : state) { TestSingleThreaded(t1.TaskRunner().get(), 1000, &event); @@ -41,11 +41,11 @@ void BM_TestSingleThreaded(benchmark::State& state) { } void BM_TestDoubleThreaded(benchmark::State& state) { + base::WaitableEvent event{base::WaitableEvent::ResetPolicy::kAutomatic}; base::Thread t1; base::Thread t2; t1.Start(); t2.Start(); - base::WaitableEvent event{base::WaitableEvent::ResetPolicy::kAutomatic}; for (auto _ : state) { TestDoubleThreaded(t2.TaskRunner().get(), t1.TaskRunner().get(), 1000, diff --git a/tests/unit/base/threading/delayed_task_manager_unittests.cc b/tests/unit/base/threading/delayed_task_manager_unittests.cc index 0812e58e91..ee3977bb57 100644 --- a/tests/unit/base/threading/delayed_task_manager_unittests.cc +++ b/tests/unit/base/threading/delayed_task_manager_unittests.cc @@ -16,7 +16,7 @@ namespace { const base::TimeDelta kZero = base::TimeDelta{}; -base::TimeTicks g_now_mocked; +std::atomic g_now_mocked; base::TimeTicks GetMockedTimeTicks() { return g_now_mocked; From a7f67784531dbbf19b37c62b524779764b33e19a Mon Sep 17 00:00:00 2001 From: RippeR37 Date: Sun, 6 Apr 2025 04:26:16 +0200 Subject: [PATCH 2/3] fixup! Add TSAN build option and use ASAN/TSAN in CI builds Disable TSAN by default --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0a3e2f759a..832f089990 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,7 +47,7 @@ else() endif() option(LIBBASE_BUILD_ASAN "Build with Address Sanitizer enabled" OFF) -option(LIBBASE_BUILD_TSAN "Build with Thread Sanitizer enabled" ON) +option(LIBBASE_BUILD_TSAN "Build with Thread Sanitizer enabled" OFF) # From 9077e3673596100745714888e0f5676a6147e592 Mon Sep 17 00:00:00 2001 From: RippeR37 Date: Sun, 6 Apr 2025 04:34:19 +0200 Subject: [PATCH 3/3] fixup! fixup! Add TSAN build option and use ASAN/TSAN in CI builds Cleanup in ubuntu workflow --- .github/workflows/ubuntu.yml | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 72d7a68547..1ceda4a266 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -33,46 +33,41 @@ jobs: build_type: [Release, Debug] os_compiler: [ # ubuntu 22 - {os: ubuntu-22.04, name: GCC 10, cc: gcc-10, cxx: g++-10, package: false}, - {os: ubuntu-22.04, name: GCC 11, cc: gcc-11, cxx: g++-11, package: false}, - {os: ubuntu-22.04, name: GCC 12, cc: gcc-12, cxx: g++-12, package: false}, - {os: ubuntu-22.04, name: Clang 13, cc: clang-13, cxx: clang++-13, tidy: clang-tidy-13, package: false}, - {os: ubuntu-22.04, name: Clang 14, cc: clang-14, cxx: clang++-14, tidy: clang-tidy-14, package: false}, - {os: ubuntu-22.04, name: Clang 15, cc: clang-15, cxx: clang++-15, tidy: clang-tidy-15, package: false}, + {os: ubuntu-22.04, name: GCC 10, cc: gcc-10, cxx: g++-10}, + {os: ubuntu-22.04, name: GCC 11, cc: gcc-11, cxx: g++-11}, + {os: ubuntu-22.04, name: GCC 12, cc: gcc-12, cxx: g++-12}, + {os: ubuntu-22.04, name: Clang 13, cc: clang-13, cxx: clang++-13, tidy: clang-tidy-13}, + {os: ubuntu-22.04, name: Clang 14, cc: clang-14, cxx: clang++-14, tidy: clang-tidy-14}, + {os: ubuntu-22.04, name: Clang 15, cc: clang-15, cxx: clang++-15, tidy: clang-tidy-15}, # ubuntu 24 - {os: ubuntu-24.04, name: GCC 12, cc: gcc-12, cxx: g++-12, package: false}, - {os: ubuntu-24.04, name: GCC 13, cc: gcc-13, cxx: g++-13, package: false}, + {os: ubuntu-24.04, name: GCC 12, cc: gcc-12, cxx: g++-12}, + {os: ubuntu-24.04, name: GCC 13, cc: gcc-13, cxx: g++-13}, {os: ubuntu-24.04, name: GCC 14, cc: gcc-14, cxx: g++-14, package: true}, - {os: ubuntu-24.04, name: Clang 16, cc: clang-16, cxx: clang++-16, tidy: clang-tidy-16, package: false}, - {os: ubuntu-24.04, name: Clang 17, cc: clang-17, cxx: clang++-17, tidy: clang-tidy-17, package: false}, + {os: ubuntu-24.04, name: Clang 16, cc: clang-16, cxx: clang++-16, tidy: clang-tidy-16}, + {os: ubuntu-24.04, name: Clang 17, cc: clang-17, cxx: clang++-17, tidy: clang-tidy-17}, {os: ubuntu-24.04, name: Clang 18, cc: clang-18, cxx: clang++-18, tidy: clang-tidy-18, package: true}, ] - cmake_args: [""] - code_coverage: [false] include: + # ASAN/TSAN builds - os_compiler: {os: ubuntu-24.04, name: Clang 18 (ASAN), cc: clang-18, cxx: clang++-18, tidy: clang-tidy-18 } build_type: Debug cmake_args: "-DLIBBASE_BUILD_ASAN=ON" - code_coverage: false - package: false run_all: true - os_compiler: {os: ubuntu-24.04, name: Clang 18 (TSAN), cc: clang-18, cxx: clang++-18, tidy: clang-tidy-18 } build_type: Debug cmake_args: "-DLIBBASE_BUILD_TSAN=ON" - code_coverage: false - package: false run_all: true + + # Code coverage builds - os_compiler: {os: ubuntu-24.04, name: CodeCoverage (gcov), cc: gcc-13, cxx: g++-13} build_type: Debug cmake_args: "-DLIBBASE_CODE_COVERAGE=ON" code_coverage: true - package: false - os_compiler: {os: ubuntu-24.04, name: CodeCoverage (llvm-cov), cc: clang-18, cxx: clang++-18, tidy: clang-tidy-18, llvm-cov: llvm-cov-18, llvm-profdata: llvm-profdata-18 } build_type: Debug cmake_args: "-DLIBBASE_CODE_COVERAGE=ON" code_coverage: true - package: false env: CC: ${{ matrix.os_compiler.cc }}