DeepDebug: McMini, DMTCP and TSAN#11
Draft
gc00 wants to merge 36 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TSAN's pthread_join() interceptor delegates to a genuine OS-level join and blocks via the kernel until the target thread actually dies -- it does not rely on its own creation-time bookkeeping. But mc_pthread_join()'s TARGET_BRANCH case only simulated success at the model level, while the joined thread was kept parked in thread_block_indefinitely() forever, so a real join on it (e.g. from TSan) could never complete. Give each thread its own exit_permission_sem (alongside its existing pthread_map entry). A finishing thread waits on it before returning; mc_pthread_join() posts it and performs a real libpthread_pthread_join() before returning, and as a bonus, pthread_join returns a return value.
classic_dpor::verify_using()'s forward-exploration path catches real_world::process::termination_error and reports it via the abnormal_termination callback, letting the run end cleanly. The backtrack-replay path (coordinator::return_to_depth(), which replays prior transitions against a freshly restarted process) had no such handling, so the same exception there escaped all the way to the top-level catch-all instead. Wrap return_to_depth() the same way. found_abnormal_termination() also needed a null check: return_to_depth()'s target thread may have no pending transition in the model's current view (unlike the forward path, where the culprit is always the runner DPOR just selected as enabled). The report then falls back to a plain "no longer pending" line instead of dereferencing a null transition.
The TSAN-supporting DMTCP branch (tsan-phased-init) bumped the plugin API
from v3 to v4, an ABI change (DmtcpPluginDescriptor_t / DmtcpUniqueProcessId,
new DmtcpCkptHeader etc.). DMTCP refused to load libmcmini.so:
ASSERT pluginmanager.cpp:228: incompatible DMTCP plugin API version:
plugin_api=3 expected=4
Sync the vendored include/dmtcp.h to DMTCP's v4 header (correct version
string and descriptor ABI), and carry forward the only McMini-specific
additions -- the mcmini_virtual_pid / mcmini_real_pid macros -- updated to
the v4 function names (dmtcp_{real_to_virtual,virtual_to_real}_pid became
dmtcp_pid_{real_to_virtual,virtual_to_real}). Also update the two direct
callers in multithreaded_fork.c. The unused dmtcp_restore_buf_* decls are
dropped (not referenced by libmcmini, and gone from v4).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
template_thread() computed the number of threads to wait for by scanning /proc/self/task live, at restart time, then subtracting a blanket 2 (for itself and the checkpoint thread). DMTCP recreates checkpointed threads asynchronously via clone(), so a scan that runs before it has finished recreating all of them undercounts -- this barrier then declares "consistent state" and lets the template thread proceed before every thread has actually restarted. Confirmed via added diagnostic logging: one thread's own restart-completion signal could arrive after the barrier already released. Fix: count ALIVE THREAD entries in head_record_mode instead. That list only ever gets entries for genuine target threads (the template thread and checkpoint thread never go through libmcmini's wrapped pthread_create(), so neither is ever recorded there), and since a DMTCP checkpoint is a full memory snapshot, it's preserved exactly as-is across every restart -- immune to any restart-time scheduling race.
…e=thread Off by default. Experiment to see whether instrumenting libmcmini.so itself (as opposed to only the target) changes TSan-registration-order crashes; empirically it did not move the crash site, so the normal path stays uninstrumented per PLAN.txt's suppressions-file design.
getcontext()/setcontext() already restore a thread's blocked-signal set via uc_sigmask, even across the raw clone() used to recreate a checkpointed thread. The thread_sigmask field and abort were unnecessary; both are removed. This check ran on every restarted thread regardless of mode, so it also blocked plain (non-multithreaded-fork) restart of any target with a blocked-signal thread.
When a -fsanitize=thread target runs under DMTCP, dmtcp_launch prepends
libtsan ahead of libmcmini, so a new thread's entry order is
dmtcp thread_start -> libmcmini's mc_thread_routine_wrapper -> libtsan
trampoline -> user routine.
libmcmini's wrapper thus runs BEFORE libtsan has registered the thread.
Its prologue then called libc functions that libtsan intercepts (malloc,
and the raw pthread_rwlock_* in insert_pthread_map), and those interceptors
dereference the unregistered thread's null ThreadState -> SEGV / TSan
"sanitizer_thread_registry.cpp:348" CHECK. (Full analysis in
TSAN-McMini-DMTCP.txt.)
Make the prologue free of TSan-intercepted libc calls:
- Add mc_ts_alloc (mem.c/mem.h): a bump allocator over a static BSS arena.
No libc call and no syscall on the fast path (only an atomic bump), so
it never enters a TSan interceptor. Fail path uses raw syscalls only.
Never frees (the pthread_map / rec_list nodes it backs are never freed).
- insert_pthread_map / search_pthread_map: use libmcmini's libpthread_*
handle wrappers (which bypass libtsan) instead of the raw pthread_rwlock_*
symbols, and mc_ts_alloc instead of malloc. Also fixes a pre-existing
missing-unlock bug in search_pthread_map's found path.
- add_rec_entry_record_mode_ts (record.c): mc_ts_alloc-backed variant of
add_rec_entry_record_mode; the prologue's THREAD-record insert uses it.
Verified: mcmini -i 3 on ~/dmtcp.git/test/tsan_target (DMTCP tsan-phased-init,
plugin API v4) no longer crashes in the thread-creation prologue; both worker
threads now run under RECORD mode.
Scope: this fixes the prologue only. A separate TSan-interception site remains
downstream -- mc_pthread_join calls pthread_timedjoin_np directly (no bypass
handle), tripping libtsan's ConsumeThreadUserId CHECK -- to be addressed next.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mc_pthread_join's RECORD loop called pthread_timedjoin_np directly. Under a TSAN target, that symbol resolves to libtsan's interceptor, whose ConsumeThreadUserId trips a thread-registry CHECK (sanitizer_thread_registry.cpp:348) and aborts. Add a libpthread_timedjoin_np handle (dlsym'd from libpthread, like the mutex/cond/sem wrappers) that bypasses libtsan, and call it from mc_pthread_join's RECORD loop instead of the raw symbol. With this, the three-fix stack achieves END-TO-END checkpointing of a -fsanitize=thread target under deep-debug (mcmini record mode): 1. 5be8500 DMTCP plugin API v3 -> v4 (DMTCP loads libmcmini.so) 2. 4bf2720 TSan-safe RECORD prologue (worker creation doesn't crash before libtsan registers the thread) 3. this timed-join via bypass handle (join loop doesn't trip the registry CHECK) Verified: `mcmini -i 3 ~/dmtcp.git/test/tsan_target` (DMTCP tsan-phased-init) now runs with no SEGV / no ThreadSanitizer errors and DMTCP produces a valid checkpoint (ckpt_tsan_target_*.dmtcp, ~5 MB, matching the no-mcmini baseline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
libmcmini's template thread was created via libdmtcp_pthread_create, bypassing libtsan's pthread_create interceptor. When the target is built with -fsanitize=thread, that left the template thread with no libtsan ThreadState. On DMTCP restart, libtsan's setjmp/longjmp restore (reached from threadlist.cpp:stopthisthread) dereferences the thread's absent ThreadState and crashes with SIGSEGV during thread restore. Create the template thread through the public pthread_create instead, so libtsan's interceptor registers it and wraps its start routine. A new thread-local flag mc_creating_internal_thread tells mc_pthread_create to skip the user-thread/model-checking machinery and still route the actual creation through DMTCP (libdmtcp_pthread_create), keeping the thread DMTCP-known. When the target is not instrumented, the public pthread_create resolves straight to mc_pthread_create and behavior is unchanged. With this, record -> checkpoint -> restart of a TSAN target survives thread restore (verified: raw dmtcp_restart restores all threads with no SIGSEGV, where it previously crashed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the throwaway SIGSEGV-backtrace handler and its install call
sites added during TSan-restart investigation, and restore the
DMTCP_EVENT_RESTART SIG_DFL reset for SIGSEGV. Keep the R2 (TSan fork
syscall hooks around _Fork()) and R4 (fresh TSan fiber for recreated
threads) prototypes as the starting point for the multithreaded_fork
TSan port described in PLAN.txt.
This is an unfinished prototype on this branch: R3 (__clone) and the
R4 remainder were never applied here, and none of this code has been
exercised by a --multithreaded-fork run under TSan in this branch's
history.
The completed version lives on branch recreated-thread-sigmask (also
reachable as an ancestor from tsan-checkpoint-restart), continuing
with:
- Add design spec for TSan port Phase 2 (R2+R3+R4)
- Add implementation plan for TSan port Phase 2 (R2+R3+R4)
- Add standalone harness proving R3 (__clone) + R4-remainder
(forker fiber) under TSan
- Apply R3 (__clone) and R4 remainder (forker fiber) to
fast_multithreaded_fork
- Correct overstated R4-remainder claim in fastpath fork/clone/fiber
harness comment
- Fix cosmetic inaccuracy in harness comment re: forking-thread
fiber switch
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implements a helper function to detect if a thread has a signal blocked via its /proc/self/task/<tid>/status SigBlk mask. This will be consumed by Task 2 to identify and skip ThreadSanitizer's internal background thread during DMTCP checkpoint restart. - Add include/mcmini/spy/checkpointing/tsan_support.h with function declaration - Add src/lib/tsan_support.c with implementation - Add test/tsan_support/test_thread_blocks_signal.c standalone unit test - Wire new source file into CMakeLists.txt LIBMCMINI_C_SRC list Test verified: compiles with -Wall -Werror, test passes (exit code 0). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Under a TSan-instrumented target, TSan's own background thread is spawned as a nested pthread_create() during the same window in which DMTCP creates its checkpoint thread. Both requests reach libmcmini's dmtcp_create_checkpoint_thread_wrapper(), which previously had no way to tell them apart at creation time. The real checkpoint thread could end up misclassified as an ordinary user thread and get recorded into the target's rec_list, corrupting the recording and causing "Expected a callback for 0" on restart. dmtcp_is_tsan_background_thread() resolves this, but classification requires DMTCP's endCkptThreadCreationWindow(), which can't run until the new thread has been registered with TSan -- which only happens once dmtcp_create_checkpoint_thread_wrapper() calls its real routine. Blocking on classification before that call therefore deadlocks. Fix: mark the thread as an unresolved candidate and run its real routine immediately, then resolve classification lazily on this same thread's first subsequent wrapped call (get_current_mode()), by which point TSan registration is guaranteed to have already happened.
The mode previously meant only "the DMTCP checkpoint thread is calling in". ThreadSanitizer's own internal background thread calls into libmcmini.so's overridden functions too, on a thread that is likewise not part of the target program, so get_current_mode() now reports the same value for it. Renamed to reflect the broader meaning. Detection reuses thread_blocks_signal() (already used by the R1 restart-barrier fix) via two new tsan_support.c helpers: mc_real_tid(), which reads /proc/thread-self since DMTCP virtualizes gettid() (including via the raw syscall path); and mc_is_current_thread_tsan_internal(), a per-thread-cached wrapper around it. Isolating which of thread_blocks_signal()'s calls were themselves TSan-safe required bisecting its RECORD-mode-original fopen/fgets/ sscanf/fclose implementation one call at a time. Findings: raw syscalls for openat/read/readlink are safe from a thread TSan has not yet registered, but even a raw syscall(SYS_close, fd) is not, so the one fd this code opens is deliberately never closed (bounded to one leak per thread, since the result is cached for that thread's lifetime).
mc_is_current_thread_tsan_internal() is only needed in TARGET_BRANCH/ restart modes, but was also running during RECORD, where its first call per thread leaks an fd pointing at that thread's own /proc/<pid>/task/<tid>/status. That fd is open at checkpoint time, so DMTCP must checkpoint/restore a path whose pid/tid cannot exist after restart. Skipping the check during RECORD/PRE_CHECKPOINT empirically confirmed this is why template_thread() was failing to wake up after restart -- it now does. Not yet understood: WHY this causes that hang, i.e. whether DMTCP is failing to restore the fd correctly or something else is at fault. Keep this gate until that's diagnosed and a permanent fix (most likely: never opening the fd at all) replaces it.
The prior gate excluded RECORD/PRE_CHECKPOINT but missed PRE_DMTCP_INIT and PRE_CHECKPOINT_THREAD, so one fd (the calling thread's own /proc/<pid>/task/<tid>/status, opened by its very first call to mc_is_current_thread_tsan_internal()) was still leaking during those earlier modes and getting checkpointed. Switched from a negative exclusion list to a positive list of the four modes where the check actually matters, matching the exact case-label grouping every wrapper function already uses for this purpose. Confirmed via /proc/<pid>/fd census: zero leaked fds now, for the whole RECORD phase. But restarting from a checkpoint recorded this way does not by itself get further than before: template_thread() still hangs, now in a different, later place (the `dmtcp_restart_sem` count loop). So this fd was not, on its own, responsible for that specific hang -- still needs its own analysis.
A DMTCP_RESTART_INTO_BRANCH process explores exactly one trace, then gets discarded -- it never legitimately needs to checkpoint again. Left alone, the checkpoint thread resumes its normal sleep-checkpoint-resume loop and blocks forever waiting for a checkpoint request from this restart's one-shot, otherwise-idle coordinator (dmtcp_process_source spins up a fresh coordinator per branch). Call the new dmtcp_skip_post_restart_checkpoint_loop() to tell DMTCP not to resume that loop. Verified: the checkpoint thread now parks in pause() right after restart instead of hanging in read() waiting on the coordinator.
Under DMTCP, libtsan.so resolves ahead of libmcmini.so, so a target's own pthread_join() calls reach TSan's interceptor first. For a thread DMTCP resurrected via clone() (bypassing TSan's pthread_create() interceptor), TSan can never resolve its Tid and hangs forever -- see TSAN-pthread-join.md. Fix: link the target with -Wl,--wrap=pthread_join plus the new pthread_join_wrap.c, rewriting the target's own pthread_join() calls at link time so they reach mc_pthread_join_maybe_defer() before the dynamic linker (and TSan) ever sees them. A DMTCP-resurrected thread's join is handled entirely via a really_exited_sem handshake; any thread with a valid TSan Tid (classic mode, or created via pthread_create() after restart) is still really joined via __real_pthread_join(), so TSan keeps seeing and tracking that join normally.
mcmini_log() calls localtime() from any thread, unsynchronized. glibc's tzset_internal() (invoked on every localtime_r() call, not just the first) is not safe to run concurrently even when TZ never changes, so under TSan this showed up as a data race and, at least once, a subsequent SEGV. A real pthread mutex around the whole function isn't enough by itself: it's taken via libpthread_mutex_lock(), which resolves straight to libpthread's own symbol (see mc_load_intercepted_pthread_functions() in interception.c), bypassing TSan's interceptor entirely. So the lock genuinely serializes the calls, but TSan never sees it and keeps reporting a race. Fix that with TSan's own public annotation API (__tsan_acquire/__tsan_release, declared weak so this is a no-op without a TSan runtime in the process) around the same lock. Also switch localtime() to localtime_r() with a stack-local struct tm, and add tzset() as a constructor so the timezone database is loaded once, single-threaded, before any thread can race on it. Verified against a freshly-recorded DMTCP checkpoint (rebuilding libmcmini.so does not affect restarts of an already-recorded checkpoint, which pins whatever plugin build was loaded at record time): three consecutive restarts now show zero tzset_internal races, versus 3/3 racing beforehand.
A CMake-managed, TSan-instrumented build of producer-consumer, wired with -Wl,--wrap=pthread_join and src/lib/pthread_join_wrap.c so it exercises the DMTCP-restart + TSan pthread_join fix without a manual ad hoc compile step.
Without --wrap (see src/lib/pthread_join_wrap.c), a DMTCP-resurrected thread's pthread_join() can hang forever under TSan. Detect both conditions via weak symbols (same idiom as the other TSan/DMTCP optional-symbol checks in this codebase): __tsan_acquire is non-NULL only when libtsan.so is loaded (i.e. a TSan target), and __wrap_pthread_join is non-NULL only if the target itself was linked with --wrap. Warn as early as possible (a constructor) when the former is true and the latter isn't. Verified via `mcmini`: a TSan target built without --wrap prints the warning, producer-consumer-tsan (built with --wrap) and a plain non-TSan target both stay silent.
Its destructor unconditionally ran `dmtcp_command -q --port <coordinator_target.get_port()>`, but coordinator_target (a dmtcp_coordinator member) never had launch_and_wait() called anywhere in this class, so its port field stayed at its default-constructed 0 forever -- this call always failed, since no coordinator ever listens on port 0. It's also unnecessary: each branch's own `dmtcp_restart --new-coordinator --port 0` call spawns its own private coordinator with --exit-on-last baked in by DMTCP itself, so it already self-terminates once its sole client disconnects. Nothing in this class needs explicit shutdown. Confirmed via a restart run: the bogus `dmtcp_command -q --port 0` [...] exited with status 2` error no longer appears, with no other change in behavior.
local_linux_process::execute_runner() previously treated any pending SIGCHLD as proof that *this* branch had just died, and reported it with a hardcoded SIGTERM regardless of the true cause. But signal_tracker's SIGCHLD count is a global counter, not tied to a pid, and ~local_linux_process() (invoked by coordinator::assign_new_process_handle() to tear down the previous branch immediately before spawning a new one, e.g. from return_to_depth() on every DPOR backtrack) kills that old process and reaps it without ever consuming the SIGCHLD it generates. That leftover count then lingers until the *next* branch process's very first execute_runner() call, which sees try_consume_signal() return true and wrongly concludes the brand-new process just died -- even though it's alive and simply hasn't responded yet. Confirmed via a waitpid() on the supposedly-dead pid: it blocked forever, proving the process was never actually dead. Fix: ~local_linux_process() now consumes the SIGCHLD from its own deliberate kill, then drains (non-blocking) any other already-reapable zombies -- e.g. the old branch's own private DMTCP coordinator -- consuming one signal per reap, since none of them are tied to a specific pid either. execute_runner() itself also loops on a pending SIGCHLD instead of treating it as automatic proof of death: since the counter isn't pid-scoped, a signal here can still belong to some other descendant. It confirms via a non-blocking waitpid() on this->pid specifically, resuming the wait if that comes back empty, and only reports the real signal/exit code once waitpid() actually confirms this->pid died. Verified: 9/10 fresh-checkpoint restart runs now fully explore all 9 traces and complete cleanly, with zero false "Abnormally Termination" reports across every run (previously: every run failed after 1-2 branches). One rarer, separate hang remains -- a genuine restart-synchronization stall, not a signal-tracking issue -- tracked separately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ber) under TSan Adds test/tsan_support/test_fastpath_fork_clone_fiber.c, a self-contained host-side test (compiled/run directly with gcc, not via CMake) that mirrors dmtcp-callback.c's fast-path fork+clone+fiber resumption mechanism to prove under real ThreadSanitizer that: - R3 (using libc's __clone instead of the public, libtsan-intercepted clone()) and R4's forking-thread fiber switch are both required: with -DMTF_BUGGY (public clone(), no forker fiber switch) the recreated threads crash with "ThreadSanitizer: CHECK failed: tsan_rtl.cpp:253 ((!thr->slot)) != (0)" inside ForkChildAfter, and the child process exits with code 66. - The default (fixed) configuration passes cleanly: both parent and child report shared_counter=300000 (expected 300000), the child exits 0, and no CHECK failures occur across repeated runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ded_fork restart_child_threads_fast() now calls libc's raw __clone() instead of the public clone(), since libtsan's clone() interceptor treats every call as a fork and corrupts its thread-slot state for the CLONE_THREAD clone used to recreate pre-checkpoint threads. Also switch the forking thread itself onto a fresh TSan fiber in fast_multithreaded_fork()'s child branch, since it otherwise keeps its inherited (fork-copied) ThreadState whose shadow call stack can overflow as the thread keeps running. Both fixes were already proven against a standalone TSan repro harness in a prior commit; this applies them to the production DMTCP restart path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rness comment Final review found the harness's forking thread (main()) never does enough post-fork instrumented work to trigger the shadow-call-stack overflow that the forking-thread fiber switch (R4 remainder) guards against, so the RED/GREEN evidence only actually falsifies/confirms R3 (__clone), not R4. Rewrite the header comment to state this accurately; no code logic changes.
…witch The fiber switch is guarded by #ifndef MTF_BUGGY, so it does not execute in the buggy build -- only the default (fixed) one. Doesn't change any of the comment's substantive conclusions (still not independently validated by this harness), per final re-review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Real DMTCP restart testing (dmtcp-tsan.git) surfaced a second LD_PRELOAD interceptor on the raw __clone symbol used to recreate threads after checkpoint restart: DMTCP's own libdmtcp.so strongly exports __clone (threadwrappers.cpp) and unconditionally aborts unless the caller is mid-DMTCP's-own pthread_create -- ASSERT at threadwrappers.cpp:177: false: Thread creation with clone syscall is not supported: flags=4001536 matching restart_child_threads_fast()'s exact clone_flags. Calling the bare `__clone` symbol hits this interceptor instead of libc's real implementation, exactly like the public clone() hits libtsan's (the reason this code already avoided clone() in favor of __clone). Resolve the real, uninterposed __clone the same way this codebase already resolves other real libc primitives around interceptors: add a libc_clone() forwarding function to interception.c's existing mc_load_intercepted_pthread_functions() table, dlsym'd from libc once at process startup, well before any fork or checkpoint restart ever happens. An initial version of this fix instead did a fresh dlopen/dlsym lazily inside restart_child_threads_fast() itself (post-_Fork(), in the child); code review flagged that as a duplicate resolution mechanism that also risked deadlocking on a dynamic-linker lock left held by a thread that no longer exists in that child, so it was folded into the existing, already-proven-safe libc_* table instead. Verified end-to-end against a real DMTCP checkpoint/restart of the producer-consumer example (record via `mcmini -i 1`, restart via `mcmini --from-checkpoint ... --multithreaded-fork`): the full pipeline (checkpoint -> restart -> fast_multithreaded_fork() -> libc_clone()-based thread recreation -> DPOR exploration) completes cleanly -- 9 traces explored, correct DEADLOCK detection, exit 0, "Deep debugging completed!", zero ASSERT/CHECK-failed/segfault occurrences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…technique This test demonstrates that pthread_exit can be safely resolved via dlopen+dlsym and called from a worker thread, with the return value correctly delivered to pthread_join(). This technique is used in the next task for production code. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Intercepts pthread_exit() and routes it through the existing THREAD_EXIT_TYPE model-checking machinery (mc_exit_thread_in_child() / mc_exit_main_thread_in_child()) uniformly across all restart/branch modes, instead of falling through to the real libpthread pthread_exit and, on a __clone()-recreated thread, tripping libtsan's real interceptor (which is not registered for that "fiber" thread and can crash). Pre-restart modes forward to the real pthread_exit via a new libtsan-bypassing libpthread_pthread_exit() handle, resolved with the same dlopen+dlsym technique already used for pthread_timedjoin_np and validated standalone in the prior commit's harness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ion.h Matches the header's existing convention (libc_exit/libc_abort use the macro two lines below); interception.c's own pointer declarations already use the raw attribute for a different, pre-existing reason (exit_ptr/abort_ptr), so that file is untouched. Per Phase 3 Task 2 review's one Minor finding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…as live DMTCP_RESTART_INTO_BRANCH/TEMPLATE were grouped with TARGET_BRANCH* in a single case, skipping the mandatory thread_handle_after_dmtcp_restart() call that every other wrapper performs for those two restart modes. Split them into their own case that reports THREAD_EXIT_TYPE, calls thread_handle_after_dmtcp_restart(), then falls through into the unchanged TARGET_BRANCH*/TARGET_BRANCH_AFTER_RESTART body, matching the mc_transparent_exit/mc_transparent_abort fallthrough precedent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A DMTCP-restarted target thread can be genuinely blocked at the kernel level on this exact futex word while glibc's own userspace "is anyone really waiting" bookkeeping (packed into the same memory sem_post() checks before deciding whether to skip the underlying FUTEX_WAKE syscall) is desynced from that -- because mc_runner_mailbox_init()/ _destroy() reinitialize this memory before every new DMTCP-restarted branch, independent of whatever kernel-level futex wait state a resurrected thread still has queued. When that happens, sem_post() silently skips the wake and the branch hangs forever. A plain futex word has no such bookkeeping: mc_raw_sem_post() always calls FUTEX_WAKE unconditionally, and mc_raw_sem_wait() only ever blocks after re-checking the atomic counter, so a wake can never be lost regardless of what order post/wait actually race in. Diagnosed via Gemini-assisted research into glibc's NPTL sem_t internals (packed nwaiters/value in one 64-bit word on 64-bit architectures) plus a direct FUTEX_WAKE(INT_MAX) probe confirming exactly one real waiter was queued when a hang occurred -- ruling out stale/multiple waiters as the cause and pointing squarely at the userspace-bookkeeping desync. model_side_sem is untouched: mcmini itself is never checkpointed, so glibc's bookkeeping for it can never desync. Condition variables (pthread_cond_wait/pthread_cond_signal) have the same class of vulnerability via glibc's G1/G2 waiter-group bookkeeping and will need an analogous fix separately. See doc/glibc-sem-desync.txt for a fuller explanation of the desync mechanism and why it is specific to checkpoint/restart. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
do_recording() (src/mcmini/mcmini.cpp) resolves the DMTCP plugin path as getcwd()/libmcmini.so, so mcmini must be invoked from the directory holding libmcmini.so -- CMAKE_BINARY_DIR, not this target's own output directory (build/src/examples/). A manually-placed copy in CMAKE_BINARY_DIR from days ago silently went stale relative to rebuilds of libmcmini.so and this target itself, costing significant debugging time chasing what looked like a live TSan/DMTCP hang but was actually just a pre-fix binary missing -Wl,--wrap=pthread_join's effect. Add a POST_BUILD copy so this can't happen again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mc_pthread_cond_wait() never calls the real libpthread_cond_wait()/ libpthread_cond_timedwait() in any post-restart mode (DMTCP_RESTART_ INTO_BRANCH/TEMPLATE, TARGET_BRANCH, TARGET_BRANCH_AFTER_RESTART): the wait is entirely simulated via the mailbox handshake, true even in classic (non-DMTCP) mode. But mc_pthread_cond_signal()/_broadcast()/ _init()/_destroy() still called the real libpthread_cond_signal()/ broadcast()/init()/destroy() in those same modes -- an asymmetry. That asymmetry is dangerous specifically under DMTCP restart: a clone()-recreated thread can still be genuinely, kernel-level blocked inside a pre-restart real pthread_cond_timedwait() call (from RECORD mode, if the checkpoint landed mid-call). A real signal/broadcast reaching that thread wakes it for real, letting it resume running application code without ever going through the model checker's own scheduling -- breaking DPOR's single-stepping invariant. Unlike the child_side_sem fix, this isn't a lost-wakeup story: it's a real, uncontrolled wakeup escaping the model checker entirely, which is arguably worse than a hang. Fix: remove the real libpthread_cond_signal()/broadcast()/init()/ destroy() calls from all four post-restart cases, mirroring what mc_pthread_cond_wait() already did -- once wait never consults the real object's state, touching it from signal/broadcast/init/destroy serves no purpose, only risk. RECORD/PRE_CHECKPOINT mode is untouched: those real calls remain necessary and safe there, since it's one continuous execution before any checkpoint exists. Verified classic-mode cv-test produces byte-for-byte-equivalent output (modulo debug-log pid/interleaving) before and after, confirming no behavior change for the already-real-call-free wait path this mirrors. See doc/glibc-cond-var-desync.txt for the full analysis, including why a normally single-process (pshared=0) condition variable is affected by the same class of bug as the pshared=1 child_side_sem mailbox semaphore despite the different pshared requirement. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
producer-consumer(-tsan) has no condition variables at all, so it can't exercise doc/glibc-cond-var-desync.txt's fix (f54bb73) under a real DMTCP+TSan checkpoint/restart cycle. Add a structurally identical producer-consumer variant that uses a mutex + condition variable (count-based bounded buffer, single shared cond, classic while-loop predicate wait) instead of the two semaphores, plus the matching TSan target -- same build recipe as producer-consumer-tsan (-fsanitize=thread, -Wl,--wrap=pthread_join + pthread_join_wrap.c, POST_BUILD copy into CMAKE_BINARY_DIR). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DMTCP now supports TSAN. Working on porting this support to McMini/DeepDebug.
Part of the issue is that McMini, TSAN and DMTCP all have wrapper functions, and they can interfere with each other if we are not careful.