fiber: fix TSan SEGV on fiber switch (func-entry/exit shadow-stack underflow) (#134)#197
Merged
Merged
Conversation
…ks (#134) Under TSan with clang, `bazel test --config=tsan //flare/fiber:*` crashes on the first fiber resume: ThreadSanitizer: SEGV ... in __tsan_func_entry <- FiberProc (first instruction of a freshly-started fiber) Root cause (traced with gdb): TSan keeps a per-fiber "shadow stack" of call frames; `__tsan_func_entry` pushes and `__tsan_func_exit` pops it. `flare::internal::tsan::SwitchToFiber` calls `__tsan_switch_to_fiber`, which makes the *target* fiber's shadow stack active. But that call sits inside an instrumented function, so the function's own injected `__tsan_func_exit` runs *after* the switch -- popping the just-activated, and for a brand-new fiber empty, shadow stack. shadow_stack_pos underflows to base-8; the next `__tsan_func_entry` (at FiberProc) writes below the mapped region and SEGVs. Confirmed: `-fno-sanitize-thread-func-entry-exit` (which removes those hooks) makes it pass. Crucially, plain `[[no_sanitize("thread")]]` does NOT fix it -- it disables the data-race read/write checks but keeps the func-entry/exit hooks, so the underflow remains. Fix: add `FLARE_INTERNAL_DISABLE_SANITIZER_INSTRUMENTATION` (`[[clang::disable_sanitizer_instrumentation]]`, which strips ALL hooks including func-entry/exit) and apply it to the functions that bracket a raw context switch: the `SwitchToFiber` wrapper, `FiberEntity::Resume`, `FiberEntity::ResumeOn`, and the `FiberProc` entry trampoline. This keeps full TSan instrumentation (and call stacks in race reports) everywhere else, unlike the global compile flag. GCC has no such attribute (only Clang), so the macro expands to nothing there -- GCC builds are non-sanitized, and TSan-with-fibers is a Clang-only configuration in practice. Verified with clang-17 under `--config=tsan` on both x86_64 and aarch64: future_test / fiber_test / timer_test / this_fiber_test all pass. ASan (#196) and the normal non-sanitized build are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
65a327e to
491b65e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the TSan half of #134 (the ASan half landed in #196).
Symptom
bazel test --config=tsan //flare/fiber:*(clang) crashes on the first fiber resume:Root cause (traced with gdb)
TSan keeps a per-fiber shadow stack of call frames:
__tsan_func_entrypushes,__tsan_func_exitpops, and__tsan_switch_to_fibermakes a given fiber's shadow stack the active one.Tracing
shadow_stack_pos(ThreadState+16) across the switch:flare::internal::tsan::SwitchToFibercalls__tsan_switch_to_fiberfrom inside an instrumented function. So the sequence is:__tsan_func_entry→ push onto the old fiber's shadow stack__tsan_switch_to_fiber(new)→ active stack = the new fiber's (empty,pos = base)__tsan_func_exit(the wrapper's own, on return) → pop the new fiber's empty stack →pos = base − 8FiberProc→__tsan_func_entrywrites atbase − 8(unmapped) → SEGVTwo things this rules out, both verified:
[[no_sanitize("thread")]]does NOT fix it — it disables the data-race read/write checks but keeps the func-entry/exit hooks, so the underflow remains. (-fno-sanitize-thread-func-entry-exitdoes fix it, confirming the mechanism.)Fix
Add
FLARE_INTERNAL_DISABLE_SANITIZER_INSTRUMENTATION([[clang::disable_sanitizer_instrumentation]], which strips all hooks incl. func-entry/exit — unlikeno_sanitize) and apply it to the functions that bracket a raw context switch:flare::internal::tsan::SwitchToFiber(the wrapper around__tsan_switch_to_fiber)FiberEntity::Resume,FiberEntity::ResumeOn(perform thejump_context)FiberProc(fiber entry trampoline)This keeps full TSan instrumentation — including call stacks in race reports — everywhere else, unlike the global
-fno-sanitize-thread-func-entry-exitflag. A GCC 12+ spelling is provided; on older GCC it expands to nothing (those release builds are non-sanitized, so it's harmless; TSan-with-fibers under GCC < 12 is simply unsupported).Verification (clang-17, Ubuntu 22.04 container)
All pass under
--config=tsan, on both x86_64 (the report's arch) and aarch64:future_test,fiber_test,timer_test,this_fiber_testASan (#196) and the normal non-sanitized build are unaffected (re-verified).
Follow-up
With #196 + this, both ASan and TSan fiber tests pass under sanitizers. flare's CI is currently blade-based; a
--config=asan/--config=tsanbazel lane over//flare/fiber:...would be a good regression guard — happy to add one if you'd like.🤖 Generated with Claude Code