From acd4dbbf2fd695d5099fc67c1d8e62c36bbefe64 Mon Sep 17 00:00:00 2001 From: David Allison Date: Sun, 3 May 2026 12:24:43 -0700 Subject: [PATCH] Add TSAN fiber-switching support, sanitizer/valgrind bazel configs, and CI Following the same pattern as the existing AddressSanitizer integration, the coroutine library now cooperates with ThreadSanitizer via its fiber API (__tsan_create_fiber / __tsan_switch_to_fiber / ...) on every coroutine context switch, eliminating spurious data-race reports across cooperative yields between the scheduler and its coroutines. * detect_sanitizers.h: add CO_THREAD_SANITIZER and CO_DISABLE_THREAD_SANITIZER, mirroring the ASAN macros. * coroutine.h: forward-declare the TSan fiber API and add a per-coroutine tsan_fiber_ plus a per-scheduler tsan_fiber_. * coroutine.cc: switch fibers on every Resume/Yield/Exit and at the end of InvokeFunction; create and name a fiber in the Coroutine ctor, destroy it in the dtor, and capture the scheduler thread's fiber lazily in Run(). Also adds .bazelrc configurations for asan, tsan, and valgrind so the test suite can be exercised under each, e.g. bazel test --config=asan //co:coroutines_test //co:test_cpp20 bazel test --config=tsan //co:coroutines_test //co:test_cpp20 bazel test --config=valgrind //co:coroutines_test //co:test_cpp20 And adds a GitHub Actions CI workflow (.github/workflows/ci.yml) modeled on the one used by dallison/subspace, extended to exercise every config above on every push and pull request. --- .bazelrc | 63 ++++++++++++++++--- .github/workflows/ci.yml | 132 +++++++++++++++++++++++++++++++++++++++ co/coroutine.cc | 82 ++++++++++++++++++++++-- co/coroutine.h | 30 +++++++++ co/detect_sanitizers.h | 27 ++++++++ 5 files changed, 322 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.bazelrc b/.bazelrc index 7b1f772..e60c4ad 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,10 +1,3 @@ -build:asan --strip=never -build:asan --copt -fsanitize=address -build:asan --copt -DADDRESS_SANITIZER -build:asan --copt -g -build:asan --copt -fno-omit-frame-pointer -build:asan --linkopt -fsanitize=address - # For all builds, use C++17 build --cxxopt="-std=c++17" @@ -12,3 +5,59 @@ build --cxxopt="-std=c++17" build:apple_silicon --cpu=darwin_arm64 build:apple_silicon --features=oso_prefix_is_pwd +# ----------------------------------------------------------------------------- +# Sanitizer / dynamic-analysis configurations. +# +# Pick one with `--config=`, e.g. +# bazel test --config=asan //co:coroutines_test +# bazel test --config=tsan //co:coroutines_test +# bazel test --config=valgrind //co:coroutines_test +# +# All three configs preserve debug information so failures point back to +# meaningful source locations. +# ----------------------------------------------------------------------------- + +# AddressSanitizer. Detects use-after-free, heap/stack/global buffer +# overflows, use-after-return, etc. The coroutine library cooperates with +# ASan via __sanitizer_start/finish_switch_fiber on every context switch. +build:asan --strip=never +build:asan --copt=-fsanitize=address +build:asan --copt=-DADDRESS_SANITIZER +build:asan --copt=-g +build:asan --copt=-O1 +build:asan --copt=-fno-omit-frame-pointer +build:asan --linkopt=-fsanitize=address +# Note: leak detection (LSan) is only supported on Linux; opting out keeps the +# config portable to macOS. On Linux you can enable it with +# --test_env=ASAN_OPTIONS=halt_on_error=1:detect_leaks=1 +test:asan --test_env=ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:detect_leaks=0:print_summary=1 + +# ThreadSanitizer. Detects data races and several kinds of synchronization +# bugs. The coroutine library cooperates with TSan via the fiber API +# (__tsan_create_fiber / __tsan_switch_to_fiber / ...) so cooperative yields +# between the scheduler and its coroutines do not look like races. +build:tsan --strip=never +build:tsan --copt=-fsanitize=thread +build:tsan --copt=-DTHREAD_SANITIZER +build:tsan --copt=-g +build:tsan --copt=-O1 +build:tsan --copt=-fno-omit-frame-pointer +build:tsan --linkopt=-fsanitize=thread +test:tsan --test_env=TSAN_OPTIONS=halt_on_error=1:second_deadlock_stack=1:history_size=7 + +# Valgrind. Runs the unmodified binary under Memcheck. The coroutine +# library already calls VALGRIND_STACK_REGISTER for every coroutine stack +# (when valgrind/valgrind.h is available at compile time) so unwinding works +# correctly across switches. +# +# Notes: +# * Valgrind is not available on macOS arm64. Use Linux (or x86_64) to +# exercise this config. +# * --error-exitcode=1 makes the test fail on any reported error. +# * --child-silent-after-fork=yes suppresses noise from forking helpers. +build:valgrind --strip=never +build:valgrind --copt=-g +build:valgrind --copt=-O1 +build:valgrind --copt=-fno-omit-frame-pointer +test:valgrind --run_under='valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=definite,possible --track-origins=yes --child-silent-after-fork=yes --trace-children=yes' +test:valgrind --test_timeout=300 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..39e8101 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,132 @@ +name: CI + +on: [push, pull_request] + +jobs: + # Plain build + test on each supported host. Mirrors the workflow used by + # the dallison/subspace repository so behavior stays consistent across + # projects. + test: + name: Build & test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + - os: ubuntu-latest + - os: ubuntu-24.04-arm + - os: macos-latest + bazel_flags: --config=apple_silicon + + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Install Bazel + uses: bazel-contrib/setup-bazel@0.18.0 + with: + # Avoid downloading Bazel every time. + bazelisk-cache: true + # Store the build cache per workflow. + disk-cache: ${{ github.workflow }}-${{ matrix.os }} + # Share the repository cache between workflows. + repository-cache: true + + - name: Build all targets + run: | + bazel build //... \ + --verbose_failures \ + ${{ matrix.bazel_flags }} + + - name: Run tests + run: | + bazel test //... \ + --verbose_failures \ + --test_output=errors \ + ${{ matrix.bazel_flags }} + + - name: Upload Bazel test logs + uses: actions/upload-artifact@v7 + if: failure() + with: + name: bazel-test-logs-${{ matrix.os }} + path: bazel-testlogs + + # Run the test suite under AddressSanitizer and ThreadSanitizer. These + # exercise the cooperation between the coroutine library and the sanitizer + # runtimes (start_switch_fiber / __tsan_switch_to_fiber) on every context + # switch. Linux only because LSan + ASan, and TSan, behave most uniformly + # there. + sanitizers: + name: ${{ matrix.config }} + runs-on: ubuntu-latest + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + config: [asan, tsan] + + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Install Bazel + uses: bazel-contrib/setup-bazel@0.18.0 + with: + bazelisk-cache: true + disk-cache: ${{ github.workflow }}-${{ matrix.config }} + repository-cache: true + + - name: Run tests under ${{ matrix.config }} + run: | + bazel test //... \ + --config=${{ matrix.config }} \ + --verbose_failures \ + --test_output=errors + + - name: Upload Bazel test logs + uses: actions/upload-artifact@v7 + if: failure() + with: + name: bazel-test-logs-${{ matrix.config }} + path: bazel-testlogs + + # Run the test suite under Valgrind's Memcheck. The library calls + # VALGRIND_STACK_REGISTER for every coroutine stack so unwinding works + # correctly across switches. Valgrind is only available on Linux/x86_64 + # in practice. + valgrind: + name: valgrind + runs-on: ubuntu-latest + timeout-minutes: 45 + + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Install Valgrind + run: | + sudo apt-get update + sudo apt-get install -y valgrind + + - name: Install Bazel + uses: bazel-contrib/setup-bazel@0.18.0 + with: + bazelisk-cache: true + disk-cache: ${{ github.workflow }}-valgrind + repository-cache: true + + - name: Run tests under Valgrind + run: | + bazel test //... \ + --config=valgrind \ + --verbose_failures \ + --test_output=errors + + - name: Upload Bazel test logs + uses: actions/upload-artifact@v7 + if: failure() + with: + name: bazel-test-logs-valgrind + path: bazel-testlogs diff --git a/co/coroutine.cc b/co/coroutine.cc index a25b667..20797b8 100644 --- a/co/coroutine.cc +++ b/co/coroutine.cc @@ -45,17 +45,40 @@ constexpr bool kCoDebug = false; +// Tell ThreadSanitizer that we are about to switch to the given fiber. Must +// be called immediately before the actual context switch instruction (longjmp, +// setcontext, ...) so that subsequent code is attributed to the new fiber and +// the appropriate happens-before relation between fibers is established. See +// https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/sanitizer/tsan_interface.h +#if defined(CO_THREAD_SANITIZER) +#define CO_TSAN_SWITCH_TO_FIBER(fiber) \ + do { \ + if ((fiber) != nullptr) { \ + __tsan_switch_to_fiber((fiber), 0); \ + } \ + } while (0) +#else +#define CO_TSAN_SWITCH_TO_FIBER(fiber) ((void)0) +#endif + #if CO_CTX_MODE == CO_CTX_SETJMP #define SETCONTEXT(ctx) __real_longjmp(ctx, 1) #define GETCONTEXT(ctx) setjmp(ctx) #define SWAPCONTEXT(from, to) \ - if (setjmp(from) == 0) { \ - __real_longjmp(to, 1); \ - } + do { \ + CO_TSAN_SWITCH_TO_FIBER(scheduler_.tsan_fiber_); \ + if (setjmp(from) == 0) { \ + __real_longjmp(to, 1); \ + } \ + } while (0) #elif CO_CTX_MODE == CO_CTX_UCONTEXT #define SETCONTEXT(ctx) setcontext(&ctx) #define GETCONTEXT(ctx) getcontext(&ctx) -#define SWAPCONTEXT(from, to) swapcontext(&from, &to) +#define SWAPCONTEXT(from, to) \ + do { \ + CO_TSAN_SWITCH_TO_FIBER(scheduler_.tsan_fiber_); \ + swapcontext(&from, &to); \ + } while (0) #else #define SETCONTEXT(ctx) CoroutineSetContext(&ctx) #define GETCONTEXT(ctx) CoroutineGetContext(&ctx) @@ -66,6 +89,7 @@ constexpr bool kCoDebug = false; #define SWAPCONTEXT(from, to) \ do { \ __sanitizer_finish_switch_fiber(scheduler_.fake_stack_, nullptr, nullptr); \ + CO_TSAN_SWITCH_TO_FIBER(scheduler_.tsan_fiber_); \ CoroutineSwapContext(&from, &to); \ __sanitizer_start_switch_fiber(&scheduler_.fake_stack_, stack_.data(), \ stack_.size()); \ @@ -73,6 +97,7 @@ constexpr bool kCoDebug = false; #else #define SWAPCONTEXT(from, to) \ do { \ + CO_TSAN_SWITCH_TO_FIBER(scheduler_.tsan_fiber_); \ CoroutineSwapContext(&from, &to); \ } while (0) #endif @@ -313,6 +338,16 @@ Coroutine::Coroutine(CoroutineScheduler &scheduler, stack_.data(), static_cast(stack_.data()) + stack_.size()); #endif +#if defined(CO_THREAD_SANITIZER) + // Create a TSan fiber for this coroutine. TSan will use it to attribute + // memory operations and synchronization to the right logical thread when we + // call __tsan_switch_to_fiber on every context switch. + tsan_fiber_ = __tsan_create_fiber(0); + if (tsan_fiber_ != nullptr) { + __tsan_set_fiber_name(tsan_fiber_, name_.c_str()); + } +#endif + event_fd_ = EventFd::Create(); if (!event_fd_.IsValid()) { fprintf(stderr, "Failed to allocate event fd: %s\n", strerror(errno)); @@ -345,6 +380,12 @@ Coroutine::~Coroutine() { VALGRIND_STACK_DEREGISTER(valgrind_stack_id_); } #endif +#if defined(CO_THREAD_SANITIZER) + if (tsan_fiber_ != nullptr) { + __tsan_destroy_fiber(tsan_fiber_); + tsan_fiber_ = nullptr; + } +#endif } const char *Coroutine::StateName(State state) { @@ -420,7 +461,12 @@ void Coroutine::SetState(State state) const { state_ = state; } -void Coroutine::Exit() const { SETCONTEXT(exit_); } +void Coroutine::Exit() const { + // Tell TSan we are leaving the coroutine fiber and returning to the + // scheduler before we actually switch stacks. + CO_TSAN_SWITCH_TO_FIBER(scheduler_.tsan_fiber_); + SETCONTEXT(exit_); +} void Coroutine::Start() { if (state_ == State::kCoNew) { @@ -961,6 +1007,16 @@ void Coroutine::InvokeFunction() { } #if defined(CO_ADDRESS_SANITIZER) __sanitizer_finish_switch_fiber(nullptr, nullptr, nullptr); +#endif + // The coroutine body has returned; the assembly trampoline (or the + // ucontext uc_link/getcontext+setcontext path) is about to longjmp/setcontext + // back to the scheduler. Tell TSan we are switching fibers before that + // happens. We use the no-sync flag because there is nothing the scheduler + // can synchronize with on a dying coroutine. +#if defined(CO_THREAD_SANITIZER) + if (scheduler_.tsan_fiber_ != nullptr) { + __tsan_switch_to_fiber(scheduler_.tsan_fiber_, 0); + } #endif } @@ -974,6 +1030,7 @@ void __co_Invoke(Coroutine *c) { c->InvokeFunction(); } } CO_DISABLE_ADDRESS_SANITIZER +CO_DISABLE_THREAD_SANITIZER void Coroutine::Resume(int value) const { if (aborted_) { // Cannot resume an aborted coroutine. @@ -997,6 +1054,9 @@ void Coroutine::Resume(int value) const { __sanitizer_start_switch_fiber(&scheduler_.fake_stack_, stack_.data(), stack_.size()); #endif + // Tell TSan we are about to switch from the scheduler to this + // coroutine's fiber before doing the actual stack switch below. + CO_TSAN_SWITCH_TO_FIBER(tsan_fiber_); SetState(State::kCoRunning); yielded_address_ = nullptr; #if CO_CTX_MODE == CO_CTX_SETJMP @@ -1073,6 +1133,8 @@ void Coroutine::Resume(int value) const { case State::kCoWaiting: SetState(State::kCoRunning); wait_result_ = value; + // Switch TSan over to the coroutine's fiber before we resume it. + CO_TSAN_SWITCH_TO_FIBER(tsan_fiber_); SETCONTEXT(resume_); break; case State::kCoRunning: @@ -1244,6 +1306,16 @@ void CoroutineScheduler::BuildPollFds(PollState *poll_state) { void CoroutineScheduler::Run() { running_ = true; co::scheduler = this; // Thread local. +#if defined(CO_THREAD_SANITIZER) + // Capture the TSan fiber for the thread that runs the scheduler loop. It + // has to be captured here (rather than in the constructor) because the + // scheduler may have been constructed on a different thread from the one + // that calls Run(). __tsan_get_current_fiber returns the implicit fiber + // associated with the calling thread. + if (tsan_fiber_ == nullptr) { + tsan_fiber_ = __tsan_get_current_fiber(); + } +#endif #if CO_POLL_MODE == CO_POLL_EPOLL std::vector epoll_events; #endif diff --git a/co/coroutine.h b/co/coroutine.h index 6b9193a..e9d1e79 100644 --- a/co/coroutine.h +++ b/co/coroutine.h @@ -216,6 +216,19 @@ void __sanitizer_finish_switch_fiber(void *fake_stack_save, } #endif +#if defined(CO_THREAD_SANITIZER) +extern "C" { +// Fiber switching API exposed by the ThreadSanitizer runtime. We forward +// declare the entry points we use rather than including +// because that header is not always available on every toolchain. +void *__tsan_get_current_fiber(void); +void *__tsan_create_fiber(unsigned flags); +void __tsan_destroy_fiber(void *fiber); +void __tsan_switch_to_fiber(void *fiber, unsigned flags); +void __tsan_set_fiber_name(void *fiber, const char *name); +} +#endif + namespace co { class CoroutineScheduler; @@ -335,6 +348,10 @@ struct WaitFd { // errors (use-after-return). The principal function of a coroutine is likely // to need to be prefixed with CO_DISABLE_ADDRESS_SANITIZER // (detect_sanitizers.h) to disable diagnostics related to its stack frame. +// +// ThreadSanitizer is also informed about every coroutine context switch via the +// __tsan_switch_to_fiber API, so cooperative yields between coroutines and the +// scheduler do not trigger spurious data-race reports. class Coroutine { public: // Important note: when using an interrupt_fd, you need to be careful @@ -648,6 +665,12 @@ class Coroutine { #if CO_HAVE_VALGRIND int valgrind_stack_id_ = -1; #endif +#if defined(CO_THREAD_SANITIZER) + // Opaque per-coroutine fiber context owned by the TSan runtime. Created in + // the constructor and destroyed in the destructor. Switched to whenever the + // scheduler resumes this coroutine. + void *tsan_fiber_ = nullptr; +#endif }; // A Generator is a coroutine that generates values. The magic lamda line @@ -805,6 +828,13 @@ class CoroutineScheduler { absl::flat_hash_set deletions_; #if defined(CO_ADDRESS_SANITIZER) void *fake_stack_ = nullptr; +#endif +#if defined(CO_THREAD_SANITIZER) + // TSan fiber identifying the thread that runs the scheduler loop. Captured + // lazily on the first call to Run() (which is when we know which thread the + // scheduler is executing on) and switched to whenever a coroutine yields + // back to the scheduler. + void *tsan_fiber_ = nullptr; #endif std::atomic aborts_enabled_ = true; std::atomic abort_on_stop_ = false; diff --git a/co/detect_sanitizers.h b/co/detect_sanitizers.h index 35ef9e2..b11abbb 100644 --- a/co/detect_sanitizers.h +++ b/co/detect_sanitizers.h @@ -11,6 +11,13 @@ // The longjmp / ucontext or any equivalent stack switching mechanism necessary for coroutines // will trigger false-positives with address-sanitizer. // +// Likewise, defined(CO_THREAD_SANITIZER) is produced when thread sanitizer is in use, and +// CO_DISABLE_THREAD_SANITIZER is the corresponding attribute to disable thread sanitizer +// instrumentation on a function or variable. Stack switching also confuses thread sanitizer +// (longjmp is intercepted and assumed to target the same stack as the matching setjmp), so +// the coroutine implementation cooperates with TSan via its fiber API +// (__tsan_create_fiber/__tsan_switch_to_fiber/...) to tell it about every context switch. +// // NOTE: Users will likely have to prefix their coroutine functions with CO_DISABLE_ADDRESS_SANITIZER. #ifndef CO_ADDRESS_SANITIZER @@ -33,4 +40,24 @@ #define CO_DISABLE_ADDRESS_SANITIZER #endif // CO_ADDRESS_SANITIZER +#ifndef CO_THREAD_SANITIZER +#if defined(__has_feature) +#if __has_feature(thread_sanitizer) // for clang +#define CO_THREAD_SANITIZER +#endif +#else // defined(__has_feature) +#if defined(__SANITIZE_THREAD__) // for gcc +#define CO_THREAD_SANITIZER +#endif +#endif // defined(__has_feature) +#endif // CO_THREAD_SANITIZER + +#ifdef CO_THREAD_SANITIZER +#ifndef CO_DISABLE_THREAD_SANITIZER +#define CO_DISABLE_THREAD_SANITIZER __attribute__((no_sanitize("thread"))) +#endif // CO_DISABLE_THREAD_SANITIZER +#else +#define CO_DISABLE_THREAD_SANITIZER +#endif // CO_THREAD_SANITIZER + #endif // coroutine_detect_sanitizers_h