diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index e9e395b..104dc63 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -44,14 +44,49 @@ jobs: - name: Build and run under ThreadSanitizer run: | + # TSan needs personality(ADDR_NO_RANDOMIZE) to disable ASLR for + # itself; some containerized runners (including this project's + # own registered Gitea runner, confirmed empirically, not just + # hypothesized — see CONTRIBUTING.md and CLAUDE.md) block that + # syscall outright via their default seccomp profile, in which + # case every TSan binary fails identically with `FATAL: + # ThreadSanitizer: unexpected memory mapping`, regardless of + # PackFS's own correctness. Treating that exact, specific + # failure signature as fatal would make CI permanently red for + # a reason that has nothing to do with the code under test — so + # this step distinguishes it from a real finding (a genuine data + # race, a crash, or any other failure) rather than either + # blanket-ignoring TSan failures (which would also hide a real + # race) or blanket-failing the build on an environment limitation + # this repository doesn't control. mkdir -p build/tsan for f in src/*.c; do cc -std=c11 -Wall -Wextra -O1 -g -fPIC -Iinclude -Isrc -D_GNU_SOURCE \ -fsanitize=thread -c "$f" -o "build/tsan/$(basename "${f%.c}").o" done + KNOWN_FLAKE=0 + REAL_FAILURE=0 for t in tests/test_*.c; do name=$(basename "${t%.c}") cc -std=c11 -O1 -g -Iinclude -Isrc -D_GNU_SOURCE -fsanitize=thread \ "$t" build/tsan/*.o -lpthread -o "build/tsan/$name" - "./build/tsan/$name" + OUT=$("./build/tsan/$name" 2>&1) + RC=$? + if [ $RC -eq 0 ]; then + echo "$OUT" + elif echo "$OUT" | grep -q "FATAL: ThreadSanitizer: unexpected memory mapping" \ + && ! echo "$OUT" | grep -qE "WARNING: ThreadSanitizer: |SUMMARY: ThreadSanitizer:"; then + echo "::warning::$name — known runner limitation (TSan can't start here), not a code finding: $OUT" + KNOWN_FLAKE=1 + else + echo "::error::$name — real ThreadSanitizer failure: $OUT" + REAL_FAILURE=1 + fi done + if [ $REAL_FAILURE -ne 0 ]; then + echo "One or more binaries failed ThreadSanitizer for a reason other than the known runner limitation. Failing the build." + exit 1 + fi + if [ $KNOWN_FLAKE -ne 0 ]; then + echo "ThreadSanitizer could not run at all on this runner (known limitation, not a code issue) — this step is not treated as a build failure, but TSan verification did NOT happen this run. See CONTRIBUTING.md." + fi diff --git a/CLAUDE.md b/CLAUDE.md index 01d7b37..127658f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,7 +19,7 @@ A single test: `make build/test_mem && ./build/test_mem` (substitute any `test_* Sanitizer builds are not wired into `make test` (they need per-file compilation with sanitizer flags plus `-D_GNU_SOURCE -Iinclude -Isrc`); see `.gitea/workflows/ci.yml` (Gitea Actions) for the exact invocation, which CI runs on every push. **Any change to `upper.c`, `overlay.c`, or `vfs.c` must be verified under `-fsanitize=address,undefined` and `-fsanitize=thread` before being considered done** — this is not a formality: exactly this process caught a real use-after-free in the snapshot-reclamation logic during initial development (see the `reclaim_gate` note below), which the plain build and even repeated plain test runs never surfaced. -**If your environment cannot run ThreadSanitizer at all, say so rather than skipping it silently.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR for itself; some sandboxes block that syscall outright, in which case *every* TSan build fails identically (`FATAL: ThreadSanitizer: unexpected memory mapping`), including a trivial unrelated pthread program — that is the confirming test, not a PackFS-specific symptom. In that situation, ASan/UBSan are still run and still matter (they are what actually caught the `reclaim_gate` bug above), but they do not perform TSan's happens-before race analysis and are not a substitute for it; a change is TSan-verified only once it has passed on a machine or CI run that can actually execute it, not merely because ASan/UBSan passed. +**If your environment cannot run ThreadSanitizer at all, say so rather than skipping it silently.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR for itself; some sandboxes block that syscall outright, in which case *every* TSan build fails identically (`FATAL: ThreadSanitizer: unexpected memory mapping`), including a trivial unrelated pthread program — that is the confirming test, not a PackFS-specific symptom. **This project's own CI runner is one of them, confirmed by its first real run** (see `CONTRIBUTING.md` for the details and for exactly how `.gitea/workflows/ci.yml`'s TSan step now tells that specific, known failure apart from a real finding rather than either hiding it or failing the build over it) — a green CI run is therefore not proof TSan executed. In that situation, ASan/UBSan are still run and still matter (they are what actually caught the `reclaim_gate` bug above), but they do not perform TSan's happens-before race analysis and are not a substitute for it; a change is TSan-verified only once it has actually passed on a machine confirmed able to execute it, never merely because ASan/UBSan passed or because a CI step reported success. **A related but separate flake affects ASan/UBSan themselves in that kind of sandbox, not just TSan:** a sanitizer-built test binary can non-deterministically fail to start with `AddressSanitizer:DEADLYSIGNAL`, occasionally as an unbounded repeating loop rather than a single line — a sandbox startup race, confirmed by it hitting different, unrelated binaries across repeated runs, each of which then passes cleanly on retry. See `CONTRIBUTING.md`'s workflow section for the full description and the required mitigation (`timeout`-wrap sanitizer runs in such an environment; treat `DEADLYSIGNAL` alone, without an actual `ERROR: AddressSanitizer` or `runtime error:` string, as inconclusive and re-run rather than as a finding). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e419c3..89f63e9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,13 +43,22 @@ project's development, see the `reclaim_gate` note in `internal.h`) but do not do TSan's happens-before race analysis, so they are not a substitute for it. CI (`.gitea/workflows/ci.yml`, Gitea Actions — this project is hosted on Gitea, not GitHub) runs on this project's own self-hosted -runner; whether that runner can execute TSan depends on that runner's own -environment, which is not controlled by this repository the way a -hosted-runner VM would be — do not assume CI's TSan step passing means -what it would on an unrestricted machine without having actually checked -what environment the registered runner provides. Treat a change as -TSan-verified only once it has actually passed on a machine confirmed able -to run it, not merely because a CI step reported success. +runner. **Confirmed, not just anticipated: that runner cannot run TSan +either.** The first real CI run on it failed the ThreadSanitizer step with +the exact same signature described above (`FATAL: ThreadSanitizer: +unexpected memory mapping`), consistent with the runner's job containers +using a default seccomp profile that blocks `personality()` the same way +some local sandboxes do. The CI step now detects this specific failure +signature and does not fail the build over it (while still failing hard on +any *other* TSan outcome — a real race, a crash, anything without that +exact signature) — see the step's own comment in `ci.yml` for the +detection logic. This means **CI's ThreadSanitizer step passing is not +evidence that TSan actually ran** on a given push; it may just mean the +runner couldn't start it and the step correctly didn't treat that as a +failure. Treat a change as TSan-verified only once it has actually passed +on a machine confirmed able to run it (a local machine or container with +`personality(ADDR_NO_RANDOMIZE)` available), not merely because CI is +green. **A second, separate environment quirk, also observed directly rather than assumed:** in the same kind of sandboxed environment, an ASan/UBSan-built diff --git a/README.md b/README.md index 59ead0c..0267316 100644 --- a/README.md +++ b/README.md @@ -259,8 +259,20 @@ cell directly and never touch the snapshot. `mem`-backed buffer growth never mutates a buffer address a reader might be reading (Section 5.7): growth always allocates a new buffer and publishes it, never reallocates in place. `tests/test_concurrency.c` exercises this under concurrent reader and -writer threads, and the suite is regularly run under ThreadSanitizer and -AddressSanitizer (see `.gitea/workflows/ci.yml`). +writer threads. The suite is regularly run under AddressSanitizer/ +UndefinedBehaviorSanitizer, both locally and in CI (see +`.gitea/workflows/ci.yml`), and this is not aspirational — ASan caught a +real heap-use-after-free in the snapshot-reclamation logic during +development (see the `reclaim_gate` note in `src/internal.h`). +ThreadSanitizer is configured the same way but, as of this writing, has +not actually completed a run in either environment this project has been +built and tested in so far — the local development sandbox and this +project's own CI runner both block the `personality(ADDR_NO_RANDOMIZE)` +syscall TSan needs to start (see `CONTRIBUTING.md` for the confirming +tests in each case). CI's TSan step is written to not fail the build over +that specific, known-benign failure, which means a green CI run is not +evidence TSan actually executed — stated plainly here rather than left to +be assumed from CI showing green. ## Security