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