diff --git a/CMakeLists.txt b/CMakeLists.txt index c2fa5fce..72b4f971 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -74,6 +74,7 @@ set(LIBMCMINI_C_SRC src/lib/sem-wrappers.c src/lib/template/loop.c src/lib/template/sig.c + src/lib/tsan_support.c src/lib/wrappers.c src/mcmini/Thread_queue.c ) diff --git a/docs/superpowers/plans/2026-07-04-tsan-port-phase1-r1-implementation.md b/docs/superpowers/plans/2026-07-04-tsan-port-phase1-r1-implementation.md new file mode 100644 index 00000000..48e729a3 --- /dev/null +++ b/docs/superpowers/plans/2026-07-04-tsan-port-phase1-r1-implementation.md @@ -0,0 +1,498 @@ +# TSan Port Phase 1 (R1): Exclude TSan-internal Threads from the Restart Barrier — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `template_thread()`'s DMTCP-restart thread-count barrier correctly exclude ThreadSanitizer's internal background thread, so restarting a `-fsanitize=thread` target under `--multithreaded-fork` no longer hangs waiting for a semaphore post that thread will never send. + +**Architecture:** Two small, additive changes to `src/lib/dmtcp-callback.c`, backed by a new tiny helper module `src/lib/tsan_support.c`. `tsan_support.c` provides `thread_blocks_signal()`, a `/proc`-based probe that identifies threads with a specific signal blocked (how libtsan's background thread can be recognized, since it blocks all signals at creation). `dmtcp-callback.c` gains a read-only `get_tid_from_pthread_descriptor()` helper (a non-mutating sibling of the existing `patchThreadDescriptor()`) and uses both to replace `template_thread()`'s blanket `thread_count -= 2` with explicit per-tid classification (self / checkpoint thread / TSan-internal / countable). + +**Tech Stack:** C11, CMake, glibc/Linux `/proc` filesystem, POSIX threads and signals. No new external dependencies. + +## Global Constraints + +- Build uses `-Wall -Werror` (`CMakeLists.txt:81`) — every new file must compile warning-free. +- C11 atomics are required to compile libmcmini (`include/mcmini/spy/checkpointing/record.h:7-8`) — not directly relevant to this plan's new code, but any new header included from that translation unit must not break this. +- `pthreadDescriptorTidOffset()` (and thus the new `get_tid_from_pthread_descriptor()`) only has defined values for `__x86_64__`, `__aarch64__`, and `__riscv` (`src/lib/dmtcp-callback.c:119-129`). This plan's standalone test only covers `__x86_64__` (the architecture of this dev machine); it is not a substitute for testing the other architectures. +- Design reference: `docs/superpowers/specs/2026-07-04-tsan-port-phase1-r1-design.md`. Follow it for rationale; this plan follows it for file placement and interfaces. +- Baseline: branch `tsan-multithreaded-fork-port`, commit `4d78676` (spec) on top of `b38da82` (Phase 0 cleanup). + +--- + +### Task 1: `thread_blocks_signal()` in a new `tsan_support` module + +**Files:** +- Create: `include/mcmini/spy/checkpointing/tsan_support.h` +- Create: `src/lib/tsan_support.c` +- Modify: `CMakeLists.txt:68-77` (add the new source file to `LIBMCMINI_C_SRC`) +- Test: `test/tsan_support/test_thread_blocks_signal.c` (standalone host-side unit test — compiled and run directly with `gcc`, not through CMake or as an mcmini model-checking target; this project has no C unit-test harness yet, per `CLAUDE.md`) + +**Interfaces:** +- Produces: `int thread_blocks_signal(pid_t tid, int signo);` — declared in `tsan_support.h`, implemented in `tsan_support.c`. Returns nonzero if thread `tid` currently has signal `signo` blocked (per its `/proc/self/task//status` `SigBlk` mask), zero otherwise (including if `tid` no longer exists). Consumed by Task 2. + +- [ ] **Step 1: Write the header and the failing test** + +Create `include/mcmini/spy/checkpointing/tsan_support.h`: + +```c +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +/** + * Returns nonzero if thread `tid` currently has signal `signo` blocked, per + * the "SigBlk" field of /proc/self/task//status. Returns 0 if `tid` + * cannot be inspected (e.g. it has already exited). + */ +int thread_blocks_signal(pid_t tid, int signo); + +#ifdef __cplusplus +} +#endif +``` + +Create `test/tsan_support/test_thread_blocks_signal.c`: + +```c +// Standalone host-side unit test for thread_blocks_signal(). Not an mcmini +// model-checking target: compile and run directly (see the command in the +// implementation plan / commit message), not through CMake. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mcmini/spy/checkpointing/tsan_support.h" + +static sem_t ready; +static pid_t worker_tid; + +static void *worker(void *arg) { + (void)arg; + worker_tid = (pid_t)syscall(SYS_gettid); + + sigset_t set; + sigemptyset(&set); + sigaddset(&set, SIGUSR1); + pthread_sigmask(SIG_BLOCK, &set, NULL); + + sem_post(&ready); + + for (;;) { + pause(); // cancellation point; SIGUSR1 stays blocked the whole time + } + return NULL; +} + +int main(void) { + int rc = sem_init(&ready, 0, 0); + assert(rc == 0); + + pthread_t t; + rc = pthread_create(&t, NULL, worker, NULL); + assert(rc == 0); + + rc = sem_wait(&ready); + assert(rc == 0); + + pid_t self_tid = (pid_t)syscall(SYS_gettid); + + assert(thread_blocks_signal(worker_tid, SIGUSR1) == 1); + assert(thread_blocks_signal(self_tid, SIGUSR1) == 0); + assert(thread_blocks_signal(999999 /* bogus tid, should not exist */, SIGUSR1) == 0); + + rc = pthread_cancel(t); + assert(rc == 0); + rc = pthread_join(t, NULL); + assert(rc == 0); + + printf("PASS\n"); + return 0; +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: +```bash +gcc -Wall -Werror -Iinclude src/lib/tsan_support.c test/tsan_support/test_thread_blocks_signal.c -o /tmp/test_thread_blocks_signal -lpthread +``` + +Expected: FAIL — `src/lib/tsan_support.c: No such file or directory` (it doesn't exist yet). + +- [ ] **Step 3: Implement `thread_blocks_signal()`** + +Create `src/lib/tsan_support.c`: + +```c +#include "mcmini/spy/checkpointing/tsan_support.h" + +#include + +int thread_blocks_signal(pid_t tid, int signo) { + char path[64]; + snprintf(path, sizeof(path), "/proc/self/task/%d/status", (int)tid); + FILE *f = fopen(path, "r"); + if (f == NULL) { + return 0; + } + + char line[256]; + unsigned long long sigblk = 0; + int found = 0; + while (fgets(line, sizeof(line), f)) { + if (sscanf(line, "SigBlk: %llx", &sigblk) == 1) { + found = 1; + break; + } + } + fclose(f); + + if (!found) { + return 0; + } + return (int)((sigblk >> (signo - 1)) & 1ULL); +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: +```bash +gcc -Wall -Werror -Iinclude src/lib/tsan_support.c test/tsan_support/test_thread_blocks_signal.c -o /tmp/test_thread_blocks_signal -lpthread +/tmp/test_thread_blocks_signal +echo "exit: $?" +``` + +Expected: compiles with no warnings, prints `PASS`, `exit: 0`. + +- [ ] **Step 5: Wire the new file into the CMake build** + +Modify `CMakeLists.txt`, in the `LIBMCMINI_C_SRC` list (currently lines 68-77): + +```cmake + src/lib/dmtcp-callback.c + src/lib/entry.c + src/lib/interception.c + src/lib/log.c + src/lib/main.c + src/lib/record.c + src/lib/sem-wrappers.c + src/lib/template/loop.c + src/lib/template/sig.c + src/lib/tsan_support.c + src/lib/wrappers.c +``` + +(single line added: ` src/lib/tsan_support.c`, alphabetically before `src/lib/wrappers.c`) + +Run: +```bash +cmake --build build --target libmcmini +``` + +Expected: `[100%] Built target libmcmini` with no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add include/mcmini/spy/checkpointing/tsan_support.h src/lib/tsan_support.c CMakeLists.txt test/tsan_support/test_thread_blocks_signal.c +git commit -m "Add thread_blocks_signal() TSan-internal-thread probe" +``` + +--- + +### Task 2: Per-tid classification in `template_thread()`'s restart barrier + +**Files:** +- Modify: `src/lib/dmtcp-callback.c:1-22` (add `#include` for the new header) +- Modify: `src/lib/dmtcp-callback.c:131-138` (add `get_tid_from_pthread_descriptor()` next to `patchThreadDescriptor()`) +- Modify: `src/lib/dmtcp-callback.c:368-384` (replace the blanket `thread_count -= 2` in `template_thread()` with per-tid classification) +- Test: `test/tsan_support/test_tid_from_descriptor_offset.c` (standalone host-side unit test, same style as Task 1) + +**Interfaces:** +- Consumes: `int thread_blocks_signal(pid_t tid, int signo)` from Task 1 (`tsan_support.h`). +- Produces: `static inline pid_t get_tid_from_pthread_descriptor(pthread_t pthread_descriptor)` — file-local to `dmtcp-callback.c` (not exported; matches the file-local style of the neighboring `patchThreadDescriptor()`/`pthreadDescriptorTidOffset()`). No other task consumes it directly. + +- [ ] **Step 1: Write a standalone test proving the read-only offset technique** + +`get_tid_from_pthread_descriptor()` reads the same fixed offset into a `pthread_t` that the existing (mutating) `patchThreadDescriptor()` already uses and which is already self-verified at every restart (`saveThreadStateBeforeFork()`, `src/lib/dmtcp-callback.c:140-150`, aborts via `libc_abort()` on mismatch). The new risk this phase introduces is reading *another* thread's descriptor (the checkpoint thread's) instead of only ever reading `pthread_self()`. This test proves that part of the technique in isolation, independent of the rest of `dmtcp-callback.c`'s heavy dependencies (confirmed via `nm -u` on the compiled object file — over two dozen unresolved libmcmini-internal symbols — so linking the real file into a lightweight test binary is impractical; see the design spec for context). + +Create `test/tsan_support/test_tid_from_descriptor_offset.c`: + +```c +// Standalone host-side unit test. Not an mcmini model-checking target. +// +// Independently re-verifies the x86_64 pthread_t -> tid offset that +// src/lib/dmtcp-callback.c's pthreadDescriptorTidOffset() uses, specifically +// for READING (not patching) another thread's descriptor -- the new use case +// get_tid_from_pthread_descriptor() introduces. The existing +// patchThreadDescriptor() already self-verifies the same offset for +// `pthread_self()` on every restart (see saveThreadStateBeforeFork()); this +// test covers the "another thread's descriptor" case that path never +// exercises. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +#ifndef __x86_64__ +#error "This standalone test only covers __x86_64__; see dmtcp-callback.c's pthreadDescriptorTidOffset() for other architectures." +#endif + +static pid_t tid_from_descriptor(pthread_t descriptor) { + const int offset = 720; // matches pthreadDescriptorTidOffset() for __x86_64__ + return *(pid_t *)((char *)descriptor + offset); +} + +static sem_t ready; +static pthread_t worker_self; +static pid_t worker_tid; + +static void *worker(void *arg) { + (void)arg; + worker_self = pthread_self(); + worker_tid = (pid_t)syscall(SYS_gettid); + sem_post(&ready); + for (;;) { + pause(); + } + return NULL; +} + +int main(void) { + int rc = sem_init(&ready, 0, 0); + assert(rc == 0); + + pthread_t t; + rc = pthread_create(&t, NULL, worker, NULL); + assert(rc == 0); + + rc = sem_wait(&ready); + assert(rc == 0); + + // Read another thread's tid from its descriptor without mutating it. + assert(tid_from_descriptor(worker_self) == worker_tid); + // Reading twice must be idempotent (unlike patchThreadDescriptor()). + assert(tid_from_descriptor(worker_self) == worker_tid); + + rc = pthread_cancel(t); + assert(rc == 0); + rc = pthread_join(t, NULL); + assert(rc == 0); + + printf("PASS\n"); + return 0; +} +``` + +- [ ] **Step 2: Run the test to verify it passes** + +Run: +```bash +gcc -Wall -Werror test/tsan_support/test_tid_from_descriptor_offset.c -o /tmp/test_tid_from_descriptor_offset -lpthread +/tmp/test_tid_from_descriptor_offset +echo "exit: $?" +``` + +Expected: compiles with no warnings, prints `PASS`, `exit: 0`. (This test is expected to pass immediately — it documents and locks in an already-proven assumption from elsewhere in the codebase, rather than driving new production code through a red/green cycle.) + +- [ ] **Step 3: Include the new header in `dmtcp-callback.c`** + +In `src/lib/dmtcp-callback.c`, current lines 21-22: + +```c +#include "dmtcp.h" +#include "mcmini/mcmini.h" +``` + +Change to: + +```c +#include "dmtcp.h" +#include "mcmini/mcmini.h" +#include "mcmini/spy/checkpointing/tsan_support.h" +``` + +- [ ] **Step 4: Add `get_tid_from_pthread_descriptor()`** + +In `src/lib/dmtcp-callback.c`, current lines 131-138: + +```c +static inline pid_t patchThreadDescriptor(pthread_t pthreadSelf) { + int offset = pthreadDescriptorTidOffset(); + pid_t oldtid = *(pid_t *)((char *)pthreadSelf + offset); + // Since glibc.2.25, tid, but not pid, is stored in pthread_t. + // gettid() supported only in glibc-2.30; So, we use syscall(). + *(pid_t *)((char *)pthreadSelf + offset) = syscall(SYS_gettid); + return oldtid; +} +``` + +Add immediately after it: + +```c +// Read-only sibling of patchThreadDescriptor(): returns the tid recorded in +// `pthread_descriptor` without mutating it. Used to look up the checkpoint +// thread's tid (a *different* thread's descriptor) from the template thread; +// patchThreadDescriptor() is only ever called by a thread on its own +// descriptor. +static inline pid_t get_tid_from_pthread_descriptor(pthread_t pthread_descriptor) { + int offset = pthreadDescriptorTidOffset(); + return *(pid_t *)((char *)pthread_descriptor + offset); +} +``` + +- [ ] **Step 5: Replace the blanket `-2` with per-tid classification in `template_thread()`** + +In `src/lib/dmtcp-callback.c`, current lines 368-384: + +```c + int thread_count = 0; + struct dirent *entry; + DIR *dp = opendir("/proc/self/task"); + if (dp == NULL) { + perror("opendir"); + mc_exit(EXIT_FAILURE); + } + + while ((entry = readdir(dp))) + if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) + thread_count++; + + // We don't want to count the template thread nor + // the checkpoint thread, but these will appear in + // `/proc/self/tasks` + thread_count -= 2; + closedir(dp); +``` + +Replace with: + +```c + int thread_count = 0; + struct dirent *entry; + DIR *dp = opendir("/proc/self/task"); + if (dp == NULL) { + perror("opendir"); + mc_exit(EXIT_FAILURE); + } + + const pid_t self_tid = syscall(SYS_gettid); + const pid_t ckpt_tid = get_tid_from_pthread_descriptor(ckpt_pthread_descriptor); + + // Self-check: get_tid_from_pthread_descriptor() reads the same offset that + // patchThreadDescriptor() already relies on and that saveThreadStateBeforeFork() + // already self-verifies for `pthread_self()` on every restart. Confirm the + // read-only variant agrees for the template thread's own descriptor before + // trusting it to read the checkpoint thread's descriptor above. + if (get_tid_from_pthread_descriptor(pthread_self()) != self_tid) { + fprintf(stderr, + "PID %d: template_thread(): get_tid_from_pthread_descriptor: " + "bad offset:\n Run: DMTCP:util/check-pthread-tid-offset.c\n", + getpid()); + libc_abort(); + } + + // We don't want to count the template thread itself, the checkpoint + // thread, or TSan-internal threads (e.g. libtsan's background thread, + // which blocks all signals at creation and never calls into libmcmini's + // wrappers, so it will never post to `dmtcp_restart_sem` below). + while ((entry = readdir(dp))) { + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { + continue; + } + pid_t tid = (pid_t)atoi(entry->d_name); + if (tid == self_tid || tid == ckpt_tid) { + continue; + } + if (thread_blocks_signal(tid, SIG_MULTITHREADED_FORK)) { + log_debug("Excluding TSan-internal thread %d from the restart barrier\n", tid); + continue; + } + thread_count++; + } + closedir(dp); +``` + +- [ ] **Step 6: Rebuild and verify** + +Run: +```bash +cmake --build build --target libmcmini +``` + +Expected: `[100%] Built target libmcmini` with no warnings (build uses `-Wall -Werror`). + +- [ ] **Step 7: Commit** + +```bash +git add src/lib/dmtcp-callback.c test/tsan_support/test_tid_from_descriptor_offset.c +git commit -m "Exclude TSan-internal threads from template_thread()'s restart barrier" +``` + +--- + +### Task 3: End-to-end verification against a real TSan target (environment-gated) + +**Precondition:** this task requires the DMTCP toolchain (`dmtcp_launch`, `dmtcp_restart`) installed and on `PATH`, and the project built with `MCMINI_WITH_DMTCP=ON`. Neither is currently true in this dev environment (`which dmtcp_launch` returns nothing; `CMakeLists.txt:20` has `set(MCMINI_WITH_DMTCP OFF)`). If this environment is unavailable when this task is reached, stop after Task 2, report that Tasks 1-2 are complete and committed, and hand this task off to whoever has a DMTCP-enabled environment — do not mark this task done without actually running it. + +**Files:** none (this task runs commands and observes output; it does not change source). + +- [ ] **Step 1: Reconfigure and rebuild with DMTCP enabled** + +```bash +cmake -S . -B build -DMCMINI_WITH_DMTCP=ON +cmake --build build --target libmcmini +``` + +Expected: build succeeds and produces `build/libmcmini.so`. + +- [ ] **Step 2: Build a TSan-instrumented example target** + +```bash +cd build/src/examples +gcc -fsanitize=thread -g -pthread -o producer-consumer-tsan ../../../src/examples/producer-consumer.c +cd - +``` + +Expected: `producer-consumer-tsan` binary produced with no compile errors. + +- [ ] **Step 3: Record a checkpoint** + +From the directory containing `libmcmini.so` (per the `mcmini` cwd-relative plugin-path gotcha documented in `CLAUDE.md`): + +```bash +cd build +TSAN_OPTIONS="handle_segv=0 die_after_fork=0" \ + dmtcp_launch --disable-alloc-plugin -i 5 --with-plugin "$PWD/libmcmini.so" \ + ./src/examples/producer-consumer-tsan +``` + +Let it run for at least one checkpoint interval (5s), then stop it (Ctrl-C) once a `ckpt_*.dmtcp` file appears in the current directory. + +Expected: a `ckpt_producer-consumer-tsan_*.dmtcp` file is created. + +- [ ] **Step 4: Restart under `mcmini` with `--multithreaded-fork`** + +```bash +setarch -R ./mcmini --from-checkpoint ckpt_producer-consumer-tsan_*.dmtcp --multithreaded-fork ./src/examples/producer-consumer-tsan +``` + +Expected (this phase's pass criterion, per `PLAN.txt` Phase 1 and the design spec's Testing section): the `template_thread()` debug log (enable with `MCMINI_LOG_LEVEL=debug` if not already visible) shows a thread count matching the real number of application threads, any `Excluding TSan-internal thread from the restart barrier` lines for libtsan's background thread, and `The threads are now in a consistent state` — **not** an indefinite hang. Reaching (or failing to reach) the model-checker handshake afterward is Phase 2's concern (R2/R3/R4), not this task's pass/fail criterion. + +- [ ] **Step 5: Record the observed outcome** + +No commit for this task (no source changes). If the pass criterion in Step 4 is met, note it in the PR/handoff description; if not, capture the log output and hand off to Phase 2 planning rather than attempting further fixes here (out of scope for R1). diff --git a/docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md b/docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md new file mode 100644 index 00000000..12b06016 --- /dev/null +++ b/docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md @@ -0,0 +1,475 @@ +# TSan Port Phase 2 (R2+R3+R4): fork hooks, __clone, fiber switching — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the two remaining gaps in libmcmini's TSan `multithreaded_fork` port — R3 (`__clone` instead of the public `clone()`) and the R4 remainder (a fresh TSan fiber for the *forking* thread, not just recreated threads) — and prove the fix with a standalone test harness that reproduces the real ThreadSanitizer failure before the fix and passes cleanly after. + +**Architecture:** Two small, mechanical edits to `src/lib/dmtcp-callback.c` (R2 and R4's recreated-thread half are already committed from Phase 0/1). A new standalone test harness, `test/tsan_support/test_fastpath_fork_clone_fiber.c`, mirrors `dmtcp-callback.c`'s actual getcontext-direct-call resumption mechanism (not the vendor package's signal-based one) and is built/run twice: once in a "buggy" configuration (public `clone()`, no forking-thread fiber switch) to capture the real TSan `CHECK failed` this phase fixes, then again after applying the identical fix, to confirm it passes. + +**Tech Stack:** C11, CMake, ThreadSanitizer (`-fsanitize=thread`), glibc `__clone`/fiber APIs, POSIX threads/semaphores/ucontext. + +## Global Constraints + +- Build uses `-Wall -Werror` (`CMakeLists.txt:81`) — the production build must stay warning-free. +- The TLS/pthread-descriptor offset helpers (`pthreadDescriptorTidOffset()` etc.) are only defined for `__x86_64__` in the standalone harness (matching Phase 1's `test_tid_from_descriptor_offset.c` precedent); the harness guards this with `#error` on other architectures. +- **ThreadSanitizer requires ASLR disabled in this sandbox.** A bare `-fsanitize=thread` binary fails immediately with `FATAL: ThreadSanitizer: unexpected memory mapping`; running it via `setarch -R ./binary` fixes this (confirmed during design). Every TSan run in this plan uses `setarch -R`. +- Every TSan run in this plan sets `TSAN_OPTIONS="handle_segv=0 die_after_fork=0"` (matching the vendor package's own Makefile and PLAN.txt section 7). +- Design reference: `docs/superpowers/specs/2026-07-04-tsan-port-phase2-r2-r3-r4-design.md`. +- Baseline: branch `tsan-record-thread-fix`, commit `3664e7a` (Phase 2 design spec, on top of the Phase 0+1 work already merged). +- R2 (fork-hook bracketing of `_Fork()`) and R4's recreated-thread fiber switch are **already committed** (`src/lib/dmtcp-callback.c:236-242` and `:304-315` respectively) — do not modify them; this plan only adds the two remaining pieces. + +--- + +### Task 1: Standalone test harness proving R3 + R4-remainder under real TSan + +**Files:** +- Create: `test/tsan_support/test_fastpath_fork_clone_fiber.c` + +**Interfaces:** None consumed from other tasks. Produces no interface other tasks depend on — this is a standalone, self-contained host-side test (compiled/run directly with `gcc`, not via CMake, per this project's lack of a C unit-test harness — see `CLAUDE.md`). + +This task is unusual in that the "RED" state is achieved with a compile-time flag (`-DMTF_BUGGY`) rather than a missing function — the harness is fully self-contained from Step 1, and Step 2 proves it fails in the *buggy* configuration before Step 4 proves the *default* (fixed) configuration passes. + +- [ ] **Step 1: Write the harness (default = fixed; `-DMTF_BUGGY` = buggy)** + +Create `test/tsan_support/test_fastpath_fork_clone_fiber.c`: + +```c +// Standalone host-side test harness. Not an mcmini model-checking target. +// +// Mirrors dmtcp-callback.c's ACTUAL fast-path resumption mechanism (each +// thread calls getcontext() directly, as a plain function call -- no signal +// handler, unlike the multithreaded-fork-tsan-2.0 standalone package) to +// exercise R2 (TSan fork hooks around _Fork()), R3 (__clone instead of the +// public clone()), and R4 (fresh TSan fiber for the forking thread AND each +// recreated thread) under real ThreadSanitizer. +// +// BUGGY MODE (for RED evidence): compile with -DMTF_BUGGY to use the public +// clone() (no R3) and skip the forking-thread fiber switch (no R4 remainder), +// reproducing the failure these two fixes exist to prevent. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef __x86_64__ +#include +#include +#else +#error "This standalone harness only covers __x86_64__; see dmtcp-callback.c for other architectures." +#endif + +#ifndef NUM_WORKERS +#define NUM_WORKERS 3 +#endif +#define ITERS 100000 + +// ---- TSan externs, copied verbatim from src/lib/dmtcp-callback.c ---- +extern void *__tsan_create_fiber(unsigned flags) __attribute__((weak)); +extern void __tsan_switch_to_fiber(void *fiber, unsigned flags) + __attribute__((weak)); +extern void __sanitizer_syscall_pre_impl_fork(void) __attribute__((weak)); +extern void __sanitizer_syscall_post_impl_fork(long res) __attribute__((weak)); + +// libc's internal clone (NOT intercepted by libtsan, unlike public clone()). +extern int __clone(int (*fn)(void *), void *child_stack, int flags, void *arg, + ... /* pid_t *ptid, void *newtls, pid_t *ctid */); + +// ---- struct threadinfo + TLS/descriptor helpers, copied verbatim from +// src/lib/dmtcp-callback.c (x86_64 branch only) for fidelity to production, +// including the "syscall(SYS_arch_prctl, 2, ARCH_SET_FS, ...)" call already +// proven correct by the multithreaded-fork-tsan-2.0 standalone package. ---- +struct threadinfo { + ucontext_t context; + unsigned long fs; + unsigned long gs; + pthread_t pthread_descriptor; +}; +static struct threadinfo threadInfos[NUM_WORKERS]; +static atomic_int threadIdx = 0; + +static void getTLSPointer(struct threadinfo *ti) { + assert(syscall(SYS_arch_prctl, ARCH_GET_FS, &ti->fs) == 0); + assert(syscall(SYS_arch_prctl, ARCH_GET_GS, &ti->gs) == 0); +} +static void setTLSPointer(struct threadinfo *ti) { + assert(syscall(SYS_arch_prctl, 2, ARCH_SET_FS, ti->fs) != 0); + assert(syscall(SYS_arch_prctl, 2, ARCH_SET_GS, ti->gs) != 0); +} +static int pthreadDescriptorTidOffset(void) { return 720; } +static pid_t patchThreadDescriptor(pthread_t pthreadSelf) { + int offset = pthreadDescriptorTidOffset(); + pid_t oldtid = *(pid_t *)((char *)pthreadSelf + offset); + *(pid_t *)((char *)pthreadSelf + offset) = syscall(SYS_gettid); + return oldtid; +} + +// ---- Harness state ---- +static pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER; +static long shared_counter = 0; +static sem_t sem_checkin, sem_release, sem_done, sem_park; + +static void sem_wait_retry(sem_t *s) { + while (sem_wait(s) != 0) /* EINTR */; +} + +// ---- child_setcontext_fast(), copied verbatim in structure from +// src/lib/dmtcp-callback.c ---- +static int child_setcontext_fast(void *arg) { + struct threadinfo *ti = arg; + setTLSPointer(ti); + patchThreadDescriptor(ti->pthread_descriptor); + setcontext(&ti->context); // never returns + return 0; +} + +static void restart_child_threads_fast(void) { + int maxThreadIdx = atomic_load(&threadIdx); + for (int i = 0; i < maxThreadIdx; i++) { + int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM + | CLONE_SIGHAND | CLONE_THREAD + | CLONE_SETTLS | CLONE_PARENT_SETTID + | CLONE_CHILD_CLEARTID); + void *stack = malloc(0x10000) + 0x10000 - 128; // 64 KB, intentionally leaked + int offset = pthreadDescriptorTidOffset(); + pid_t *ctid = (pid_t *)((char *)threadInfos[i].pthread_descriptor + offset); + pid_t *ptid = ctid; +#ifdef MTF_BUGGY + // R3 NOT applied: public clone() is intercepted by libtsan and treated + // as a fork, corrupting its thread-slot state for this CLONE_THREAD clone. + clone(child_setcontext_fast, stack, clone_flags, + (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); +#else + // R3: libc's raw __clone, not intercepted by libtsan. + __clone(child_setcontext_fast, stack, clone_flags, + (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); +#endif + } +} + +// ---- fast_multithreaded_fork(), copied in structure from +// src/lib/dmtcp-callback.c ---- +static pid_t fast_multithreaded_fork(void) { + pid_t _Fork(); + if (__sanitizer_syscall_pre_impl_fork != NULL) { + __sanitizer_syscall_pre_impl_fork(); + } + int childpid = _Fork(); + if (__sanitizer_syscall_post_impl_fork != NULL) { + __sanitizer_syscall_post_impl_fork(childpid); + } + if (childpid == 0) { // child process +#ifndef MTF_BUGGY + // R4 remainder: the forking thread keeps its inherited (fork-copied) TSan + // ThreadState, whose shadow call stack starts at the parent's fork-time + // depth and can overflow as this thread keeps running. Fresh fiber too. + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } +#endif + restart_child_threads_fast(); + } + return childpid; +} + +// ---- worker thread: mirrors thread_handle_after_dmtcp_restart()'s +// getcontext-direct-call mechanism (no signal handler). ---- +static void *worker(void *arg) { + (void)arg; + pid_t orig_pid = getpid(); + int idx = atomic_fetch_add(&threadIdx, 1); + struct threadinfo *ti = &threadInfos[idx]; + memset(ti, 0, sizeof(*ti)); + ti->pthread_descriptor = pthread_self(); + getTLSPointer(ti); + + int rc = getcontext(&ti->context); + assert(rc == 0); + + if (getpid() == orig_pid) { + // Still in the original process (pre-fork): check in, then block until + // released (mirrors production's cond_wait parking; a plain semaphore is + // sufficient here since R1's mode-machinery is out of scope for R2/R3/R4). + sem_post(&sem_checkin); + sem_wait_retry(&sem_release); + } else { + // Resumed via __clone()+setcontext in the forked child: give this + // TSan-invisible OS thread a valid ThreadState before any instrumented + // call below. (Already-proven R4 half; unchanged by MTF_BUGGY.) + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } + } + + // TSan-intercepted work: locked shared write, exercised both by the + // original threads (parent) and the recreated threads (child). + for (int i = 0; i < ITERS; i++) { + pthread_mutex_lock(&mtx); + shared_counter++; + pthread_mutex_unlock(&mtx); + } + sem_post(&sem_done); + sem_wait_retry(&sem_park); // park forever; R5 join/exit shims out of scope + return NULL; +} + +int main(void) { + sem_init(&sem_checkin, 0, 0); + sem_init(&sem_release, 0, 0); + sem_init(&sem_done, 0, 0); + sem_init(&sem_park, 0, 0); + + pthread_t th[NUM_WORKERS]; + for (int i = 0; i < NUM_WORKERS; i++) { + pthread_create(&th[i], NULL, worker, NULL); + } + for (int i = 0; i < NUM_WORKERS; i++) { + sem_wait(&sem_checkin); + } + + fprintf(stderr, "[main pid=%d] all workers checked in; forking...\n", getpid()); + pid_t pid = fast_multithreaded_fork(); + const char *who = (pid == 0) ? "CHILD" : "PARENT"; + fprintf(stderr, "[%s pid=%d] returned from fast_multithreaded_fork\n", who, getpid()); + + if (pid > 0) { + // PARENT: release the still-parked original workers. + for (int i = 0; i < NUM_WORKERS; i++) { + sem_post(&sem_release); + } + } + // CHILD: recreated threads proceed on their own (see worker()'s else branch). + + for (int i = 0; i < NUM_WORKERS; i++) { + sem_wait(&sem_done); + } + fprintf(stderr, "[%s pid=%d] shared_counter=%ld (expected %d)\n", + who, getpid(), shared_counter, NUM_WORKERS * ITERS); + + if (pid > 0) { + int status; + waitpid(pid, &status, 0); + fprintf(stderr, "[PARENT] child: exited=%d code=%d signaled=%d\n", + WIFEXITED(status), WEXITSTATUS(status), WIFSIGNALED(status)); + } + fprintf(stderr, "[%s pid=%d] done, _exit(0)\n", who, getpid()); + _exit(0); +} +``` + +- [ ] **Step 2: Build and run the BUGGY configuration — verify it fails (RED)** + +Run: +```bash +gcc -DMTF_BUGGY -fsanitize=thread -g -O1 -Wall -pthread -o /tmp/mtf_fastpath_buggy test/tsan_support/test_fastpath_fork_clone_fiber.c -fsanitize=thread -pthread +TSAN_OPTIONS="handle_segv=0 die_after_fork=0" setarch -R /tmp/mtf_fastpath_buggy +echo "exit: $?" +``` + +Expected: compiles clean, then FAILS with output containing (one line per recreated thread): +``` +ThreadSanitizer: CHECK failed: tsan_rtl.cpp:253 "((!thr->slot)) != (0)" (0x0, 0x0) (tid=...) +``` +and `[PARENT] child: exited=1 code=66 signaled=0` (the child crashes; only the parent's `shared_counter=300000 (expected 300000)` line appears — the child never reaches its own). This is `ForkChildAfter` in libtsan's fork pipeline choking on the intercepted public `clone()` — exactly the R3 failure mode this phase fixes. `exit: 0` refers to the harness process itself (it doesn't propagate the child's crash as its own exit code; the crash is visible in the printed child status and the CHECK-failed lines). + +- [ ] **Step 3: No code change needed — Step 1 already wrote the fixed (default) path** + +The `#ifdef MTF_BUGGY` / `#else` branches in Step 1's file already contain both configurations. There is no separate "fix" edit to make in the harness itself; Step 4 simply compiles without `-DMTF_BUGGY`. + +- [ ] **Step 4: Build and run the default (fixed) configuration — verify it passes (GREEN)** + +Run: +```bash +gcc -fsanitize=thread -g -O1 -Wall -pthread -o /tmp/mtf_fastpath_fixed test/tsan_support/test_fastpath_fork_clone_fiber.c -fsanitize=thread -pthread +TSAN_OPTIONS="handle_segv=0 die_after_fork=0" setarch -R /tmp/mtf_fastpath_fixed +echo "exit: $?" +``` + +Expected: compiles clean, prints (pids will differ): +``` +[main pid=...] all workers checked in; forking... +[PARENT pid=...] returned from fast_multithreaded_fork +[CHILD pid=...] returned from fast_multithreaded_fork +[CHILD pid=...] shared_counter=300000 (expected 300000) +[CHILD pid=...] done, _exit(0) +[PARENT pid=...] shared_counter=300000 (expected 300000) +[PARENT] child: exited=1 code=0 signaled=0 +[PARENT pid=...] done, _exit(0) +``` +No `CHECK failed`, no SEGV. `exit: 0`. Run it 3 times in a row to confirm it isn't flaky: +```bash +for i in 1 2 3; do TSAN_OPTIONS="handle_segv=0 die_after_fork=0" setarch -R /tmp/mtf_fastpath_fixed 2>&1 | grep -E "shared_counter|CHECK failed|exited"; done +``` +Expected: all 3 runs show both `shared_counter=300000 (expected 300000)` lines and `exited=1 code=0 signaled=0`, no `CHECK failed` in any run. + +- [ ] **Step 5: Commit** + +```bash +git add test/tsan_support/test_fastpath_fork_clone_fiber.c +git commit -m "Add standalone harness proving R3 (__clone) + R4-remainder (forker fiber) under TSan" +``` + +--- + +### Task 2: Apply R3 + R4-remainder to production (`src/lib/dmtcp-callback.c`) + +**Files:** +- Modify: `src/lib/dmtcp-callback.c:37-51` (add `__clone` extern next to the existing weak TSan externs) +- Modify: `src/lib/dmtcp-callback.c:186-208` (`restart_child_threads_fast()`: swap `clone()` for `__clone()`) +- Modify: `src/lib/dmtcp-callback.c:273-275` (`fast_multithreaded_fork()`'s child branch: add the forking-thread fiber switch) + +**Interfaces:** None — this task applies, verbatim, the exact fix already proven in Task 1's harness to the real production functions of the same name and structure. No new functions are introduced. + +- [ ] **Step 1: Add the `__clone` extern declaration** + +In `src/lib/dmtcp-callback.c`, current lines 50-51: + +```c +extern void __sanitizer_syscall_pre_impl_fork(void) __attribute__((weak)); +extern void __sanitizer_syscall_post_impl_fork(long res) __attribute__((weak)); +``` + +Add immediately after: + +```c + +// libc's internal clone (NOT intercepted by libtsan, unlike the public +// clone()). libtsan's clone() interceptor treats every call as a fork +// (ForkChildAfter), which corrupts its thread-slot state for the +// CLONE_THREAD clone restart_child_threads_fast() performs below. __clone is +// not intercepted, so it performs the raw thread creation; the recreated +// thread's own fiber switch (see thread_handle_after_dmtcp_restart()) then +// gives it a valid TSan ThreadState. +extern int __clone(int (*fn)(void *), void *child_stack, int flags, + void *arg, ... /* pid_t *ptid, void *newtls, pid_t *ctid */); +``` + +- [ ] **Step 2: Swap `clone()` for `__clone()` in `restart_child_threads_fast()`** + +In `src/lib/dmtcp-callback.c`, current lines 202-206: + +```c + // For more insight, read 'man set_tid_address'. + clone(child_setcontext_fast, + stack, + clone_flags, + (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); +``` + +Replace with: + +```c + // For more insight, read 'man set_tid_address'. + // R3: use libc's raw __clone, not the public clone() (see the __clone + // extern declaration above for why). + __clone(child_setcontext_fast, + stack, + clone_flags, + (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); +``` + +- [ ] **Step 3: Add the forking-thread fiber switch in `fast_multithreaded_fork()`** + +In `src/lib/dmtcp-callback.c`, current lines 273-275: + +```c + if (childpid == 0) { // child process + restart_child_threads_fast(); + } +``` + +Replace with: + +```c + if (childpid == 0) { // child process + // R4 (remainder): the forking thread keeps its inherited (fork-copied) + // TSan ThreadState, whose shadow call stack starts at the parent's + // fork-time depth and can overflow as this thread keeps running. Switch + // it onto a fresh fiber too, mirroring the recreated-thread fiber switch + // in thread_handle_after_dmtcp_restart(). Weak symbol: a no-op for + // non-TSan targets. + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } + restart_child_threads_fast(); + } +``` + +- [ ] **Step 4: Rebuild and verify** + +Run: +```bash +cmake --build build --target libmcmini +``` + +Expected: `[100%] Built target libmcmini` with no warnings (build uses `-Wall -Werror`). + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/dmtcp-callback.c +git commit -m "Apply R3 (__clone) and R4 remainder (forker fiber) to fast_multithreaded_fork" +``` + +--- + +### Task 3: End-to-end verification against a real TSan target under DMTCP (environment-gated) + +**Precondition:** this task requires the DMTCP toolchain (`dmtcp_launch`, `dmtcp_restart`) installed and on `PATH`, and the project built with `MCMINI_WITH_DMTCP=ON`. As of this plan's writing, this environment has neither (same gap Phase 1's Task 3 hit). If unavailable when this task is reached, stop after Task 2, report Tasks 1-2 complete and committed, and hand this task off — do not mark it done without actually running it. + +**Files:** none (this task runs commands and observes output; it does not change source). + +- [ ] **Step 1: Reconfigure and rebuild with DMTCP enabled** + +```bash +cmake -S . -B build -DMCMINI_WITH_DMTCP=ON +cmake --build build --target libmcmini +``` + +Expected: build succeeds and produces `build/libmcmini.so`. + +- [ ] **Step 2: Build a TSan-instrumented example target** + +```bash +cd build/src/examples +gcc -fsanitize=thread -g -pthread -o producer-consumer-tsan ../../../src/examples/producer-consumer.c +cd - +``` + +Expected: `producer-consumer-tsan` binary produced with no compile errors. + +- [ ] **Step 3: Record a checkpoint** + +From the directory containing `libmcmini.so`: + +```bash +cd build +TSAN_OPTIONS="handle_segv=0 die_after_fork=0" \ + dmtcp_launch --disable-alloc-plugin -i 5 --with-plugin "$PWD/libmcmini.so" \ + ./src/examples/producer-consumer-tsan +``` + +Let it run for at least one checkpoint interval (5s), then stop it (Ctrl-C) once a `ckpt_*.dmtcp` file appears in the current directory. + +Expected: a `ckpt_producer-consumer-tsan_*.dmtcp` file is created. + +- [ ] **Step 4: Restart under `mcmini` with `--multithreaded-fork`** + +```bash +setarch -R ./mcmini --from-checkpoint ckpt_producer-consumer-tsan_*.dmtcp --multithreaded-fork ./src/examples/producer-consumer-tsan +``` + +Expected (this phase's pass criterion, per `PLAN.txt` Phase 2): the branch child runs and reaches the model-checker handshake without SIGSEGV or `ThreadSanitizer: CHECK failed` — the analog of Task 1's standalone "park" test passing, but through the real DMTCP-restart path. Full model-checking progress/completion is not required here (R5 join/exit shims are Phase 3); reaching the handshake without a crash is the pass criterion. + +- [ ] **Step 5: Record the observed outcome** + +No commit for this task (no source changes). If the pass criterion in Step 4 is met, note it in the PR/handoff description; if not, capture the log output and hand off to Phase 3 planning rather than attempting further fixes here (out of scope for this phase). diff --git a/docs/superpowers/plans/2026-07-05-tsan-port-phase3-r5-implementation.md b/docs/superpowers/plans/2026-07-05-tsan-port-phase3-r5-implementation.md new file mode 100644 index 00000000..e8270743 --- /dev/null +++ b/docs/superpowers/plans/2026-07-05-tsan-port-phase3-r5-implementation.md @@ -0,0 +1,349 @@ +# TSan Port Phase 3 (R5): pthread_join / pthread_exit shim — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a `pthread_exit()` interceptor (`mc_pthread_exit`) that fixes both a TSan-safety gap (a recreated/fiber thread's explicit `pthread_exit()` call currently falls through to libtsan's real interceptor, which can crash) and a pre-existing model-checking gap (explicit `pthread_exit()` calls are invisible to McMini's scheduler today, for every thread). `pthread_join` needs no change — confirmed unnecessary by static analysis in the design spec. + +**Architecture:** One new interceptor function, `mc_pthread_exit()`, mirroring the existing `mc_transparent_exit()`/`mc_transparent_abort()` mode-dispatch shape: pre-restart modes forward to a newly-added cached "real pthread_exit" handle (matching this codebase's established `dlopen`+`dlsym` bypass pattern); restart/branch modes route into the *already-existing* `mc_exit_thread_in_child()`/`mc_exit_main_thread_in_child()` machinery, for every thread uniformly (no per-thread "is this recreated" tracking needed — see design spec for why). + +**Tech Stack:** C11, CMake, `dlopen`/`dlsym`, POSIX threads. + +## Global Constraints + +- Build uses `-Wall -Werror` (`CMakeLists.txt:81`) — every change must compile warning-free. +- Design reference: `docs/superpowers/specs/2026-07-05-tsan-port-phase3-r5-design.md`. Follow it for rationale (in particular: why no `mtf_is_recreated_thread()`, no `exit_retval` field, and no fiber switch inside the shim are needed here, unlike the standalone package). +- Baseline: branch `tsan-record-thread-fix`, commit `e6575df` (Phase 3 design spec, on top of prior phases). +- `pthread_join` requires **no code change** — this plan does not touch `mc_pthread_join` at all. +- This project has no C unit-test harness (per `CLAUDE.md`); standalone tests are compiled and run directly with `gcc`, never via CMake. + +--- + +### Task 1: Standalone test proving the `libpthread_pthread_exit` forwarding technique + +**Files:** +- Create: `test/tsan_support/test_pthread_exit_forwarding.c` + +**Interfaces:** None consumed from other tasks. Produces no interface other tasks depend on — a standalone, self-contained proof that `dlopen("libpthread.so"/"libpthread.so.0")` + `dlsym(..., "pthread_exit")` resolves to a working real `pthread_exit` that correctly terminates a thread and delivers its `retval` to a real `pthread_join()`. Task 2 applies this exact technique to production, following the identical `dlopen`+`dlsym` pattern already used by every other `libpthread_*` handle in `src/lib/interception.c`. + +- [ ] **Step 1: Write the test** + +Create `test/tsan_support/test_pthread_exit_forwarding.c`: + +```c +// Standalone host-side unit test. Not an mcmini model-checking target. +// +// Proves the dlopen+dlsym technique Task 2 uses to add a cached "real +// pthread_exit" handle to src/lib/interception.c (matching the existing +// libpthread_pthread_join_ptr/libpthread_timedjoin_np_ptr pattern): resolve +// pthread_exit from a freshly dlopen'd libpthread.so/libpthread.so.0, call +// it from a thread, and confirm the retval reaches a real pthread_join(). +#define _GNU_SOURCE +#include +#include +#include +#include + +typedef void (*pthread_exit_fn)(void *); + +static pthread_exit_fn real_pthread_exit; + +static void *worker(void *arg) { + (void)arg; + real_pthread_exit((void *)(long)42); + return NULL; // not reached +} + +int main(void) { + void *libpthread_handle = dlopen("libpthread.so", RTLD_LAZY); + if (!libpthread_handle) { + libpthread_handle = dlopen("libpthread.so.0", RTLD_LAZY); + } + assert(libpthread_handle != NULL); + + real_pthread_exit = (pthread_exit_fn)dlsym(libpthread_handle, "pthread_exit"); + assert(real_pthread_exit != NULL); + + pthread_t t; + int rc = pthread_create(&t, NULL, worker, NULL); + assert(rc == 0); + + void *retval = NULL; + rc = pthread_join(t, &retval); + assert(rc == 0); + assert((long)retval == 42); + + printf("PASS\n"); + return 0; +} +``` + +- [ ] **Step 2: Compile and run — verify it passes** + +Run: +```bash +gcc -Wall -Werror test/tsan_support/test_pthread_exit_forwarding.c -o /tmp/test_pthread_exit_forwarding -ldl -lpthread +/tmp/test_pthread_exit_forwarding +echo "exit: $?" +``` + +Expected: compiles clean, prints `PASS`, `exit: 0`. (This test is expected to pass immediately — like Phase 1's `test_tid_from_descriptor_offset.c`, it documents and locks in a technique this codebase already uses elsewhere for other symbols, rather than driving new production code through a red/green cycle.) + +- [ ] **Step 3: Commit** + +```bash +git add test/tsan_support/test_pthread_exit_forwarding.c +git commit -m "Add standalone test proving the libpthread_pthread_exit dlopen+dlsym technique" +``` + +--- + +### Task 2: Add `mc_pthread_exit()` interceptor to production + +**Files:** +- Modify: `src/lib/interception.c:19` (add `libpthread_pthread_exit_ptr` declaration) +- Modify: `src/lib/interception.c:73` (resolve it in `mc_load_intercepted_pthread_functions()`) +- Modify: `src/lib/interception.c:241-242` (add the `pthread_exit`/`libpthread_pthread_exit` forwarding functions) +- Modify: `include/mcmini/spy/intercept/interception.h:35` (declare both) +- Modify: `include/mcmini/spy/intercept/wrappers.h:48` (declare `mc_pthread_exit`) +- Modify: `src/lib/wrappers.c:466` (add the `mc_pthread_exit()` implementation) + +**Interfaces:** +- Consumes: nothing from Task 1 directly (Task 1 is a standalone proof of technique, not linked code) — but Task 2 must produce a byte-for-byte-equivalent `dlopen`+`dlsym` resolution to what Task 1 proved works. +- Produces: `MCMINI_NO_RETURN void mc_pthread_exit(void *retval);` (declared in `wrappers.h`) and the public `void pthread_exit(void *) __attribute__((__noreturn__));` override (declared in `interception.h`). No later task in this plan consumes these directly; Task 3 exercises them end-to-end. + +- [ ] **Step 1: Declare the new cached-handle pointer** + +In `src/lib/interception.c`, current line 19: + +```c +typeof(&pthread_timedjoin_np) libpthread_timedjoin_np_ptr; +``` + +Add immediately after: + +```c +__attribute__((__noreturn__)) typeof(&pthread_exit) libpthread_pthread_exit_ptr; +``` + +- [ ] **Step 2: Resolve it in `mc_load_intercepted_pthread_functions()`** + +In `src/lib/interception.c`, current line 73: + +```c + libpthread_timedjoin_np_ptr = dlsym(libpthread_handle, "pthread_timedjoin_np"); +``` + +Add immediately after: + +```c + libpthread_pthread_exit_ptr = dlsym(libpthread_handle, "pthread_exit"); +``` + +- [ ] **Step 3: Add the public override and the forwarding wrapper** + +In `src/lib/interception.c`, current lines 238-242: + +```c +int libdmtcp_pthread_join(pthread_t thread, void **rv) { + libmcmini_init(); + return (*libdmtcp_pthread_join_ptr)(thread, rv); +} + +void exit(int status) { +``` + +Replace with: + +```c +int libdmtcp_pthread_join(pthread_t thread, void **rv) { + libmcmini_init(); + return (*libdmtcp_pthread_join_ptr)(thread, rv); +} + +MCMINI_NO_RETURN void pthread_exit(void *retval) { + mc_pthread_exit(retval); +} +MCMINI_NO_RETURN void libpthread_pthread_exit(void *retval) { + libmcmini_init(); + (*libpthread_pthread_exit_ptr)(retval); +} + +void exit(int status) { +``` + +- [ ] **Step 4: Declare the new interception.c functions in the header** + +In `include/mcmini/spy/intercept/interception.h`, current line 35: + +```c +int libpthread_timedjoin_np(pthread_t thread, void**, const struct timespec*); +``` + +Add immediately after (blank line, then the two declarations): + +```c + +void pthread_exit(void *) __attribute__((__noreturn__)); +// TSan-safe (libtsan-bypassing) handle for pthread_exit, used by +// mc_pthread_exit's pre-restart forwarding path. +void libpthread_pthread_exit(void *) __attribute__((__noreturn__)); +``` + +- [ ] **Step 5: Declare `mc_pthread_exit` in wrappers.h** + +In `include/mcmini/spy/intercept/wrappers.h`, current line 48: + +```c +MCMINI_NO_RETURN void mc_transparent_exit(int status); +``` + +Add immediately after: + +```c +MCMINI_NO_RETURN void mc_pthread_exit(void *retval); +``` + +- [ ] **Step 6: Implement `mc_pthread_exit()` in wrappers.c** + +In `src/lib/wrappers.c`, find the end of `mc_transparent_abort()` — current lines 462-468: + +```c + default: { + libc_abort(); + } + } +} + +struct mc_thread_routine_arg { +``` + +Replace with: + +```c + default: { + libc_abort(); + } + } +} + +MCMINI_NO_RETURN void mc_pthread_exit(void *retval) { + switch (get_current_mode()) { + case PRE_DMTCP_INIT: + case PRE_CHECKPOINT_THREAD: + case CHECKPOINT_THREAD: + case RECORD: + case PRE_CHECKPOINT: { + libpthread_pthread_exit(retval); + } + case DMTCP_RESTART_INTO_BRANCH: + case DMTCP_RESTART_INTO_TEMPLATE: + case TARGET_BRANCH: + case TARGET_BRANCH_AFTER_RESTART: { + if (tid_self == RID_MAIN_THREAD) { + mc_exit_main_thread_in_child(); + } else { + mc_exit_thread_in_child(); + } + } + default: { + libc_abort(); + } + } +} + +struct mc_thread_routine_arg { +``` + +(This exact text is unique in the file: `mc_transparent_exit()`'s `default:` case calls `libc_exit(status)`, not `libc_abort()`, so only `mc_transparent_abort()`'s closing block is immediately followed by the blank line and `struct mc_thread_routine_arg {` declaration. Already verified by test-applying this exact replacement in a scratch copy before this plan was written.) + +- [ ] **Step 7: Rebuild and verify** + +Run: +```bash +cmake --build build --target libmcmini +``` + +Expected: `[100%] Built target libmcmini` with no warnings (build uses `-Wall -Werror`). + +- [ ] **Step 8: Commit** + +```bash +git add src/lib/interception.c src/lib/wrappers.c include/mcmini/spy/intercept/interception.h include/mcmini/spy/intercept/wrappers.h +git commit -m "Add mc_pthread_exit() interceptor (R5: pthread_exit shim)" +``` + +--- + +### Task 3: End-to-end verification against a real TSan target under DMTCP (environment-gated) + +**Precondition:** this task requires the DMTCP toolchain (`dmtcp_launch`, `dmtcp_restart`) installed and on `PATH`, and the project built with `MCMINI_WITH_DMTCP=ON`. As of this plan's writing, this environment has neither (same gap Phase 1 and Phase 2's Task 3 hit). If unavailable when this task is reached, stop after Task 2, report Tasks 1-2 complete and committed, and hand this task off — do not mark it done without actually running it. + +**Files:** none (this task runs commands and observes output; it does not change source). + +- [ ] **Step 1: Reconfigure and rebuild with DMTCP enabled** + +```bash +cmake -S . -B build -DMCMINI_WITH_DMTCP=ON +cmake --build build --target libmcmini +``` + +Expected: build succeeds and produces `build/libmcmini.so`. + +- [ ] **Step 2: Build a TSan-instrumented example target that calls pthread_exit() explicitly** + +The existing example targets under `src/examples/` may not call `pthread_exit()` explicitly (most likely just `return` from their thread routines). Write a minimal target that does, so this task actually exercises the new interceptor: + +```c +// /tmp/pthread_exit_target.c -- minimal target exercising explicit pthread_exit() +#include +#include + +static void *worker(void *arg) { + (void)arg; + printf("worker exiting via pthread_exit()\n"); + pthread_exit((void *)1); + return NULL; // not reached +} + +int main(void) { + pthread_t t; + pthread_create(&t, NULL, worker, NULL); + void *retval; + pthread_join(t, &retval); + printf("joined, retval=%ld\n", (long)retval); + return 0; +} +``` + +```bash +gcc -fsanitize=thread -g -pthread -o /tmp/pthread_exit_target-tsan /tmp/pthread_exit_target.c +``` + +Expected: compiles with no errors. + +- [ ] **Step 3: Record a checkpoint** + +From the directory containing `libmcmini.so`: + +```bash +cd build +TSAN_OPTIONS="handle_segv=0 die_after_fork=0" \ + dmtcp_launch --disable-alloc-plugin -i 5 --with-plugin "$PWD/libmcmini.so" \ + /tmp/pthread_exit_target-tsan +``` + +Let it run for at least one checkpoint interval (5s) — the target itself finishes almost instantly, so this may need a `sleep` added to `worker()` before `pthread_exit()` to give the checkpoint interval time to fire; adjust the target in Step 2 if the process exits before a checkpoint is taken. Stop it (Ctrl-C) once a `ckpt_*.dmtcp` file appears in the current directory. + +Expected: a `ckpt_pthread_exit_target-tsan_*.dmtcp` file is created. + +- [ ] **Step 4: Restart under `mcmini` with `--multithreaded-fork`** + +```bash +setarch -R ./mcmini --from-checkpoint ckpt_pthread_exit_target-tsan_*.dmtcp --multithreaded-fork /tmp/pthread_exit_target-tsan +``` + +Expected (this phase's pass criterion): the branch reaches and executes the target's `pthread_exit()` call without SIGSEGV or `ThreadSanitizer: CHECK failed`, and the join in `main()` completes. This is the R5 analog of Phase 2's Task 3 pass criterion (reaching the model-checker handshake without crashing), extended to cover an explicit `pthread_exit()` call specifically. + +- [ ] **Step 5: Record the observed outcome** + +No commit for this task (no source changes, and the scratch target file lives outside the repo in `/tmp`). If the pass criterion in Step 4 is met, note it in the PR/handoff description; if not, capture the log output and hand off to Phase 4 planning rather than attempting further fixes here. diff --git a/docs/superpowers/specs/2026-07-04-tsan-port-phase1-r1-design.md b/docs/superpowers/specs/2026-07-04-tsan-port-phase1-r1-design.md new file mode 100644 index 00000000..c4369958 --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-tsan-port-phase1-r1-design.md @@ -0,0 +1,129 @@ +# Design: TSan Port Phase 1 (R1) — Exclude TSan-internal threads from the restart barrier + +Date: 2026-07-04 +Related: `PLAN.txt` (Phase 1, fix R1), branch `tsan-multithreaded-fork-port`, +baseline commit `b38da82` (Phase 0 cleanup). + +## Problem + +`template_thread()` in `src/lib/dmtcp-callback.c` computes a thread-count +barrier at DMTCP restart: + +```c +thread_count = (entries in /proc/self/task) - 2; // self + checkpoint thread +for (int i = 0; i < thread_count; i++) + libpthread_sem_wait_loop(&dmtcp_restart_sem); +``` + +Each userspace thread posts to `dmtcp_restart_sem` lazily, from inside +`thread_handle_after_dmtcp_restart()`, which only runs when that thread's next +libpthread call passes through one of libmcmini's own wrappers +(`sem-wrappers.c`, `wrappers.c`) and notices `is_in_restart_mode()`. There is +no active signal broadcast to every thread in this fast path (unlike the +standalone `multithreaded-fork-tsan-2.0` recipe, which uses a real signal +barrier). + +When the target is built with ThreadSanitizer, libtsan spawns its own +background thread at process start with all signals blocked. That thread +never calls a wrapped libpthread function, so it never reaches +`thread_handle_after_dmtcp_restart()` and never posts to `dmtcp_restart_sem` — +but it *is* counted in `/proc/self/task`, inflating `thread_count` by one. +`template_thread()` then waits forever for a post that will never come: the +"stabilization hang" described in `PLAN.txt` section 2/4 (D1). + +## Design + +### Files + +- New `src/lib/tsan_support.c` + `include/mcmini/spy/checkpointing/tsan_support.h` + (alongside the existing `record.h`, `alloc.h`, etc. in that directory). + This phase adds one function; later phases (R3 `__clone`, R4 fiber, R5 + join/exit) add their helpers to the same pair of files. +- `src/lib/tsan_support.c` added to the `libmcmini` source list in + `CMakeLists.txt`, alphabetically between `template/sig.c` and `wrappers.c`. + +### `tsan_support.c`: `thread_blocks_signal()` + +```c +int thread_blocks_signal(pid_t tid, int signo); +``` + +Direct port of the standalone's helper +(`multithreaded-fork-tsan-2.0/multithreaded_fork.c:323-337`): opens +`/proc/self/task//status`, parses the `SigBlk:` hex mask, and returns +whether bit `signo - 1` is set. Returns `0` (not blocked) if the status file +can't be opened (e.g. the thread already exited) — matching the standalone's +behavior; this is a pre-existing raciness in the directory scan, not +introduced by this change. + +Used as a probe, not a real signal path: nothing in the fast path sends +`SIG_MULTITHREADED_FORK` (`SIGRTMIN+6`, already `#define`d in +`dmtcp-callback.c`, currently otherwise unused there). We only check whether +each thread's `SigBlk` mask has that bit set, which is how libtsan's +background thread — which blocks all signals at creation — gets identified. +Ordinary application threads are not expected to block this reserved +real-time signal. + +### `dmtcp-callback.c`: per-tid classification in `template_thread()` + +Replace the blanket `thread_count -= 2` with explicit per-tid skips while +walking `/proc/self/task`: + +- Skip the tid equal to `syscall(SYS_gettid)` (the template thread itself). +- Skip the checkpoint thread's tid, obtained via a new **read-only** + `get_tid_from_pthread_descriptor(ckpt_pthread_descriptor)` helper added next + to the existing `patchThreadDescriptor()` / `pthreadDescriptorTidOffset()` + in `dmtcp-callback.c` (not `tsan_support.c` — this is generic + pthread-descriptor-layout infrastructure, not TSan-specific). Unlike + `patchThreadDescriptor()`, it must not mutate the descriptor; it only reads + the tid field at the same offset. +- Skip any tid where `thread_blocks_signal(tid, SIG_MULTITHREADED_FORK)` is + true, logging at debug level which tid was excluded and why. +- Everything else increments `thread_count`, exactly as today. + +### threadInfos[] recreation + +`PLAN.txt` also asks to ensure TSan-internal threads are excluded from +`threadInfos[]` recreation, not just from the count. Structurally, +`threadInfos[]` is populated only inside `thread_handle_after_dmtcp_restart()`, +which (as above) is reached only via a wrapped libpthread call. TSan's +background thread is not expected to make such a call, so it should never +register itself regardless. No defensive check is added in +`thread_handle_after_dmtcp_restart()` for this phase; this assumption is +verified empirically by the phase's pass criteria below (a crash or hang +caused by attempting to recreate the background thread would surface +immediately in Phase 2 testing when branch children run). + +## Error handling & edge cases + +- `opendir("/proc/self/task")` failure: unchanged (existing `perror` + + `mc_exit(EXIT_FAILURE)`). +- `thread_blocks_signal()` on a tid whose status file has already vanished: + returns `0`, not excluded — pre-existing raciness, out of scope here. +- `/proc/self/task` entries are always numeric tids, so `atoi(entry->d_name)` + is safe once `.`/`..` are filtered. + +## Testing / verification + +Matches `PLAN.txt` Phase 1 pass criteria: rebuild `libmcmini`, re-record a +checkpoint of a `-fsanitize=thread` example target (checkpoints must be +re-recorded after any libmcmini rebuild — PLAN.txt D4), then restart with +`--multithreaded-fork`. Success is the `template_thread()` debug log showing +a thread count matching the actual live (non-TSan-internal) thread count, +followed by "threads now in a consistent state" — instead of hanging. A +`log_debug` line names each excluded tid and the reason (self / checkpoint +thread / TSan-internal), making the exclusion directly observable in the log. + +No new unit-test harness is introduced — the project has no test harness yet +(per `CLAUDE.md`), and this is a runtime/scheduling fix best verified against +a real TSan target, per PLAN.txt's own phase methodology (section 7). + +## Out of scope for this phase + +- R2/R3/R4/R5 (Phase 2/3 of `PLAN.txt`) — fork hooks, `__clone`, fiber + switching, join/exit shims. R2/R4 keeper prototypes already exist in + `dmtcp-callback.c` from Phase 0 and are untouched here. +- Any defensive `threadInfos[]` registration check (see above) — deferred + until/unless empirical testing shows it's needed. +- Backporting to `src/common/multithreaded_fork.c` (PLAN.txt Q2) — that file + is not the live path for deep-debug's restart flow. diff --git a/docs/superpowers/specs/2026-07-04-tsan-port-phase2-r2-r3-r4-design.md b/docs/superpowers/specs/2026-07-04-tsan-port-phase2-r2-r3-r4-design.md new file mode 100644 index 00000000..fd540f0e --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-tsan-port-phase2-r2-r3-r4-design.md @@ -0,0 +1,181 @@ +# Design: TSan Port Phase 2 (R2 + R3 + R4) — fork hooks, __clone, fiber switching + +Date: 2026-07-04 +Related: `PLAN.txt` (Phase 2), branch `tsan-record-thread-fix` +(current tip after Phase 0 + Phase 1: commit `74d5126`). + +## Problem + +`PLAN.txt` Phase 2 calls for three of the five fixes proven in the standalone +package `multithreaded-fork-tsan-2.0` to be ported into libmcmini's fast +restart path (`src/lib/dmtcp-callback.c`): + +- **R2** — rerun TSan's fork pipeline around the raw `_Fork()` call in + `fast_multithreaded_fork()`. +- **R3** — recreate threads with libc's `__clone`, not the public `clone()` + (libtsan intercepts `clone()` and treats every call as a fork, corrupting + its thread-slot state for a `CLONE_THREAD` clone). +- **R4** — give every recreated thread, and the forking thread itself, a + fresh TSan fiber (a TSan `ThreadState` decoupled from the OS thread) before + either makes a TSan-intercepted call. + +Inspection of the current `dmtcp-callback.c` (inherited from Phase 0's +"keeper" prototypes, `PLAN.txt` section 5) shows **R2 and half of R4 are +already committed**: + +- R2 is fully done: `fast_multithreaded_fork()` already brackets `_Fork()` + with `__sanitizer_syscall_pre_impl_fork()` / `__sanitizer_syscall_post_impl_fork()` + (`dmtcp-callback.c:236-242`). +- R4's per-*recreated*-thread half is done: `thread_handle_after_dmtcp_restart()` + already switches onto a fresh fiber at its post-`setcontext()` resume point + (`dmtcp-callback.c:304-315`), satisfying PLAN.txt's "pick one [placement], + not both" instruction (the standalone's alternative placement, inside + `child_setcontext_fast()` before `setcontext()`, was not used and should + not be added — that would be the "both" PLAN.txt warns against). + +What remains: + +- **R3** is not done: `restart_child_threads_fast()` (`:186-208`) still calls + the public `clone()` (`:203-206`). +- **R4's other half** is not done: the *forking thread* (the thread that + calls `fast_multithreaded_fork()`, running on in the child after `_Fork()`) + never gets a fresh fiber. The standalone's `README` documents why this is + a separate, required fix: the forking thread keeps its fork-inherited TSan + `ThreadState`, whose shadow call stack starts at the parent's fork-time + depth and can overflow (crash in TSan's `FuncEntry`) as that thread keeps + running. + +## Design + +### Production code changes (`src/lib/dmtcp-callback.c`) + +**R3 — `__clone` instead of `clone()`:** + +Add a non-weak extern declaration next to the existing weak TSan externs +(`:37-51`): + +```c +extern int __clone(int (*fn)(void *), void *child_stack, int flags, + void *arg, ... /* pid_t *ptid, void *newtls, pid_t *ctid */); +``` + +(`__clone` is a normal libc-internal symbol, always present — unlike the +TSan hooks, which are weak because they only resolve when the binary links +libtsan.) + +In `restart_child_threads_fast()` (`:186-208`), replace the call to +`clone(child_setcontext_fast, stack, clone_flags, (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid)` +(`:203-206`) with the same call to `__clone(...)`. No other logic in that +function changes. + +**R4 remainder — fresh fiber for the forking thread:** + +In `fast_multithreaded_fork()`'s child branch (`:273-275`, currently just +`if (childpid == 0) { restart_child_threads_fast(); }`), add the fiber +switch *before* calling `restart_child_threads_fast()`: + +```c +if (childpid == 0) { // child process + // The forking thread keeps its inherited (fork-copied) TSan ThreadState, + // whose shadow call stack starts at the parent's fork-time depth and can + // overflow as this thread keeps running. Switch it onto a fresh fiber too. + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } + restart_child_threads_fast(); +} +``` + +This mirrors the standalone's exact placement and rationale +(`multithreaded_fork.c:450-457` in the reference package). + +### Standalone test harness + +The vendor package's own tests (`mtf_park`, `mtf_join`, `mtf_exit`) use a +realtime-signal broadcast to snapshot each thread's context — a materially +different mechanism from the fast path, which has no signal barrier at all +(per PLAN.txt D1): each thread calls `getcontext()` directly, as a plain +function call, when it happens to pass through +`thread_handle_after_dmtcp_restart()`. Reusing the vendor tests as-is would +validate the *fixes* but not the *actual resumption mechanism* this port +targets. + +New file: `test/tsan_support/test_fastpath_fork_clone_fiber.c` — a standalone +harness (compiled/run directly with `gcc -fsanitize=thread`, not via CMake, +matching Phase 1's `test/tsan_support/` convention) that mirrors +`dmtcp-callback.c`'s actual mechanism instead of the vendor's signal-based +one: + +- N worker threads (3, matching the vendor's default) each call `getcontext()` + directly inside their thread body (no signal handler), record their + `pthread_t`/TLS pointer, and — on the first pass (`getpid() == orig_pid`) + — post to a check-in semaphore and block. The main thread waits for all N + check-ins, then calls a `fast_multithreaded_fork()`-equivalent. +- That equivalent function reproduces R2 (fork hooks around `_Fork()`), the + new R4-remainder fix (fiber switch for the forking thread), and calls a + `restart_child_threads_fast()`-equivalent that uses `__clone()` (R3) to + recreate each worker via `child_setcontext_fast()`-equivalent + (`setTLSPointer` + `patchThreadDescriptor` + `setcontext`). +- Each recreated thread resumes exactly at its `getcontext()` call site and, + on this second pass (`getpid() != orig_pid`), takes the already-proven + fiber-switch branch (mirroring `thread_handle_after_dmtcp_restart()`'s + existing code) before proceeding. +- All workers (original, in the parent; recreated, in the child) then run + the same TSan-intercepted stress workload as the vendor's `test_park.c`: a + mutex-protected shared-counter increment loop, 100,000 iterations each. + Pass criterion: both parent and child print + `shared_counter=300000 (expected 300000)`, with no + `ThreadSanitizer: CHECK failed`, SEGV, or hang. +- Recreated threads never `pthread_join`/`pthread_exit` — they park forever + and the process exits via `_exit()`, matching the vendor's `test_park.c` + (R5 join/exit shims are PLAN.txt Phase 3, out of scope here). + +The harness necessarily duplicates a handful of small helpers already in +`dmtcp-callback.c` (`getTLSPointer`/`setTLSPointer`/`patchThreadDescriptor`/ +`pthreadDescriptorTidOffset`, x86_64-only) — the same accepted-duplication +precedent as Phase 1's `test_tid_from_descriptor_offset.c`, since linking +the real `dmtcp-callback.c` is impractical (confirmed in Phase 1 via `nm -u`: +~30 unresolved libmcmini-internal symbols). + +Run via `setarch -R` (disables ASLR) with +`TSAN_OPTIONS="handle_segv=0 die_after_fork=0"` — confirmed during this +design's investigation to make ThreadSanitizer work in this sandbox (a +bare `-fsanitize=thread` binary fails immediately with +`FATAL: ThreadSanitizer: unexpected memory mapping` without `setarch -R`; +with it, the vendor package's own `make check` passes all three of its +demos here). + +## Error handling & edge cases + +- `__clone`'s temporary per-thread stack (`malloc`'d in + `restart_child_threads_fast()`) is intentionally leaked, matching the + existing `// FIXME: This stack is a memory leak` comment at `:196-197` — + not a regression introduced or fixed by this phase, and irrelevant for a + short-lived branch/test process. +- The harness's check-in barrier is a plain counting semaphore with no + TSan-thread-exclusion logic (R1) — the harness only ever spawns its own + known worker threads, so there is no libtsan background thread to filter, + and R1 is already solved (Phase 1) and orthogonal to R2/R3/R4. + +## Testing / verification + +1. **Standalone harness**: built and run under `setarch -R` with the + `TSAN_OPTIONS` above; pass criterion is both parent and child printing + `shared_counter=300000 (expected 300000)` with no CHECK-failure/SEGV/hang. +2. **Production build**: `cmake --build build --target libmcmini` after the + R3/R4-remainder changes, must stay clean under `-Wall -Werror`. +3. **Environment-gated end-to-end task** (same posture as Phase 1's Task 3): + PLAN.txt's actual Phase 2 pass criterion — a real branch child reaching + the model-checker handshake under DMTCP + `--multithreaded-fork` without + SIGSEGV/CHECK — still requires the DMTCP toolchain, which remains absent + in this environment. Documented with exact commands for whoever has that + environment; not executed here. + +## Out of scope for this phase + +- R1 (already done, Phase 1) and R5 (join/exit shims — PLAN.txt Phase 3). +- The model-checker handshake (`thread_await_scheduler()`) and any + DMTCP-dependent verification. +- Re-touching the already-committed R2 fork-hook bracketing or the + recreated-thread fiber switch (both already correct and reviewed in + Phase 0/1; only the two gaps identified above are in scope). diff --git a/docs/superpowers/specs/2026-07-05-tsan-port-phase3-r5-design.md b/docs/superpowers/specs/2026-07-05-tsan-port-phase3-r5-design.md new file mode 100644 index 00000000..1ba8ce1d --- /dev/null +++ b/docs/superpowers/specs/2026-07-05-tsan-port-phase3-r5-design.md @@ -0,0 +1,243 @@ +# Design: TSan Port Phase 3 (R5) — pthread_join / pthread_exit shim for recreated threads + +Date: 2026-07-05 +Related: `PLAN.txt` (Phase 3), branch `tsan-record-thread-fix` +(current tip after Phase 0-2: commit `1b2354a`). + +## Problem + +`PLAN.txt` Phase 3 calls for porting R5 from the standalone package +`multithreaded-fork-tsan-2.0`: a `pthread_join`/`pthread_exit` shim so that +threads recreated via `restart_child_threads_fast()`'s `__clone()` (which are +TSan fibers, not TSan-registered "real" threads) can be joined and can exit +without tripping ThreadSanitizer's interceptors. PLAN.txt itself flags this as +"LIKELY SIMPLER than the standalone" and lists two open questions (§8, Q1) +that this design resolves via direct code-path analysis of the current +codebase (DMTCP is unavailable in this environment, so this analysis is +static, not a runtime reproduction — see Testing below). + +## Investigation findings + +### `pthread_join`: no code change needed + +`mc_pthread_join` (`src/lib/wrappers.c:732-810`) already strongly overrides +the public `pthread_join` symbol (via `interception.c:226-228`) in every +`libmcmini_mode`. Reading its full mode-dispatch: + +- `PRE_CHECKPOINT_THREAD` / `CHECKPOINT_THREAD`: forwards to + `libdmtcp_pthread_join` (a real, DMTCP-provided join). +- `RECORD` / `PRE_CHECKPOINT`: uses `libpthread_timedjoin_np` — already a + libtsan-bypassing handle, per the existing comment at + `interception.h:32-35`, for an unrelated, already-solved DMTCP/TSan + interaction (see `TSAN-McMini-DMTCP.txt`). +- `DMTCP_RESTART_INTO_BRANCH` / `DMTCP_RESTART_INTO_TEMPLATE` / + `TARGET_BRANCH` / `TARGET_BRANCH_AFTER_RESTART`: writes `THREAD_JOIN_TYPE` + into the shared-memory mailbox and calls `thread_wake_scheduler_and_wait()` + (or `thread_handle_after_dmtcp_restart()` pre-restart) — **a pure + mailbox/semaphore handshake with the `mcmini` scheduler process. No real + libc/libpthread join call is made on any of these paths.** + +Since libtsan's `pthread_join` interceptor can only fire if a call actually +reaches a real `pthread_join`/`pthread_timedjoin_np` symbol, and no code path +that applies to a recreated thread (which only exists in `TARGET_BRANCH*` +modes, post-restart) ever makes such a call, **libtsan's join interceptor +cannot fire for a recreated thread's join in this codebase.** This resolves +PLAN.txt §8 Q1: the standalone's join `CHECK failed` does not reproduce here, +because the crash-prone code path is structurally unreachable. + +### `pthread_exit`: real gap, but simpler shim than the standalone's + +`pthread_exit` is not intercepted anywhere in this codebase today (confirmed +via exhaustive grep — no `mc_pthread_exit`, no override in `interception.c`). +Two consequences: + +1. **TSan-safety gap** (PLAN.txt's stated concern): a recreated (fiber-only) + thread calling `pthread_exit()` explicitly falls through to libtsan's real + interceptor, which asserts the caller is a genuine TSan thread — a likely + crash, matching the standalone's documented failure. +2. **Pre-existing model-checking gap** (found during this investigation, not + in PLAN.txt): McMini's model already has full `THREAD_EXIT_TYPE` + machinery — `mc_exit_thread_in_child()` / `mc_exit_main_thread_in_child()` + (`wrappers.c:349-375`), wired into the transition registry + (`include/mcmini/model/transitions/thread/thread_exit.hpp`, + `src/mcmini/model/transition_registry.cpp:18`) — but it is only invoked + today when a thread's routine *returns normally*, via + `mc_thread_routine_wrapper()`'s epilogue (`wrappers.c:533-561`). An + explicit `pthread_exit()` call bypasses this entirely, for *any* thread, + TSan or not, today. + +The user chose to fix both together (see Approach below). + +**Why this shim can be simpler than the standalone's stash+fiber+raw-exit +design:** reading `mc_exit_thread_in_child()`/`mc_exit_main_thread_in_child()` +in full — both end in `thread_block_indefinitely()` (`wrappers.c:122-126`, +`while(1) pause();`). **Neither ever terminates the OS thread at the kernel +level.** McMini's "thread exit" is purely a model/scheduler bookkeeping +event; the whole branch *process* is discarded after its trace is explored, +not individual threads. This means: + +- There is no OS-level termination this shim needs to arrange (the + standalone's `CLONE_CHILD_CLEARTID`-triggered real-join-wakeup has no + analog here — Finding 1 above already established joins never touch real + OS join primitives in these modes). +- A recreated thread already has a valid TSan fiber by the time any of its + own code runs post-resume (the R4 fix, Phase 2, switches it in + immediately). Calling `mc_exit_thread_in_child()` — itself just ordinary + mutex/semaphore/shared-memory calls — is no more dangerous than any other + TSan-instrumented call that thread already makes routinely (e.g. + `thread_await_scheduler()`, called immediately after every resume). No + fiber switch is needed *inside* the exit shim specifically. +- `mc_pthread_join`'s `TARGET_BRANCH*` case (already read in full above) + never touches its `void **rv` output parameter — a joined thread's return + value is already unconditionally discarded in this model-checked path + today. A retval-stash mechanism (the standalone's + `mtf_set_exit_retval`/`mtf_get_exit_retval`) would stash a value nothing + downstream ever reads. + +Net effect: **no `mtf_is_recreated_thread()`, no `exit_retval` field, no +fiber switch inside the shim** — a real simplification vs. the standalone +package, matching PLAN.txt's own "likely simpler" prediction. + +## Design + +### `mc_pthread_exit()` — new interceptor in `src/lib/wrappers.c` + +Placed alongside `mc_transparent_exit()`/`mc_transparent_abort()` +(`wrappers.c:377-421`), whose mode-dispatch shape it mirrors: + +```c +MCMINI_NO_RETURN void mc_pthread_exit(void *retval) { + switch (get_current_mode()) { + case PRE_DMTCP_INIT: + case PRE_CHECKPOINT_THREAD: + case CHECKPOINT_THREAD: + case RECORD: + case PRE_CHECKPOINT: { + libpthread_pthread_exit(retval); + } + case DMTCP_RESTART_INTO_BRANCH: + case DMTCP_RESTART_INTO_TEMPLATE: + case TARGET_BRANCH: + case TARGET_BRANCH_AFTER_RESTART: { + if (tid_self == RID_MAIN_THREAD) { + mc_exit_main_thread_in_child(); + } else { + mc_exit_thread_in_child(); + } + } + default: { + libc_abort(); + } + } +} +``` + +(`libpthread_pthread_exit()` and `mc_exit_main_thread_in_child()` / +`mc_exit_thread_in_child()` are all `MCMINI_NO_RETURN`-equivalent in +practice — they either call a real `noreturn` function or park forever — +so no `return`/`break` is reachable in any branch, matching the function's +own `MCMINI_NO_RETURN` contract.) + +**Mode coverage, mirroring `mc_pthread_join`'s exact enumeration style** +(`enum libmcmini_mode` has 11 values, `record.h:124-139`): + +- Pre-restart modes (real OS threads, not yet under model-checker control): + forward to the real `pthread_exit`, preserving `retval`. Unlike + `mc_pthread_join`, `PRE_DMTCP_INIT` does not need a special `assert(0)` + here — that case in `mc_pthread_join` guards a DMTCP-internal-join edge + case with no analog for a target's own `pthread_exit()` call. +- Restart/branch modes: dispatch on the *existing* `tid_self` TLS variable + (`wrappers.c:57`, already set by `mc_register_this_thread()` — no new + state needed) to pick the main-thread-preserving variant vs. the normal + one, exactly mirroring `mc_thread_routine_wrapper()`'s own implicit + choice (it only ever calls `mc_exit_thread_in_child()`, since `main()` + never runs through that wrapper in the first place). +- `default:` (covers `TARGET_TEMPLATE` / `TARGET_TEMPLATE_AFTER_RESTART`, + the two `enum` values `mc_pthread_join` also doesn't enumerate) → + `libc_abort()`, consistent with the existing invariant that a template + process's userspace threads are permanently parked and should never reach + wrapper code in these modes. + +### `libpthread_pthread_exit()` — new cached real-function handle + +Follows the established `dlopen`+`dlsym` pattern used by every existing +`libpthread_*`/`libdmtcp_*` handle (`interception.c:15-40` declarations, +`:46-112` resolution in `mc_load_intercepted_pthread_functions()`, +`:114-300`ish forwarding wrappers) — a single new handle, no +`libdmtcp_pthread_exit` variant (unlike join, there is no documented +DMTCP-specific interception concern for `pthread_exit`, so one handle +covers every pre-restart mode uniformly): + +```c +// interception.c, near the other pointer declarations (~line 18): +__attribute__((__noreturn__)) typeof(&pthread_exit) libpthread_pthread_exit_ptr; + +// in mc_load_intercepted_pthread_functions(), near the other libpthread_handle +// resolutions (~line 73): +libpthread_pthread_exit_ptr = dlsym(libpthread_handle, "pthread_exit"); + +// new forwarding wrappers, alongside pthread_join's family (~line 241): +MCMINI_NO_RETURN void pthread_exit(void *retval) { + mc_pthread_exit(retval); +} +MCMINI_NO_RETURN void libpthread_pthread_exit(void *retval) { + libmcmini_init(); + (*libpthread_pthread_exit_ptr)(retval); +} +``` + +This handle is TSan-bypassing by construction (same `dlopen`'d-fresh-module +technique as every existing handle in this family), matching this +codebase's uniform existing convention (`libc_exit`, `libc_abort`, +`libpthread_timedjoin_np`, `libdmtcp_pthread_join` are all TSan-bypassing +the same way) rather than a new, inconsistent choice for this one call. + +### Declarations + +- `include/mcmini/spy/intercept/wrappers.h` (near `mc_pthread_join` at `:28` + and `mc_transparent_exit`/`abort` at `:47-48`): + `MCMINI_NO_RETURN void mc_pthread_exit(void *retval);` +- `include/mcmini/spy/intercept/interception.h` (near the `pthread_join` + family at `:29-35`): `void pthread_exit(void *) __attribute__((__noreturn__));` + and `void libpthread_pthread_exit(void *) __attribute__((__noreturn__));` + +## Error handling & edge cases + +- `default:` case (`TARGET_TEMPLATE` / `TARGET_TEMPLATE_AFTER_RESTART`) + aborts, matching the existing invariant already enforced the same way by + `mc_pthread_join`. +- The main-thread-vs-not dispatch reuses `tid_self` (already-existing TLS + state, `wrappers.c:57`) — no new per-thread state is introduced. +- No new `struct threadinfo` fields, no new global tables. + +## Testing + +1. **`pthread_join`**: no code change, so no new test — the "no fix needed" + conclusion rests on the static mode-dispatch analysis above (per user + decision, sufficient given there is no dynamic behavior on the mailbox + path to exercise, and DMTCP is unavailable here for a live reproduction). +2. **`libpthread_pthread_exit()` (pre-restart forwarding half)**: a + standalone test proving the new cached handle resolves and correctly + terminates a thread with the given `retval`, verified via a real + `pthread_join()` on that thread returning the expected value. This + exercises the same `RECORD`/pre-restart code path in isolation, without + needing DMTCP. +3. **Restart-mode dispatch (`mc_exit_thread_in_child`/ + `mc_exit_main_thread_in_child` routing)**: tightly coupled to the real + shared-memory mailbox protocol and the live `mcmini` scheduler process — + unlike Phase 2's fork/clone/fiber mechanism, not practically mirrorable + in a self-contained standalone harness without reimplementing a chunk of + the scheduler's own protocol. This becomes an environment-gated + end-to-end task, matching Phase 1/2's Task 3 pattern (requires the + DMTCP toolchain, absent in this environment). +4. **Production build**: `cmake --build build --target libmcmini` must stay + clean under `-Wall -Werror`. + +## Out of scope for this phase + +- Any change to `mc_pthread_join` (confirmed unnecessary). +- `mtf_is_recreated_thread()`, `exit_retval` field, or any fiber switch + inside the exit shim (confirmed unnecessary by the "never really + terminates the OS thread" finding above). +- Phase 4 (PLAN.txt: hardening — multiple sequential branches, higher + thread counts, real example targets built with `-fsanitize=thread`). diff --git a/include/dmtcp.h b/include/dmtcp.h index d0e774e4..1a81a714 100644 --- a/include/dmtcp.h +++ b/include/dmtcp.h @@ -16,6 +16,7 @@ #define DMTCP_H #include +#include #include #include #include @@ -43,8 +44,8 @@ # define EXTERNC #endif // ifdef __cplusplus -/* Define to the version of this package. */ -#define DMTCP_PLUGIN_API_VERSION "3" +/* Bump when DmtcpPluginDescriptor_t changes ABI. */ +#define DMTCP_PLUGIN_API_VERSION "4" #ifdef __cplusplus namespace dmtcp { @@ -238,12 +239,78 @@ void dmtcp_initialize_plugin(void) __attribute((weak)); typedef struct DmtcpUniqueProcessId { uint64_t _hostid; // gethostid() uint64_t _time; // time() - pid_t _pid; // getpid() + + union { + pid_t _pid; // getpid() + int32_t _; + }; + uint32_t _computation_generation; // computationGeneration() } DmtcpUniqueProcessId; int dmtcp_unique_pids_equal(DmtcpUniqueProcessId a, DmtcpUniqueProcessId b); +typedef struct DmtcpInfo { + int argc; + const char **argv; +} DmtcpInfo; + +enum ElfType { + Elf_32, + Elf_64 +}; + +typedef struct { + uint64_t startAddr; + uint64_t endAddr; +} MemRegion; + +typedef void (*PostRestartFnPtr_t)(double, int); +#define DMTCP_CKPT_SIGNATURE "DMTCP_CHECKPOINT_IMAGE_v4.0\n" +typedef struct { + char ckptSignature[32]; + + DmtcpUniqueProcessId upid; + DmtcpUniqueProcessId uppid; + DmtcpUniqueProcessId compGroup; + + pid_t pid; + pid_t ppid; + pid_t sid; + pid_t gid; + pid_t fgid; + uint32_t isRootOfProcessTree; + + uint32_t numPeers; + uint32_t elfType; + + uint64_t clock_gettime_offset; + uint64_t getcpu_offset; + uint64_t gettimeofday_offset; + uint64_t time_offset; + + // Reserve 3 * 30MB for restore buffer. +#define RESTORE_BUF_TOTAL_SIZE (90 * 1024 * 1024) + MemRegion restoreBuf; + + MemRegion vdso; + MemRegion vvar; + MemRegion vvarVClock; + + uint64_t savedBrk; + uint64_t endOfStack; + + uint64_t postRestartAddr; + //void (*post_restart)(double, int); + + char procname[1024]; + char procSelfExe[1024]; + + char padding[1792]; +} DmtcpCkptHeader; + +static_assert(sizeof(DmtcpCkptHeader) == 4096, "DmtcpCkptHeader must be 4096 bytes"); + // FIXME: // If a plugin is not compiled with defined(__PIC__) and we can verify // that we're using DMTCP (environment variables), and dmtcp_is_enabled @@ -329,8 +396,6 @@ const char *dmtcp_get_ckpt_files_subdir(void); int dmtcp_should_ckpt_open_files(void); int dmtcp_allow_overwrite_with_ckpted_files(void); int dmtcp_skip_truncate_file_at_restart(const char* path); -void dmtcp_set_restore_buf_addr(void *new_addr, uint64_t len); -uint64_t dmtcp_restore_buf_len(); int dmtcp_get_ckpt_signal(void); const char *dmtcp_get_uniquepid_str(void) __attribute__((weak)); @@ -428,13 +493,16 @@ int dmtcp_protected_environ_fd(void); * discovers a pid without going through a system call (e.g., through * the proc filesystem), use this to virtualize the pid. */ -pid_t dmtcp_real_to_virtual_pid(pid_t realPid) __attribute((weak)); -pid_t dmtcp_virtual_to_real_pid(pid_t virtualPid) __attribute((weak)); +pid_t dmtcp_pid_real_to_virtual(pid_t realPid) __attribute((weak)); +pid_t dmtcp_pid_virtual_to_real(pid_t virtualPid) __attribute((weak)); +// McMini helpers: translate pids only when running under DMTCP. +// (DMTCP plugin API v4 renamed dmtcp_{real_to_virtual,virtual_to_real}_pid to +// dmtcp_pid_{real_to_virtual,virtual_to_real}.) #define mcmini_virtual_pid(PID) \ - (dmtcp_is_enabled() ? dmtcp_real_to_virtual_pid((PID)) : (PID)) + (dmtcp_is_enabled() ? dmtcp_pid_real_to_virtual((PID)) : (PID)) #define mcmini_real_pid(PID) \ - (dmtcp_is_enabled() ? dmtcp_virtual_to_real_pid((PID)) : (PID)) + (dmtcp_is_enabled() ? dmtcp_pid_virtual_to_real((PID)) : (PID)) // bq_file -> "batch queue file"; used only by batch-queue plugin int dmtcp_is_bq_file(const char *path) __attribute((weak)); @@ -445,7 +513,7 @@ int dmtcp_bq_restore_file(const char *path, int type) __attribute((weak)); /* These next two functions are defined in contrib/ckptfile/ckptfile.cpp - * But they are currently used only in src/plugin/ipc/file/fileconnection.cpp + * But they are currently used only in src/plugin/file/fileconnection.cpp * and in a trivial fashion. These are intended for future extensions. */ int dmtcp_must_ckpt_file(const char *path) __attribute((weak)); diff --git a/include/mcmini/mem.h b/include/mcmini/mem.h index 0d2154ca..8cd2cfd8 100644 --- a/include/mcmini/mem.h +++ b/include/mcmini/mem.h @@ -9,6 +9,18 @@ extern "C" { volatile void *memset_v(volatile void *, int ch, size_t n); volatile void *memcpy_v(volatile void *, const volatile void *, size_t n); +// TSan-safe bump allocator over a fixed static (BSS) arena. +// +// Used for allocations that may execute on a thread BEFORE ThreadSanitizer has +// registered it -- specifically the mc_thread_routine_wrapper prologue when a +// TSAN'd target runs under DMTCP (libtsan wraps libmcmini, so libmcmini's +// wrapper runs before libtsan's thread-start trampoline). It performs NO libc +// call and NO syscall on the fast path -- only a static-memory atomic bump -- +// so it never enters a TSan interceptor and never dereferences an +// unregistered thread's (null) ThreadState. Never frees (nodes it backs are +// never freed; see record.c / wrappers.c). See TSAN-McMini-DMTCP.txt. +void *mc_ts_alloc(size_t n); + #ifdef __cplusplus } // extern "C" #endif diff --git a/include/mcmini/model/pending_transitions.hpp b/include/mcmini/model/pending_transitions.hpp index 668db878..ec65268d 100644 --- a/include/mcmini/model/pending_transitions.hpp +++ b/include/mcmini/model/pending_transitions.hpp @@ -43,6 +43,7 @@ struct pending_transitions final { } auto cend() -> decltype(_contents.cend()) const { return _contents.cend(); } size_t size() const { return this->_contents.size(); } + bool empty() const { return this->_contents.empty(); } /** * @brief Returns the transition mapped to id `id`, or `nullptr` if no such * runner has been mapped to an id. diff --git a/include/mcmini/model/state.hpp b/include/mcmini/model/state.hpp index c161816e..f08344ff 100644 --- a/include/mcmini/model/state.hpp +++ b/include/mcmini/model/state.hpp @@ -102,6 +102,11 @@ class mutable_state : public state { */ std::unique_ptr clone() const { return this->mutable_clone(); } + // Bring the base-class (virtual and template) overloads into scope so the + // templates declared below do not hide them (`-Werror=overloaded-virtual`). + using state::get_state_of_object; + using state::get_state_of_runner; + template const concrete_visible_object_state *get_state_of_object(objid_t id) const { return (static_cast(this)) diff --git a/include/mcmini/model_checking/algorithms/classic_dpor/stack_item.hpp b/include/mcmini/model_checking/algorithms/classic_dpor/stack_item.hpp index 8e41fb50..992440a2 100644 --- a/include/mcmini/model_checking/algorithms/classic_dpor/stack_item.hpp +++ b/include/mcmini/model_checking/algorithms/classic_dpor/stack_item.hpp @@ -1,10 +1,12 @@ #pragma once +#include #include #include #include #include "mcmini/defines.h" #include "mcmini/model_checking/algorithms/classic_dpor/clock_vector.hpp" +#include "mcmini/model_checking/algorithms/classic_dpor/runner_item.hpp" namespace model_checking { diff --git a/include/mcmini/spy/checkpointing/record.h b/include/mcmini/spy/checkpointing/record.h index 59388a26..2a4309f4 100644 --- a/include/mcmini/spy/checkpointing/record.h +++ b/include/mcmini/spy/checkpointing/record.h @@ -187,6 +187,11 @@ bool is_dmtcp_object(void *addr); /// @note you must acquire `rec_list_lock` before calling this function rec_list *add_rec_entry(const visible_object *, rec_list **, rec_list **); rec_list *add_rec_entry_record_mode(const visible_object *); +// Like add_rec_entry_record_mode, but allocates the node with mc_ts_alloc +// instead of malloc, so it is safe to call from a thread that libtsan has not +// yet registered (the mc_thread_routine_wrapper prologue under DMTCP). See +// TSAN-McMini-DMTCP.txt. +rec_list *add_rec_entry_record_mode_ts(const visible_object *); void print_rec_list(const rec_list *); rec_list *add_dmctp_object(const visible_object *); diff --git a/include/mcmini/spy/checkpointing/tsan_support.h b/include/mcmini/spy/checkpointing/tsan_support.h new file mode 100644 index 00000000..9bd225a4 --- /dev/null +++ b/include/mcmini/spy/checkpointing/tsan_support.h @@ -0,0 +1,18 @@ +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +/** + * Returns nonzero if thread `tid` currently has signal `signo` blocked, per + * the "SigBlk" field of /proc/self/task//status. Returns 0 if `tid` + * cannot be inspected (e.g. it has already exited). + */ +int thread_blocks_signal(pid_t tid, int signo); + +#ifdef __cplusplus +} +#endif diff --git a/include/mcmini/spy/intercept/interception.h b/include/mcmini/spy/intercept/interception.h index 105f024f..05ad5664 100644 --- a/include/mcmini/spy/intercept/interception.h +++ b/include/mcmini/spy/intercept/interception.h @@ -29,6 +29,16 @@ int libdmtcp_pthread_create(pthread_t *thread, const pthread_attr_t *attr, int pthread_join(pthread_t thread, void**); int libpthread_pthread_join(pthread_t thread, void**); int libdmtcp_pthread_join(pthread_t thread, void**); +// TSan-safe (libtsan-bypassing) handle for pthread_timedjoin_np, used by +// mc_pthread_join's RECORD loop. Calling pthread_timedjoin_np directly resolves +// to libtsan's interceptor, which trips a thread-registry CHECK. See +// TSAN-McMini-DMTCP.txt. +int libpthread_timedjoin_np(pthread_t thread, void**, const struct timespec*); + +MCMINI_NO_RETURN void pthread_exit(void *); +// TSan-safe (libtsan-bypassing) handle for pthread_exit, used by +// mc_pthread_exit's pre-restart forwarding path. +MCMINI_NO_RETURN void libpthread_pthread_exit(void *); int libpthread_mutex_init(pthread_mutex_t *, const pthread_mutexattr_t *); int libpthread_mutex_lock(pthread_mutex_t *); diff --git a/include/mcmini/spy/intercept/wrappers.h b/include/mcmini/spy/intercept/wrappers.h index 88208668..dc96eb39 100644 --- a/include/mcmini/spy/intercept/wrappers.h +++ b/include/mcmini/spy/intercept/wrappers.h @@ -2,9 +2,15 @@ #include +#include "mcmini/defines.h" #include "mcmini/lib/entry.h" #include "mcmini/real_world/mailbox/runner_mailbox.h" +// See definition in wrappers.c: set while libmcmini creates one of its own +// helper threads, so mc_pthread_create creates it plainly (TSAN-visible, +// DMTCP-known) instead of as a model-checked user thread. +extern MCMINI_THREAD_LOCAL int mc_creating_internal_thread; + void thread_await_scheduler(void); void thread_wake_scheduler_and_wait(void); void thread_awake_scheduler_for_thread_finish_transition(void); @@ -40,3 +46,4 @@ int mc_pthread_cond_destroy(pthread_cond_t *cond); void mc_exit_main_thread_in_child(void); MCMINI_NO_RETURN void mc_transparent_abort(void); MCMINI_NO_RETURN void mc_transparent_exit(int status); +MCMINI_NO_RETURN void mc_pthread_exit(void *retval); diff --git a/src/common/mem.c b/src/common/mem.c index 6e1b61aa..97cf8e43 100644 --- a/src/common/mem.c +++ b/src/common/mem.c @@ -1,5 +1,29 @@ #include "mcmini/mem.h" +#include +#include +#include + +// See mc_ts_alloc() in mem.h for why this allocator exists and why it must not +// call libc or a wrapped syscall on its fast path. +#define MC_TS_ARENA_SIZE (4u * 1024 * 1024) +static char mc_ts_arena[MC_TS_ARENA_SIZE]; +static atomic_size_t mc_ts_off = 0; + +void *mc_ts_alloc(size_t n) { + n = (n + 15u) & ~(size_t)15u; // 16-byte align + size_t off = atomic_fetch_add(&mc_ts_off, n); + if (off + n > MC_TS_ARENA_SIZE) { + // Exhausted. We may be running before this thread is registered with TSan, + // so we cannot fall back to malloc (it would enter a TSan interceptor). + // Fail via raw syscalls only (no interceptors). + static const char msg[] = "libmcmini: mc_ts_alloc arena exhausted\n"; + syscall(SYS_write, 2, msg, sizeof(msg) - 1); + syscall(SYS_exit_group, 1); + } + return &mc_ts_arena[off]; +} + volatile void *memset_v(volatile void *dst, int ch, size_t n) { volatile unsigned char *dstc = dst; while ((n--) > 0) dstc[n] = ch; diff --git a/src/common/multithreaded_fork.c b/src/common/multithreaded_fork.c index 7d60a103..1112c954 100644 --- a/src/common/multithreaded_fork.c +++ b/src/common/multithreaded_fork.c @@ -177,7 +177,7 @@ pid_t get_tid_from_pthread_descriptor(pthread_t pthread_descriptor) { int offset = pthreadDescriptorTidOffset(); pid_t ctid = *(pid_t*)((char*)(pthread_descriptor) + offset); #ifdef DMTCP - pid_t virttid = dmtcp_real_to_virtual_pid(ctid); + pid_t virttid = dmtcp_pid_real_to_virtual(ctid); ctid = (virttid ? virttid : ctid); #endif return ctid; @@ -209,7 +209,7 @@ int get_child_threads(int child_threads[]) { if (atoi(entry->d_name) != 0) { pid_t nexttid = atoi(entry->d_name); #ifdef DMTCP - pid_t virttid = dmtcp_real_to_virtual_pid(nexttid); + pid_t virttid = dmtcp_pid_real_to_virtual(nexttid); nexttid = (virttid ? virttid : nexttid); #endif child_threads[i++] = nexttid; diff --git a/src/lib/dmtcp-callback.c b/src/lib/dmtcp-callback.c index 7438c57a..2dba561c 100644 --- a/src/lib/dmtcp-callback.c +++ b/src/lib/dmtcp-callback.c @@ -20,9 +20,46 @@ #include "dmtcp.h" #include "mcmini/mcmini.h" +#include "mcmini/spy/checkpointing/tsan_support.h" #define SIG_MULTITHREADED_FORK (SIGRTMIN+6) +// ThreadSanitizer Fiber API (weak: resolves to the TSAN runtime only for TSAN +// targets, NULL no-op otherwise). A "fiber" is a TSAN ThreadState decoupled +// from the OS thread. `multithreaded_fork` recreates the pre-checkpoint threads +// with clone() + setcontext (so their original %fs/TLS and pthread descriptor +// are preserved and libmcmini does not intercept the creation). But clone() +// means libtsan never registered these threads, so their cur_thread() is torn +// on resume and the first TSAN-intercepted call (e.g. munmap) faults. Mirroring +// DMTCP's own restore fix (threadlist.cpp: "fresh fiber on restart"), each +// resurrected thread switches onto a fresh fiber the moment it resumes, giving +// it a valid ThreadState before any traced call. +extern void *__tsan_create_fiber(unsigned flags) __attribute__((weak)); +extern void __tsan_switch_to_fiber(void *fiber, unsigned flags) + __attribute__((weak)); +extern void __tsan_destroy_fiber(void *fiber) __attribute__((weak)); + +// TSan fork syscall hooks (weak). `_Fork()` bypasses libtsan's fork() +// interceptor, so TSan's BeforeFork/AfterFork pipeline never runs: the child +// inherits TSan's runtime with internal locks still held and a ThreadRegistry +// full of now-dead parent threads. The first instrumented call in the child +// then faults. Bracketing `_Fork()` with these hooks re-runs that pipeline -- +// pre acquires TSan's internal locks (parent), post releases them and, in the +// child, scrubs the registry down to the calling thread. This MUST happen +// before any fiber work, since __tsan_create_fiber itself touches the registry. +extern void __sanitizer_syscall_pre_impl_fork(void) __attribute__((weak)); +extern void __sanitizer_syscall_post_impl_fork(long res) __attribute__((weak)); + +// libc's internal clone (NOT intercepted by libtsan, unlike the public +// clone()). libtsan's clone() interceptor treats every call as a fork +// (ForkChildAfter), which corrupts its thread-slot state for the +// CLONE_THREAD clone restart_child_threads_fast() performs below. __clone is +// not intercepted, so it performs the raw thread creation; the recreated +// thread's own fiber switch (see thread_handle_after_dmtcp_restart()) then +// gives it a valid TSan ThreadState. +extern int __clone(int (*fn)(void *), void *child_stack, int flags, + void *arg, ... /* pid_t *ptid, void *newtls, pid_t *ctid */); + // We probably won't need the '#undef', but just in case a .h file defined it: #undef dmtcp_mcmini_is_loaded int dmtcp_mcmini_is_loaded(void) { return 1; } @@ -111,6 +148,16 @@ static inline pid_t patchThreadDescriptor(pthread_t pthreadSelf) { return oldtid; } +// Read-only sibling of patchThreadDescriptor(): returns the tid recorded in +// `pthread_descriptor` without mutating it. Used to look up the checkpoint +// thread's tid (a *different* thread's descriptor) from the template thread; +// patchThreadDescriptor() is only ever called by a thread on its own +// descriptor. +static inline pid_t get_tid_from_pthread_descriptor(pthread_t pthread_descriptor) { + int offset = pthreadDescriptorTidOffset(); + return *(pid_t *)((char *)pthread_descriptor + offset); +} + static void saveThreadStateBeforeFork(struct threadinfo* threadInfo) { threadInfo->origTid = syscall(SYS_gettid); @@ -163,7 +210,9 @@ void restart_child_threads_fast(void) { pid_t *ctid = (pid_t*)((char*)threadInfos[i].pthread_descriptor + offset); pid_t *ptid = ctid; // For more insight, read 'man set_tid_address'. - clone(child_setcontext_fast, + // R3: use libc's raw __clone, not the public clone() (see the __clone + // extern declaration above for why). + __clone(child_setcontext_fast, stack, clone_flags, (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); @@ -193,7 +242,16 @@ pid_t fast_multithreaded_fork(void) { *********************************************************************/ #if 1 pid_t _Fork(); + // Re-run TSan's fork pipeline around the raw _Fork() (see the syscall-hook + // declarations above). pre = BeforeFork (parent acquires TSan locks); + // post = AfterFork (release; child scrubs the ThreadRegistry). + if (__sanitizer_syscall_pre_impl_fork != NULL) { + __sanitizer_syscall_pre_impl_fork(); + } int childpid = _Fork(); + if (__sanitizer_syscall_post_impl_fork != NULL) { + __sanitizer_syscall_post_impl_fork(childpid); + } #else // NOT YET FULLY DEVELOPED: int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD; @@ -225,6 +283,15 @@ int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg, # endif #endif if (childpid == 0) { // child process + // R4 (remainder): the forking thread keeps its inherited (fork-copied) + // TSan ThreadState, whose shadow call stack starts at the parent's + // fork-time depth and can overflow as this thread keeps running. Switch + // it onto a fresh fiber too, mirroring the recreated-thread fiber switch + // in thread_handle_after_dmtcp_restart(). Weak symbol: a no-op for + // non-TSan targets. + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } restart_child_threads_fast(); } return childpid; @@ -256,7 +323,16 @@ void thread_handle_after_dmtcp_restart(void) { notify_template_thread(); } else { - // Returned from `getcontext()` in the forked child + // Returned from `getcontext()` in the forked child: this thread was just + // resurrected by `restart_child_threads_fast` via clone() + setcontext. + // Before any TSAN-intercepted call below, switch onto a fresh TSAN fiber so + // this OS thread has a valid ThreadState (clone() bypassed libtsan's + // pthread_create registration, leaving cur_thread() torn). Weak symbol: a + // no-op for non-TSAN targets. The fiber is intentionally not destroyed -- + // the branch process is short-lived and exits after one trace. + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } } switch (mode_on_entry) { @@ -329,15 +405,65 @@ static void *template_thread(void *unused) { mc_exit(EXIT_FAILURE); } - while ((entry = readdir(dp))) - if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) - thread_count++; + const pid_t self_tid = syscall(SYS_gettid); + const pid_t ckpt_tid = get_tid_from_pthread_descriptor(ckpt_pthread_descriptor); + + // Self-check: get_tid_from_pthread_descriptor() reads the same offset that + // patchThreadDescriptor() already relies on and that saveThreadStateBeforeFork() + // already self-verifies for `pthread_self()` on every restart. Confirm the + // read-only variant agrees for the template thread's own descriptor before + // trusting it to read the checkpoint thread's descriptor above. + if (get_tid_from_pthread_descriptor(pthread_self()) != self_tid) { + fprintf(stderr, + "PID %d: template_thread(): get_tid_from_pthread_descriptor: " + "bad offset:\n Run: DMTCP:util/check-pthread-tid-offset.c\n", + getpid()); + libc_abort(); + } - // We don't want to count the template thread nor - // the checkpoint thread, but these will appear in - // `/proc/self/tasks` - thread_count -= 2; + // We don't want to count the template thread itself, the checkpoint + // thread, or TSan-internal threads (e.g. libtsan's background thread, + // which blocks all signals at creation and never calls into libmcmini's + // wrappers, so it will never post to `dmtcp_restart_sem` below). + int self_tid_seen = 0; + int ckpt_tid_seen = 0; + while ((entry = readdir(dp))) { + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { + continue; + } + pid_t tid = (pid_t)atoi(entry->d_name); + if (tid == self_tid || tid == ckpt_tid) { + if (tid == self_tid) { + self_tid_seen++; + } + if (tid == ckpt_tid) { + ckpt_tid_seen++; + } + continue; + } + if (thread_blocks_signal(tid, SIG_MULTITHREADED_FORK)) { + log_debug("Excluding TSan-internal thread %d from the restart barrier\n", tid); + continue; + } + thread_count++; + } closedir(dp); + + // If either tid was not seen exactly once in /proc/self/task, the + // classification above is unreliable: the checkpoint thread's (or, in + // principle, the template thread's) real entry may have fallen through + // into the countable branch, silently corrupting `thread_count` and + // hanging the barrier loop below forever with no diagnostic. Abort loudly + // instead. + if (self_tid_seen != 1 || ckpt_tid_seen != 1) { + fprintf(stderr, + "PID %d: template_thread(): tid sanity check failed while walking " + "/proc/self/task: self_tid=%d seen %d time(s) (expected 1), " + "ckpt_tid=%d seen %d time(s) (expected 1)\n", + getpid(), self_tid, self_tid_seen, ckpt_tid, ckpt_tid_seen); + libc_abort(); + } + log_debug( "There are %d threads... waiting for them to get into a consistent " "state...\n", @@ -535,7 +661,15 @@ __attribute__((constructor)) void libmcmini_event_late_init() { pthread_attr_init(&attr); pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); libpthread_sem_init(&template_thread_sem, 0, 0); - libdmtcp_pthread_create(&template_thread_id, &attr, &template_thread, NULL); + // Create via the PUBLIC pthread_create (not libdmtcp_pthread_create) so that, + // when the target is instrumented, libtsan's pthread_create interceptor sees + // and registers this thread -- otherwise its (absent) TSAN ThreadState makes + // restart crash inside libtsan's setjmp/longjmp restore. mc_pthread_create + // recognizes the flag and still routes the actual creation through DMTCP + // without user-thread machinery. See TSAN-McMini-DMTCP.txt. + mc_creating_internal_thread = 1; + pthread_create(&template_thread_id, &attr, &template_thread, NULL); + mc_creating_internal_thread = 0; pthread_attr_destroy(&attr); } diff --git a/src/lib/interception.c b/src/lib/interception.c index 6d0b66f6..94fd4633 100644 --- a/src/lib/interception.c +++ b/src/lib/interception.c @@ -16,6 +16,8 @@ typeof(&pthread_create) libpthread_pthread_create_ptr; typeof(&pthread_create) libdmtcp_pthread_create_ptr; typeof(&pthread_join) libpthread_pthread_join_ptr; typeof(&pthread_join) libdmtcp_pthread_join_ptr; +typeof(&pthread_timedjoin_np) libpthread_timedjoin_np_ptr; +__attribute__((__noreturn__)) typeof(&pthread_exit) libpthread_pthread_exit_ptr; typeof(&pthread_mutex_init) pthread_mutex_init_ptr; typeof(&pthread_mutex_lock) pthread_mutex_lock_ptr; typeof(&pthread_mutex_trylock) pthread_mutex_trylock_ptr; @@ -69,6 +71,8 @@ void mc_load_intercepted_pthread_functions(void) { libpthread_pthread_create_ptr = dlsym(libpthread_handle, "pthread_create"); libpthread_pthread_join_ptr = dlsym(libpthread_handle, "pthread_join"); + libpthread_timedjoin_np_ptr = dlsym(libpthread_handle, "pthread_timedjoin_np"); + libpthread_pthread_exit_ptr = dlsym(libpthread_handle, "pthread_exit"); pthread_mutex_init_ptr = dlsym(libpthread_handle, "pthread_mutex_init"); pthread_mutex_lock_ptr = dlsym(libpthread_handle, "pthread_mutex_lock"); pthread_mutex_trylock_ptr = dlsym(libpthread_handle, "pthread_mutex_trylock"); @@ -228,11 +232,24 @@ int libpthread_pthread_join(pthread_t thread, void **rv) { libmcmini_init(); return (*libpthread_pthread_join_ptr)(thread, rv); } +int libpthread_timedjoin_np(pthread_t thread, void **rv, + const struct timespec *abstime) { + libmcmini_init(); + return (*libpthread_timedjoin_np_ptr)(thread, rv, abstime); +} int libdmtcp_pthread_join(pthread_t thread, void **rv) { libmcmini_init(); return (*libdmtcp_pthread_join_ptr)(thread, rv); } +MCMINI_NO_RETURN void pthread_exit(void *retval) { + mc_pthread_exit(retval); +} +MCMINI_NO_RETURN void libpthread_pthread_exit(void *retval) { + libmcmini_init(); + (*libpthread_pthread_exit_ptr)(retval); +} + void exit(int status) { mc_transparent_exit(status); } diff --git a/src/lib/record.c b/src/lib/record.c index 2023f7cc..0074b539 100644 --- a/src/lib/record.c +++ b/src/lib/record.c @@ -1,3 +1,4 @@ +#include "mcmini/mem.h" #include "mcmini/spy/checkpointing/record.h" #include "mcmini/spy/checkpointing/rec_list.h" #include "mcmini/spy/checkpointing/objects.h" @@ -78,6 +79,24 @@ rec_list *add_rec_entry_record_mode(const visible_object *vo) { return new_node; } +// TSan-safe variant: identical to add_rec_entry_record_mode but backed by +// mc_ts_alloc (no malloc), for use before libtsan has registered the calling +// thread. `new_node->vo = *vo` compiles to inline stores (verified: no memcpy +// libcall), so this whole function stays free of TSan interceptors. +rec_list *add_rec_entry_record_mode_ts(const visible_object *vo) { + rec_list *new_node = (rec_list *)mc_ts_alloc(sizeof(rec_list)); + new_node->vo = *vo; + new_node->next = NULL; + if (head_record_mode == NULL) { + head_record_mode = new_node; + current_record_mode = new_node; + } else { + current_record_mode->next = new_node; + current_record_mode = new_node; + } + return new_node; +} + //debugging puropses, will remove later // void print_rec_list(const rec_list *head) { // const rec_list *current = head; diff --git a/src/lib/tsan_support.c b/src/lib/tsan_support.c new file mode 100644 index 00000000..380e930e --- /dev/null +++ b/src/lib/tsan_support.c @@ -0,0 +1,28 @@ +#include "mcmini/spy/checkpointing/tsan_support.h" + +#include + +int thread_blocks_signal(pid_t tid, int signo) { + char path[64]; + snprintf(path, sizeof(path), "/proc/self/task/%d/status", (int)tid); + FILE *f = fopen(path, "r"); + if (f == NULL) { + return 0; + } + + char line[256]; + unsigned long long sigblk = 0; + int found = 0; + while (fgets(line, sizeof(line), f)) { + if (sscanf(line, "SigBlk: %llx", &sigblk) == 1) { + found = 1; + break; + } + } + fclose(f); + + if (!found) { + return 0; + } + return (int)((sigblk >> (signo - 1)) & 1ULL); +} diff --git a/src/lib/wrappers.c b/src/lib/wrappers.c index a385c012..594bfd4a 100644 --- a/src/lib/wrappers.c +++ b/src/lib/wrappers.c @@ -21,35 +21,50 @@ typedef struct pthread_map { struct pthread_map *next; } pthread_map_t; -static pthread_rwlock_t pthread_map_lock = PTHREAD_RWLOCK_INITIALIZER; +// NOTE: This lock and allocator must be TSan-safe: insert_pthread_map runs from +// mc_thread_routine_wrapper's prologue, which under DMTCP executes before +// libtsan has registered the thread. So we use libmcmini's libpthread_* handle +// wrappers (which bypass libtsan) rather than the raw pthread_rwlock_* symbols +// (which resolve to libtsan's interceptors), and mc_ts_alloc rather than malloc. +// See TSAN-McMini-DMTCP.txt. +static pthread_mutex_t pthread_map_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_map_t *head = NULL; void insert_pthread_map(pthread_t t, runner_id_t v) { - pthread_rwlock_wrlock(&pthread_map_lock); - pthread_map_t *n = malloc(sizeof *n); + libpthread_mutex_lock(&pthread_map_lock); + pthread_map_t *n = mc_ts_alloc(sizeof *n); n->thread = t; n->value = v; n->next = head; head = n; - pthread_rwlock_unlock(&pthread_map_lock); + libpthread_mutex_unlock(&pthread_map_lock); } runner_id_t search_pthread_map(pthread_t t) { - pthread_rwlock_rdlock(&pthread_map_lock); - pthread_map_t *cur = head; - while (cur) { + libpthread_mutex_lock(&pthread_map_lock); + runner_id_t result = RID_INVALID; + for (pthread_map_t *cur = head; cur != NULL; cur = cur->next) { if (pthread_equal(cur->thread, t)) { - return cur->value; + result = cur->value; + break; } - cur = cur->next; } - pthread_rwlock_unlock(&pthread_map_lock); - return RID_INVALID; + libpthread_mutex_unlock(&pthread_map_lock); + return result; } MCMINI_THREAD_LOCAL runner_id_t tid_self = RID_INVALID; +// Set (on the creating thread) while libmcmini creates one of its OWN helper +// threads (e.g. the template thread) via the public pthread_create. It tells +// mc_pthread_create to create the thread plainly -- routed through DMTCP and +// visible to any sanitizer's pthread_create interceptor -- rather than treating +// it as a user thread to be model-checked. Keeping it visible to +// ThreadSanitizer is what lets its ThreadState round-trip checkpoint/restart. +// See TSAN-McMini-DMTCP.txt. +MCMINI_THREAD_LOCAL int mc_creating_internal_thread = 0; + runner_id_t mc_register_this_thread(void) { static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER; static runner_id_t tid_next = 0; @@ -450,6 +465,35 @@ MCMINI_NO_RETURN void mc_transparent_abort(void) { } } +MCMINI_NO_RETURN void mc_pthread_exit(void *retval) { + switch (get_current_mode()) { + case PRE_DMTCP_INIT: + case PRE_CHECKPOINT_THREAD: + case CHECKPOINT_THREAD: + case RECORD: + case PRE_CHECKPOINT: { + libpthread_pthread_exit(retval); + } + case DMTCP_RESTART_INTO_BRANCH: + case DMTCP_RESTART_INTO_TEMPLATE: { + thread_get_mailbox()->type = THREAD_EXIT_TYPE; + thread_handle_after_dmtcp_restart(); + // Fallthrough + } + case TARGET_BRANCH: + case TARGET_BRANCH_AFTER_RESTART: { + if (tid_self == RID_MAIN_THREAD) { + mc_exit_main_thread_in_child(); + } else { + mc_exit_thread_in_child(); + } + } + default: { + libc_abort(); + } + } +} + struct mc_thread_routine_arg { void *arg; thread_routine routine; @@ -491,7 +535,9 @@ void *mc_thread_routine_wrapper(void *arg) { .thrd_state.pthread_desc = this_thread, .thrd_state.status = ALIVE, .thrd_state.id = rid}; - thread_record = add_rec_entry_record_mode(&vo); + // TSan-safe allocation: under DMTCP this runs before libtsan has + // registered the thread (see TSAN-McMini-DMTCP.txt). + thread_record = add_rec_entry_record_mode_ts(&vo); libpthread_mutex_unlock(&rec_list_lock); libpthread_sem_post(&unwrapped_arg->mc_pthread_create_binary_sem); break; @@ -591,6 +637,17 @@ int mc_pthread_create(pthread_t *thread, const pthread_attr_t *attr, // and creates no other threads during execution static pthread_once_t main_thread_once = PTHREAD_ONCE_INIT; + // libmcmini's own helper thread (e.g. the template thread). It reached us + // through the sanitizer's pthread_create interceptor (if any) -- so TSAN has + // already registered it and wrapped `routine` -- and we now just hand it to + // DMTCP so it is DMTCP-known, with no user-thread machinery. Creating it via + // the public pthread_create (rather than libdmtcp_pthread_create directly) + // is deliberate: it keeps the thread visible to libtsan, so its ThreadState + // survives checkpoint/restart. See TSAN-McMini-DMTCP.txt. + if (mc_creating_internal_thread) { + return libdmtcp_pthread_create(thread, attr, routine, arg); + } + // TODO: Reduce code duplication here! switch (get_current_mode()) { case PRE_DMTCP_INIT: { @@ -726,7 +783,10 @@ int mc_pthread_join(pthread_t t, void **rv) { struct timespec time = {.tv_sec = 2, .tv_nsec = 0}; while (1) { - int rc = pthread_timedjoin_np(t, rv, &time); + // Use the libtsan-bypassing handle: a direct pthread_timedjoin_np would + // hit libtsan's interceptor and trip its thread-registry CHECK under + // DMTCP. See TSAN-McMini-DMTCP.txt. + int rc = libpthread_timedjoin_np(t, rv, &time); if (rc == 0) { // Join succeeded libpthread_mutex_lock(&rec_list_lock); thread_record->vo.thrd_state.status = EXITED; diff --git a/test/tsan_support/test_fastpath_fork_clone_fiber.c b/test/tsan_support/test_fastpath_fork_clone_fiber.c new file mode 100644 index 00000000..bed2f661 --- /dev/null +++ b/test/tsan_support/test_fastpath_fork_clone_fiber.c @@ -0,0 +1,253 @@ +// Standalone host-side test harness. Not an mcmini model-checking target. +// +// Mirrors dmtcp-callback.c's ACTUAL fast-path resumption mechanism (each +// thread calls getcontext() directly, as a plain function call -- no signal +// handler, unlike the multithreaded-fork-tsan-2.0 standalone package) to +// exercise R2 (TSan fork hooks around _Fork()), R3 (__clone instead of the +// public clone()), and R4 (fresh TSan fiber for the forking thread AND each +// recreated thread) under real ThreadSanitizer. +// +// What this harness's RED/GREEN evidence actually proves: R3. The buggy +// build reliably reproduces a real "ThreadSanitizer: CHECK failed" in +// ForkChildAfter (public clone() intercepted by libtsan and mishandled for +// a CLONE_THREAD clone), and the fixed build (using __clone) passes +// cleanly -- this is genuine falsification/confirmation evidence. +// +// The forking-thread fiber switch (the R4 remainder, as opposed to the +// recreated-thread fiber switch, which was already proven separately) is +// included here for structural completeness and executes in the default +// (fixed) build (compiled out in the buggy build alongside R3), but it is +// NOT independently validated +// by this harness: the forking thread is just main(), which only does a +// handful of sem_wait/sem_post/fprintf calls after the fork before +// _exit(), never enough post-fork instrumented work to trigger the +// shadow-call-stack overflow that fix guards against (per the vendor +// package's README, that failure "manifests as this thread keeps +// running"). Removing that fiber switch from dmtcp-callback.c would not +// turn this harness red. Its correctness rests on direct analogy to the +// already-proven multithreaded-fork-tsan-2.0 recipe, not on evidence from +// this harness. +// +// BUGGY MODE (for RED evidence): compile with -DMTF_BUGGY to use the public +// clone() (no R3) and skip the forking-thread fiber switch (no R4 remainder), +// reproducing the failure these two fixes exist to prevent. Only the R3 +// half of this toggle is known to actually be exercised by the resulting +// pass/fail outcome; see above. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef __x86_64__ +#include +#include +#else +#error "This standalone harness only covers __x86_64__; see dmtcp-callback.c for other architectures." +#endif + +#ifndef NUM_WORKERS +#define NUM_WORKERS 3 +#endif +#define ITERS 100000 + +// ---- TSan externs, copied verbatim from src/lib/dmtcp-callback.c ---- +extern void *__tsan_create_fiber(unsigned flags) __attribute__((weak)); +extern void __tsan_switch_to_fiber(void *fiber, unsigned flags) + __attribute__((weak)); +extern void __sanitizer_syscall_pre_impl_fork(void) __attribute__((weak)); +extern void __sanitizer_syscall_post_impl_fork(long res) __attribute__((weak)); + +// libc's internal clone (NOT intercepted by libtsan, unlike public clone()). +extern int __clone(int (*fn)(void *), void *child_stack, int flags, void *arg, + ... /* pid_t *ptid, void *newtls, pid_t *ctid */); + +// ---- struct threadinfo + TLS/descriptor helpers, copied verbatim from +// src/lib/dmtcp-callback.c (x86_64 branch only) for fidelity to production, +// including the "syscall(SYS_arch_prctl, 2, ARCH_SET_FS, ...)" call already +// proven correct by the multithreaded-fork-tsan-2.0 standalone package. ---- +struct threadinfo { + ucontext_t context; + unsigned long fs; + unsigned long gs; + pthread_t pthread_descriptor; +}; +static struct threadinfo threadInfos[NUM_WORKERS]; +static atomic_int threadIdx = 0; + +static void getTLSPointer(struct threadinfo *ti) { + assert(syscall(SYS_arch_prctl, ARCH_GET_FS, &ti->fs) == 0); + assert(syscall(SYS_arch_prctl, ARCH_GET_GS, &ti->gs) == 0); +} +static void setTLSPointer(struct threadinfo *ti) { + assert(syscall(SYS_arch_prctl, 2, ARCH_SET_FS, ti->fs) != 0); + assert(syscall(SYS_arch_prctl, 2, ARCH_SET_GS, ti->gs) != 0); +} +static int pthreadDescriptorTidOffset(void) { return 720; } +static pid_t patchThreadDescriptor(pthread_t pthreadSelf) { + int offset = pthreadDescriptorTidOffset(); + pid_t oldtid = *(pid_t *)((char *)pthreadSelf + offset); + *(pid_t *)((char *)pthreadSelf + offset) = syscall(SYS_gettid); + return oldtid; +} + +// ---- Harness state ---- +static pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER; +static long shared_counter = 0; +static sem_t sem_checkin, sem_release, sem_done, sem_park; + +static void sem_wait_retry(sem_t *s) { + while (sem_wait(s) != 0) /* EINTR */; +} + +// ---- child_setcontext_fast(), copied verbatim in structure from +// src/lib/dmtcp-callback.c ---- +static int child_setcontext_fast(void *arg) { + struct threadinfo *ti = arg; + setTLSPointer(ti); + patchThreadDescriptor(ti->pthread_descriptor); + setcontext(&ti->context); // never returns + return 0; +} + +static void restart_child_threads_fast(void) { + int maxThreadIdx = atomic_load(&threadIdx); + for (int i = 0; i < maxThreadIdx; i++) { + int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM + | CLONE_SIGHAND | CLONE_THREAD + | CLONE_SETTLS | CLONE_PARENT_SETTID + | CLONE_CHILD_CLEARTID); + void *stack = malloc(0x10000) + 0x10000 - 128; // 64 KB, intentionally leaked + int offset = pthreadDescriptorTidOffset(); + pid_t *ctid = (pid_t *)((char *)threadInfos[i].pthread_descriptor + offset); + pid_t *ptid = ctid; +#ifdef MTF_BUGGY + // R3 NOT applied: public clone() is intercepted by libtsan and treated + // as a fork, corrupting its thread-slot state for this CLONE_THREAD clone. + clone(child_setcontext_fast, stack, clone_flags, + (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); +#else + // R3: libc's raw __clone, not intercepted by libtsan. + __clone(child_setcontext_fast, stack, clone_flags, + (void *)&threadInfos[i], ptid, threadInfos[i].fs, ctid); +#endif + } +} + +// ---- fast_multithreaded_fork(), copied in structure from +// src/lib/dmtcp-callback.c ---- +static pid_t fast_multithreaded_fork(void) { + pid_t _Fork(); + if (__sanitizer_syscall_pre_impl_fork != NULL) { + __sanitizer_syscall_pre_impl_fork(); + } + int childpid = _Fork(); + if (__sanitizer_syscall_post_impl_fork != NULL) { + __sanitizer_syscall_post_impl_fork(childpid); + } + if (childpid == 0) { // child process +#ifndef MTF_BUGGY + // R4 remainder: the forking thread keeps its inherited (fork-copied) TSan + // ThreadState, whose shadow call stack starts at the parent's fork-time + // depth and can overflow as this thread keeps running. Fresh fiber too. + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } +#endif + restart_child_threads_fast(); + } + return childpid; +} + +// ---- worker thread: mirrors thread_handle_after_dmtcp_restart()'s +// getcontext-direct-call mechanism (no signal handler). ---- +static void *worker(void *arg) { + (void)arg; + pid_t orig_pid = getpid(); + int idx = atomic_fetch_add(&threadIdx, 1); + struct threadinfo *ti = &threadInfos[idx]; + memset(ti, 0, sizeof(*ti)); + ti->pthread_descriptor = pthread_self(); + getTLSPointer(ti); + + int rc = getcontext(&ti->context); + assert(rc == 0); + + if (getpid() == orig_pid) { + // Still in the original process (pre-fork): check in, then block until + // released (mirrors production's cond_wait parking; a plain semaphore is + // sufficient here since R1's mode-machinery is out of scope for R2/R3/R4). + sem_post(&sem_checkin); + sem_wait_retry(&sem_release); + } else { + // Resumed via __clone()+setcontext in the forked child: give this + // TSan-invisible OS thread a valid ThreadState before any instrumented + // call below. (Already-proven R4 half; unchanged by MTF_BUGGY.) + if (__tsan_switch_to_fiber != NULL) { + __tsan_switch_to_fiber(__tsan_create_fiber(0), 0); + } + } + + // TSan-intercepted work: locked shared write, exercised both by the + // original threads (parent) and the recreated threads (child). + for (int i = 0; i < ITERS; i++) { + pthread_mutex_lock(&mtx); + shared_counter++; + pthread_mutex_unlock(&mtx); + } + sem_post(&sem_done); + sem_wait_retry(&sem_park); // park forever; R5 join/exit shims out of scope + return NULL; +} + +int main(void) { + sem_init(&sem_checkin, 0, 0); + sem_init(&sem_release, 0, 0); + sem_init(&sem_done, 0, 0); + sem_init(&sem_park, 0, 0); + + pthread_t th[NUM_WORKERS]; + for (int i = 0; i < NUM_WORKERS; i++) { + pthread_create(&th[i], NULL, worker, NULL); + } + for (int i = 0; i < NUM_WORKERS; i++) { + sem_wait(&sem_checkin); + } + + fprintf(stderr, "[main pid=%d] all workers checked in; forking...\n", getpid()); + pid_t pid = fast_multithreaded_fork(); + const char *who = (pid == 0) ? "CHILD" : "PARENT"; + fprintf(stderr, "[%s pid=%d] returned from fast_multithreaded_fork\n", who, getpid()); + + if (pid > 0) { + // PARENT: release the still-parked original workers. + for (int i = 0; i < NUM_WORKERS; i++) { + sem_post(&sem_release); + } + } + // CHILD: recreated threads proceed on their own (see worker()'s else branch). + + for (int i = 0; i < NUM_WORKERS; i++) { + sem_wait(&sem_done); + } + fprintf(stderr, "[%s pid=%d] shared_counter=%ld (expected %d)\n", + who, getpid(), shared_counter, NUM_WORKERS * ITERS); + + if (pid > 0) { + int status; + waitpid(pid, &status, 0); + fprintf(stderr, "[PARENT] child: exited=%d code=%d signaled=%d\n", + WIFEXITED(status), WEXITSTATUS(status), WIFSIGNALED(status)); + } + fprintf(stderr, "[%s pid=%d] done, _exit(0)\n", who, getpid()); + _exit(0); +} diff --git a/test/tsan_support/test_pthread_exit_forwarding.c b/test/tsan_support/test_pthread_exit_forwarding.c new file mode 100644 index 00000000..25c257cf --- /dev/null +++ b/test/tsan_support/test_pthread_exit_forwarding.c @@ -0,0 +1,45 @@ +// Standalone host-side unit test. Not an mcmini model-checking target. +// +// Proves the dlopen+dlsym technique Task 2 uses to add a cached "real +// pthread_exit" handle to src/lib/interception.c (matching the existing +// libpthread_pthread_join_ptr/libpthread_timedjoin_np_ptr pattern): resolve +// pthread_exit from a freshly dlopen'd libpthread.so/libpthread.so.0, call +// it from a thread, and confirm the retval reaches a real pthread_join(). +#define _GNU_SOURCE +#include +#include +#include +#include + +typedef void (*pthread_exit_fn)(void *); + +static pthread_exit_fn real_pthread_exit; + +static void *worker(void *arg) { + (void)arg; + real_pthread_exit((void *)(long)42); + return NULL; // not reached +} + +int main(void) { + void *libpthread_handle = dlopen("libpthread.so", RTLD_LAZY); + if (!libpthread_handle) { + libpthread_handle = dlopen("libpthread.so.0", RTLD_LAZY); + } + assert(libpthread_handle != NULL); + + real_pthread_exit = (pthread_exit_fn)dlsym(libpthread_handle, "pthread_exit"); + assert(real_pthread_exit != NULL); + + pthread_t t; + int rc = pthread_create(&t, NULL, worker, NULL); + assert(rc == 0); + + void *retval = NULL; + rc = pthread_join(t, &retval); + assert(rc == 0); + assert((long)retval == 42); + + printf("PASS\n"); + return 0; +} diff --git a/test/tsan_support/test_thread_blocks_signal.c b/test/tsan_support/test_thread_blocks_signal.c new file mode 100644 index 00000000..b9e19adc --- /dev/null +++ b/test/tsan_support/test_thread_blocks_signal.c @@ -0,0 +1,60 @@ +// Standalone host-side unit test for thread_blocks_signal(). Not an mcmini +// model-checking target: compile and run directly (see the command in the +// implementation plan / commit message), not through CMake. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mcmini/spy/checkpointing/tsan_support.h" + +static sem_t ready; +static pid_t worker_tid; + +static void *worker(void *arg) { + (void)arg; + worker_tid = (pid_t)syscall(SYS_gettid); + + sigset_t set; + sigemptyset(&set); + sigaddset(&set, SIGUSR1); + pthread_sigmask(SIG_BLOCK, &set, NULL); + + sem_post(&ready); + + for (;;) { + pause(); // cancellation point; SIGUSR1 stays blocked the whole time + } + return NULL; +} + +int main(void) { + int rc = sem_init(&ready, 0, 0); + assert(rc == 0); + + pthread_t t; + rc = pthread_create(&t, NULL, worker, NULL); + assert(rc == 0); + + rc = sem_wait(&ready); + assert(rc == 0); + + pid_t self_tid = (pid_t)syscall(SYS_gettid); + + assert(thread_blocks_signal(worker_tid, SIGUSR1) == 1); + assert(thread_blocks_signal(self_tid, SIGUSR1) == 0); + assert(thread_blocks_signal(999999 /* bogus tid, should not exist */, SIGUSR1) == 0); + + rc = pthread_cancel(t); + assert(rc == 0); + rc = pthread_join(t, NULL); + assert(rc == 0); + + printf("PASS\n"); + return 0; +} diff --git a/test/tsan_support/test_tid_from_descriptor_offset.c b/test/tsan_support/test_tid_from_descriptor_offset.c new file mode 100644 index 00000000..6708312c --- /dev/null +++ b/test/tsan_support/test_tid_from_descriptor_offset.c @@ -0,0 +1,67 @@ +// Standalone host-side unit test. Not an mcmini model-checking target. +// +// Independently re-verifies the x86_64 pthread_t -> tid offset that +// src/lib/dmtcp-callback.c's pthreadDescriptorTidOffset() uses, specifically +// for READING (not patching) another thread's descriptor -- the new use case +// get_tid_from_pthread_descriptor() introduces. The existing +// patchThreadDescriptor() already self-verifies the same offset for +// `pthread_self()` on every restart (see saveThreadStateBeforeFork()); this +// test covers the "another thread's descriptor" case that path never +// exercises. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +#ifndef __x86_64__ +#error "This standalone test only covers __x86_64__; see dmtcp-callback.c's pthreadDescriptorTidOffset() for other architectures." +#endif + +static pid_t tid_from_descriptor(pthread_t descriptor) { + const int offset = 720; // matches pthreadDescriptorTidOffset() for __x86_64__ + return *(pid_t *)((char *)descriptor + offset); +} + +static sem_t ready; +static pthread_t worker_self; +static pid_t worker_tid; + +static void *worker(void *arg) { + (void)arg; + worker_self = pthread_self(); + worker_tid = (pid_t)syscall(SYS_gettid); + sem_post(&ready); + for (;;) { + pause(); + } + return NULL; +} + +int main(void) { + int rc = sem_init(&ready, 0, 0); + assert(rc == 0); + + pthread_t t; + rc = pthread_create(&t, NULL, worker, NULL); + assert(rc == 0); + + rc = sem_wait(&ready); + assert(rc == 0); + + // Read another thread's tid from its descriptor without mutating it. + assert(tid_from_descriptor(worker_self) == worker_tid); + // Reading twice must be idempotent (unlike patchThreadDescriptor()). + assert(tid_from_descriptor(worker_self) == worker_tid); + + rc = pthread_cancel(t); + assert(rc == 0); + rc = pthread_join(t, NULL); + assert(rc == 0); + + printf("PASS\n"); + return 0; +}