From ac1f54b4539cf71e67a69cf41ac90b0a0c772db6 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Wed, 1 Jul 2026 15:32:30 -0400 Subject: [PATCH 01/16] Bypass libtsan in mc_pthread_join's timed join mc_pthread_join's RECORD loop called pthread_timedjoin_np directly, which resolves to libtsan's interceptor under a TSAN target. Its 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. This completes end-to-end TSAN-target checkpointing under deep-debug (mcmini record mode), alongside 5be8500 (DMTCP plugin API v3->v4) and 4bf2720 (TSan-safe RECORD prologue). Verified: `mcmini -i 3 ~/dmtcp.git/test/tsan_target` runs with no SEGV/ThreadSanitizer errors, producing a valid checkpoint matching the no-mcmini baseline. Co-Authored-By: Claude Opus 4.8 (1M context) --- include/mcmini/spy/intercept/interception.h | 5 +++++ src/lib/interception.c | 7 +++++++ src/lib/wrappers.c | 5 ++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/include/mcmini/spy/intercept/interception.h b/include/mcmini/spy/intercept/interception.h index 105f024f..0ab77d15 100644 --- a/include/mcmini/spy/intercept/interception.h +++ b/include/mcmini/spy/intercept/interception.h @@ -29,6 +29,11 @@ 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*); int libpthread_mutex_init(pthread_mutex_t *, const pthread_mutexattr_t *); int libpthread_mutex_lock(pthread_mutex_t *); diff --git a/src/lib/interception.c b/src/lib/interception.c index 6d0b66f6..26c0cab1 100644 --- a/src/lib/interception.c +++ b/src/lib/interception.c @@ -16,6 +16,7 @@ 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; 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 +70,7 @@ 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"); 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,6 +230,11 @@ 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); diff --git a/src/lib/wrappers.c b/src/lib/wrappers.c index a385c012..08925a3c 100644 --- a/src/lib/wrappers.c +++ b/src/lib/wrappers.c @@ -726,7 +726,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; From 2df3f55068bccc89ab1d017ff67fa39e73451c27 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Sun, 26 Jul 2026 11:29:17 -0400 Subject: [PATCH 02/16] Joined threads now terminate, not park forever 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. --- src/lib/wrappers.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/lib/wrappers.c b/src/lib/wrappers.c index 08925a3c..b73b0d40 100644 --- a/src/lib/wrappers.c +++ b/src/lib/wrappers.c @@ -18,6 +18,11 @@ typedef struct pthread_map { pthread_t thread; runner_id_t value; + // Posted by whichever thread eventually calls pthread_join() on this + // one (mc_pthread_join()'s TARGET_BRANCH case), waited on by this + // thread itself before it may really terminate (mc_exit_thread_in_child()). + // See the two functions for the full rationale. + sem_t exit_permission_sem; struct pthread_map *next; } pthread_map_t; @@ -29,6 +34,7 @@ void insert_pthread_map(pthread_t t, runner_id_t v) { pthread_map_t *n = malloc(sizeof *n); n->thread = t; n->value = v; + libpthread_sem_init(&n->exit_permission_sem, 0, 0); n->next = head; head = n; pthread_rwlock_unlock(&pthread_map_lock); @@ -47,6 +53,21 @@ runner_id_t search_pthread_map(pthread_t t) { return RID_INVALID; } +// Returns the given thread's own exit_permission_sem (see pthread_map_t), +// or NULL if the thread is not (yet) registered. +sem_t *find_exit_permission_sem(pthread_t t) { + pthread_rwlock_rdlock(&pthread_map_lock); + pthread_map_t *cur = head; + while (cur) { + if (pthread_equal(cur->thread, t)) { + break; + } + cur = cur->next; + } + pthread_rwlock_unlock(&pthread_map_lock); + return cur == NULL ? NULL : &cur->exit_permission_sem; +} + MCMINI_THREAD_LOCAL runner_id_t tid_self = RID_INVALID; @@ -335,7 +356,16 @@ void mc_exit_thread_in_child(void) { thread_get_mailbox()->type = THREAD_EXIT_TYPE; thread_wake_scheduler_and_wait(); thread_awake_scheduler_for_thread_finish_transition(); - thread_block_indefinitely(); + + // Wait for whichever thread eventually calls pthread_join() on this one + // (see mc_pthread_join()'s TARGET_BRANCH case) to grant permission before + // this thread is allowed to really terminate. This keeps this thread's + // pthread_t/tid valid for exactly as long as a real, unjoined POSIX + // thread's would be -- no longer -- rather than parking it forever + // regardless of whether anyone ever joins it. + sem_t *exit_permission = find_exit_permission_sem(pthread_self()); + assert(exit_permission != NULL); + libpthread_sem_wait(exit_permission); } void mc_exit_main_thread_in_child(void) { @@ -772,7 +802,16 @@ int mc_pthread_join(pthread_t t, void **rv) { memcpy_v(thread_get_mailbox()->cnts, &rid, sizeof(runner_id_t)); thread_get_mailbox()->type = THREAD_JOIN_TYPE; thread_wake_scheduler_and_wait(); - return 0; + + // Grant the target thread permission to really terminate now that + // it's been joined (see mc_exit_thread_in_child()), then perform a + // genuine join, so its OS thread actually dies -- and *rv is + // genuinely populated -- before we return, instead of only + // simulating success at the model level. + sem_t *exit_permission = find_exit_permission_sem(t); + assert(exit_permission != NULL); + libpthread_sem_post(exit_permission); + return libpthread_pthread_join(t, rv); } default: { libc_abort(); From 0bd036baafd5eb14ca0ef98689612313e07df6b7 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Sun, 26 Jul 2026 11:30:32 -0400 Subject: [PATCH 03/16] Fix pthread_map_lock leak in search_pthread_map() --- src/lib/wrappers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/wrappers.c b/src/lib/wrappers.c index b73b0d40..e2299bac 100644 --- a/src/lib/wrappers.c +++ b/src/lib/wrappers.c @@ -45,12 +45,12 @@ runner_id_t search_pthread_map(pthread_t t) { pthread_map_t *cur = head; while (cur) { if (pthread_equal(cur->thread, t)) { - return cur->value; + break; } cur = cur->next; } pthread_rwlock_unlock(&pthread_map_lock); - return RID_INVALID; + return cur == NULL ? RID_INVALID : cur->value; } // Returns the given thread's own exit_permission_sem (see pthread_map_t), From b53a77b2c4fc81b6477f4359134e274d98953d43 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Mon, 27 Jul 2026 00:34:27 -0400 Subject: [PATCH 04/16] DPOR backtrack replay: Report abnormal termination 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. --- src/mcmini/mcmini.cpp | 15 ++++++++++++--- .../model_checking/algorithms/classic_dpor.cpp | 13 ++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/mcmini/mcmini.cpp b/src/mcmini/mcmini.cpp index 025c28e3..26da2c67 100644 --- a/src/mcmini/mcmini.cpp +++ b/src/mcmini/mcmini.cpp @@ -142,14 +142,23 @@ void found_abnormal_termination( for (const auto& t : program_model.get_trace()) { ss << "thread " << t->get_executor() << ": " << t->to_string() << "\n"; } + // `ub.culprit` may not have a pending transition in the model's current + // view: this callback can also fire for a termination_error raised while + // `coordinator::return_to_depth()` is replaying already-recorded history + // against a freshly restarted process, and by the replay's target depth + // the culprit thread may already have exited in the model. const transition* terminator = program_model.get_pending_transition_for(ub.culprit); - ss << "thread " << terminator->get_executor() << ": " - << terminator->to_string() << "\n"; + if (terminator != nullptr) { + ss << "thread " << terminator->get_executor() << ": " + << terminator->to_string() << "\n"; + } else { + ss << "thread " << ub.culprit << ": (no longer pending)\n"; + } ss << "\nNEXT THREAD OPERATIONS\n"; for (const auto& tpair : program_model.get_pending_transitions()) { - if (tpair.first == terminator->get_executor()) { + if (terminator != nullptr && tpair.first == terminator->get_executor()) { ss << "thread " << tpair.first << ": executing" << "\n"; } else { diff --git a/src/mcmini/model_checking/algorithms/classic_dpor.cpp b/src/mcmini/model_checking/algorithms/classic_dpor.cpp index 2504f213..533ca766 100644 --- a/src/mcmini/model_checking/algorithms/classic_dpor.cpp +++ b/src/mcmini/model_checking/algorithms/classic_dpor.cpp @@ -197,7 +197,18 @@ void classic_dpor::verify_using(coordinator &coordinator, // backtracking. log_debug(dpor_logger) << "Backtracking to depth `" << (dpor_stack.size() - 1) << "`"; - coordinator.return_to_depth(dpor_stack.size() - 1); + try { + coordinator.return_to_depth(dpor_stack.size() - 1); + } catch (const real_world::process::termination_error &te) { + // The process spawned to replay history up to this depth (see + // `coordinator::return_to_depth()`) died before the replay + // finished. Report it the same way a termination during forward + // exploration is reported, rather than letting it escape + // unhandled all the way out of `verify_using()`. + if (callbacks.abnormal_termination) + callbacks.abnormal_termination(coordinator, model_checking_stats, te); + return; + } log_debug(dpor_logger) << "Finished backtracking to depth `" << (dpor_stack.size() - 1) << "`"; model_checking_stats.trace_id++; From 39d98236e998ba5b8bfb0d3c5beaa2c30c63c625 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Wed, 1 Jul 2026 13:47:19 -0400 Subject: [PATCH 05/16] Port libmcmini's DMTCP plugin to API v4, from v3 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) --- include/dmtcp.h | 102 ++++++++++++++++++++++++++++---- src/common/multithreaded_fork.c | 4 +- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/include/dmtcp.h b/include/dmtcp.h index d0e774e4..a545c348 100644 --- a/include/dmtcp.h +++ b/include/dmtcp.h @@ -16,6 +16,8 @@ #define DMTCP_H #include +#include +#include #include #include #include @@ -43,8 +45,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 { @@ -145,6 +147,11 @@ typedef union _DmtcpEventData_t { struct { char *path; } realToVirtualPath, virtualToRealPath; + + struct { + pthread_t pthread; + pid_t tid; + } pthreadInfo; } DmtcpEventData_t; typedef void (*HookFunctionPtr_t)(DmtcpEvent_t, DmtcpEventData_t *); @@ -238,12 +245,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 +402,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 +499,22 @@ 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)); + +// Returns 1 if virtual_tid names TSAN's own background thread, else 0. +int dmtcp_is_tsan_background_thread(int virtual_tid) __attribute((weak)); +#define dmtcp_is_tsan_background_thread(virtual_tid) \ + (dmtcp_is_tsan_background_thread ? \ + dmtcp_is_tsan_background_thread(virtual_tid) : 0) + +// 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 +525,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/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; From 9bd964deb239c2955676ba1b33f7b2aed694725b Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Mon, 27 Jul 2026 01:15:46 -0400 Subject: [PATCH 06/16] Fix restart-barrier race in template_thread() 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. Also remove fast_multithreaded_fork()'s #if 1/#else wrapper: the #else side held an early, never-compiled clone()-based fork prototype that predates this file's current _Fork()-based approach and was always dead code. Also remove the needless signal-mask check in threaded fork: getcontext()/setcontext() already restore a thread's blocked-signal set via uc_sigmask, even across the raw clone() used to recreate a checkpointed thread, so the thread_sigmask field and abort were unnecessary. That 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. --- src/lib/dmtcp-callback.c | 86 ++++++++++++---------------------------- 1 file changed, 26 insertions(+), 60 deletions(-) diff --git a/src/lib/dmtcp-callback.c b/src/lib/dmtcp-callback.c index 7438c57a..db6cb0e3 100644 --- a/src/lib/dmtcp-callback.c +++ b/src/lib/dmtcp-callback.c @@ -1,6 +1,5 @@ #define _GNU_SOURCE #include -#include #include #include // man 2 open #include @@ -42,8 +41,6 @@ struct threadinfo { // tlsAddr only used in __aarch64__ and __riscv // In fact, __riscv has the address in a normal register, restored w/ context. unsigned long int tlsAddr; - // The kernel has a process-wide sigmask, and also a per-thread sigmask. - sigset_t thread_sigmask; // glibc:pthread_create and pthread_self use this, but not the clone call: pthread_t pthread_descriptor; } threadInfos[1000]; @@ -125,23 +122,19 @@ static void saveThreadStateBeforeFork(struct threadinfo* threadInfo) { threadInfo->pthread_descriptor = pthread_self(); getTLSPointer(threadInfo); - // FIXME: Add func fo get/set signals in child thread of child process. - // and restore thread sigmask sfter setcontext. - pthread_sigmask(SIG_BLOCK, NULL, &threadInfo->thread_sigmask); - sigset_t sigtest; - pthread_sigmask(SIG_BLOCK, NULL, &sigtest); - sigdelset(&sigtest, SIG_MULTITHREADED_FORK); - if (! sigisemptyset(&sigtest)) { - fprintf(stderr, "PID %d: multithreaded_fork() not yet implemented" - " for non-empty thread signaks\n", getpid()); - libc_abort(); - } + // No manual signal-mask save/restore needed: getcontext()/setcontext() + // already save/restore the blocked-signal set via ucontext_t's uc_sigmask, + // even when setcontext() resumes on a brand-new clone()'d OS thread (see + // child_setcontext_fast() below). } static int child_setcontext_fast(void *arg) { struct threadinfo* threadInfo = arg; setTLSPointer(threadInfo); patchThreadDescriptor(threadInfo->pthread_descriptor); + // Does not return: jumps to the getcontext() call site in + // thread_handle_after_dmtcp_restart(), restoring uc_sigmask (see + // saveThreadStateBeforeFork() above) along with the rest of the context. setcontext(&(threadInfo->context)); return 0; // not reached } @@ -191,39 +184,8 @@ pid_t fast_multithreaded_fork(void) { * ret = INLINE_SYSCALL_CALL (clone, flags, 0, NULL, ctid, 0); *} *********************************************************************/ -#if 1 pid_t _Fork(); int childpid = _Fork(); -#else - // NOT YET FULLY DEVELOPED: - int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD; - int childpid; -// syscall(SYS_clone, ...); -// stack must be NULL -// https://stackoverflow.com/questions/2898579/clone-equivalent-of-fork -// But that says to use only SIGCHLD for flags, and glibc uses the above. -// But it's okay, since we're setting ctid and tls to NULL. -// FIXME: If we're going to set the last 3 args to NULL, who cares in what order they're found! -# ifdef __x86_64__ - long clone(unsigned long flags, void *stack, - int *parent_tid, int *child_tid, - unsigned long tls); -# elif defined(__aarch64__) - long clone(unsigned long flags, void *stack, - int *parent_tid, unsigned long tls, - int *child_tid); -# elif defined(__riscv) -# error Unimplemented CPU architecture -https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/riscv/clone.S -int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg, - void *parent_tidptr, void *tls, void *child_tidptr) */ - /* The syscall expects the args to be in different slots. */ - mv a0,a2 - mv a2,a4 - mv a3,a5 - mv a4,a6 -# endif -#endif if (childpid == 0) { // child process restart_child_threads_fast(); } @@ -321,23 +283,27 @@ static void *template_thread(void *unused) { // to ensure a stable recorded state is pointless: we're not going to read it // anyway! This is an only a potential optimization for later though. + // Counting via a live /proc/self/task scan is a TOCTOU race: DMTCP + // recreates checkpointed threads asynchronously via clone(), so a scan + // that runs before it has finished recreating all of them undercounts, + // and this barrier then releases before every thread has actually + // restarted (confirmed empirically: one thread's own restart-completion + // signal can arrive after this barrier already declared a "consistent + // state"). + // + // head_record_mode instead gives an exact, race-free count: it only ever + // gets THREAD entries for genuine target threads (the template thread and + // the checkpoint thread never go through libmcmini's wrapped + // pthread_create(), so neither is ever recorded here), and -- since a + // DMTCP checkpoint is a full memory snapshot -- this list is preserved + // exactly as it was at record time across every restart, with no + // dependence on restart-time scheduling. 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) + for (rec_list *entry = head_record_mode; entry != NULL; entry = entry->next) { + if (entry->vo.type == THREAD && entry->vo.thrd_state.status == ALIVE) { 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); + } + } log_debug( "There are %d threads... waiting for them to get into a consistent " "state...\n", From 9d56beb933f784152746298efbf7f514d96b32eb Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Sun, 26 Jul 2026 12:20:38 -0400 Subject: [PATCH 07/16] Don't resume checkpoint loop after branch restart 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. --- include/dmtcp.h | 10 ++++++++++ src/lib/dmtcp-callback.c | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/include/dmtcp.h b/include/dmtcp.h index a545c348..e2e46f39 100644 --- a/include/dmtcp.h +++ b/include/dmtcp.h @@ -508,6 +508,16 @@ int dmtcp_is_tsan_background_thread(int virtual_tid) __attribute((weak)); (dmtcp_is_tsan_background_thread ? \ dmtcp_is_tsan_background_thread(virtual_tid) : 0) +// Tells DMTCP that this restart is one-shot: the checkpoint thread should +// not resume its usual sleep-checkpoint-resume loop (i.e., it should not +// wait for a future checkpoint request), since no such request is ever +// coming. Must be called on the checkpoint thread itself, from a +// DMTCP_EVENT_RESTART hook (before that event's plugin dispatch returns). +void dmtcp_skip_post_restart_checkpoint_loop(void) __attribute((weak)); +#define dmtcp_skip_post_restart_checkpoint_loop() \ + (dmtcp_skip_post_restart_checkpoint_loop ? \ + dmtcp_skip_post_restart_checkpoint_loop() : (void)0) + // 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}.) diff --git a/src/lib/dmtcp-callback.c b/src/lib/dmtcp-callback.c index db6cb0e3..cc67884c 100644 --- a/src/lib/dmtcp-callback.c +++ b/src/lib/dmtcp-callback.c @@ -545,6 +545,14 @@ static void presuspend_eventHook(DmtcpEvent_t event, DmtcpEventData_t *data) { } else { set_current_mode(DMTCP_RESTART_INTO_BRANCH); log_debug("`MCMINI_TEMPLATE_LOOP` was not set at restart-time\n"); + + // A DMTCP_RESTART_INTO_BRANCH process explores exactly one trace, + // then this process is discarded -- it will never legitimately be + // asked to checkpoint again. Left alone, the checkpoint thread would + // resume its normal sleep-checkpoint-resume loop and block forever + // waiting for a checkpoint request from this restart's (one-shot, + // otherwise idle) coordinator. + dmtcp_skip_post_restart_checkpoint_loop(); } // During record mode, the shared memory From fbc3cdeb3233212258bcc0555bf688bfc5ef59d1 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Mon, 27 Jul 2026 01:38:13 -0400 Subject: [PATCH 08/16] Remove dead coordinator-shutdown cleanup code Its destructor unconditionally ran `dmtcp_command -q --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. --- .../mcmini/real_world/process/dmtcp_process_source.hpp | 4 +--- src/mcmini/real_world/dmtcp_process_source.cpp | 8 -------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/include/mcmini/real_world/process/dmtcp_process_source.hpp b/include/mcmini/real_world/process/dmtcp_process_source.hpp index 7e334f75..c9806d9a 100644 --- a/include/mcmini/real_world/process/dmtcp_process_source.hpp +++ b/include/mcmini/real_world/process/dmtcp_process_source.hpp @@ -6,7 +6,7 @@ #include "mcmini/defines.h" #include "mcmini/forwards.hpp" -#include "mcmini/real_world/process/dmtcp_coordinator.hpp" +#include "mcmini/real_world/dmtcp_target.hpp" #include "mcmini/real_world/process/local_linux_process.hpp" #include "mcmini/real_world/process_source.hpp" #include "mcmini/real_world/shm.hpp" @@ -27,14 +27,12 @@ class dmtcp_process_source : public process_source { private: std::string ckpt_file; dmtcp_target dmtcp_restart_target; - dmtcp_coordinator coordinator_target; private: pid_t make_new_branch(); public: dmtcp_process_source(const std::string &ckpt_file); - virtual ~dmtcp_process_source(); public: std::unique_ptr make_new_process() override; diff --git a/src/mcmini/real_world/dmtcp_process_source.cpp b/src/mcmini/real_world/dmtcp_process_source.cpp index 89b79dff..268d2dbe 100644 --- a/src/mcmini/real_world/dmtcp_process_source.cpp +++ b/src/mcmini/real_world/dmtcp_process_source.cpp @@ -66,11 +66,3 @@ std::unique_ptr dmtcp_process_source::make_new_process() { // assert(tstruct->cpid == target_branch_pid); return extensions::make_unique(target_branch_pid); } - -dmtcp_process_source::~dmtcp_process_source() { - target dmtcp_cleanup( - "dmtcp_command", - {"-q", "--port", std::to_string(this->coordinator_target.get_port())}); - dmtcp_cleanup.set_quiet(true); - dmtcp_cleanup.launch_and_wait(); -} From 0613e4481cbefd88966ca697df9e7ea7e153acee Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Mon, 27 Jul 2026 02:04:03 -0400 Subject: [PATCH 09/16] Stop leaking stale SIGCHLD into next branch 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 --- .../algorithms/classic_dpor.cpp | 4 + src/mcmini/real_world/local_linux_process.cpp | 96 ++++++++++++------- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/mcmini/model_checking/algorithms/classic_dpor.cpp b/src/mcmini/model_checking/algorithms/classic_dpor.cpp index 533ca766..d9dbde56 100644 --- a/src/mcmini/model_checking/algorithms/classic_dpor.cpp +++ b/src/mcmini/model_checking/algorithms/classic_dpor.cpp @@ -208,6 +208,10 @@ void classic_dpor::verify_using(coordinator &coordinator, if (callbacks.abnormal_termination) callbacks.abnormal_termination(coordinator, model_checking_stats, te); return; + } catch (const real_world::process::nonzero_exit_code_error &nzec) { + if (callbacks.nonzero_exit_code) + callbacks.nonzero_exit_code(coordinator, model_checking_stats, nzec); + return; } log_debug(dpor_logger) << "Finished backtracking to depth `" << (dpor_stack.size() - 1) << "`"; diff --git a/src/mcmini/real_world/local_linux_process.cpp b/src/mcmini/real_world/local_linux_process.cpp index 75c46036..9102b4ec 100644 --- a/src/mcmini/real_world/local_linux_process.cpp +++ b/src/mcmini/real_world/local_linux_process.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -52,6 +53,29 @@ local_linux_process::~local_linux_process() { } else { log_error(process_logger) << "Error: " << strerror(errno); } + } else { + // This death is expected and already fully handled (we just reaped + // it above): consume the SIGCHLD it generated, so it doesn't linger in + // signal_tracker's counter. Otherwise the next process this class + // creates (see coordinator::return_to_depth()/assign_new_process_handle(), + // which destroys the old handle immediately before creating a new + // one) would see that leftover count on its own first execute_runner() + // call and wrongly conclude *it* had just died, even though it's + // still alive and simply hasn't responded yet. + signal_tracker::instance().try_consume_signal(SIGCHLD); + + // The process we just killed may itself have left behind other + // now-orphaned descendants (e.g. its own private DMTCP coordinator, + // spawned by `dmtcp_restart --new-coordinator`) that die/reparent to + // us -- as the PR_SET_CHILD_SUBREAPER subreaper -- around the same + // time, generating their own SIGCHLDs. signal_tracker's counter isn't + // tied to a specific pid, so drain every zombie that's already + // reapable right now (non-blocking: this must never wait on a + // descendant that hasn't died yet) and consume one signal per reap, + // so none of them linger to be misattributed later either. + while (waitpid(-1, &status, WNOHANG) > 0) { + signal_tracker::instance().try_consume_signal(SIGCHLD); + } } } } @@ -83,40 +107,44 @@ volatile runner_mailbox *local_linux_process::execute_runner(runner_id_t id) { // TODO: The template process will also send a SIGCHLD if it dies // unexpectedly. Because we don't expect the template process to die, this is // OK for now, but should be handled in the future. - errno = 0; - signal_tracker::sig_semwait((sem_t *)&rmb->model_side_sem); - if (signal_tracker::instance().try_consume_signal(SIGCHLD)) { - // TODO: Get the true failure status from the template process via - // e.g. shared memory. - throw process::termination_error(SIGTERM, id, - "Process terminated abnormally."); - // // TODO: Double check that this - // // is the correct process that sent - // // the SIGCHILD using WNOHANG. - - // // `PR_SET_CHILD_SUBREAPER` enables us to wait on - // // this grandchild. - // int status; - // int rc = waitpid(this->pid, &status, 0); - // if (rc == -1) { - // throw process::execution_error( - // "Error attempting to determine the failure causing the child - // process " "to abnormally exit (or possibly an internal error of - // McMini)." + std::string(strerror(errno))); - // } else { - // if (WIFEXITED(status)) { - // int exit_code = WEXITSTATUS(status); - // throw process::nonzero_exit_code_error( - // exit_code, "Process terminated with a non-zero exit code."); - // } else if (WIFSIGNALED(status)) { - // int signo = WTERMSIG(status); - // throw process::termination_error(signo, - // "Process terminated abnormally."); - // } else { - // throw process::execution_error( - // "SIGSTOP/SIGCONT in branch processes is not yet supported."); - // } - // } + while (true) { + errno = 0; + signal_tracker::sig_semwait((sem_t *)&rmb->model_side_sem); + if (!signal_tracker::instance().try_consume_signal(SIGCHLD)) { + break; + } + // signal_tracker's SIGCHLD count is process-wide, not tied to a pid, so + // a pending count doesn't necessarily mean *this->pid* is the one that + // died -- some other descendant (e.g. a previous branch's own private + // DMTCP coordinator) may have generated it instead. Confirm with a + // non-blocking waitpid() specifically on this->pid (PR_SET_CHILD_SUBREAPER, + // set in target::prepare_mcmini_targets(), lets us wait on it even though + // it isn't a direct child): if it hasn't actually changed state, this + // SIGCHLD wasn't ours, so just resume waiting instead of misreporting a + // termination that didn't happen. + int status; + int rc = waitpid(this->pid, &status, WNOHANG); + if (rc == 0) { + continue; + } + if (rc == -1) { + throw process::execution_error( + "Error attempting to determine the failure causing the child " + "process to abnormally exit (or possibly an internal error of " + "McMini): " + std::string(strerror(errno))); + } else if (WIFEXITED(status)) { + throw process::nonzero_exit_code_error( + WEXITSTATUS(status), id, + "The program exited with code " + std::to_string(WEXITSTATUS(status))); + } else if (WIFSIGNALED(status)) { + throw process::termination_error( + WTERMSIG(status), id, + "Process terminated abnormally by signal " + + std::to_string(WTERMSIG(status))); + } else { + throw process::execution_error( + "SIGSTOP/SIGCONT in branch processes is not yet supported."); + } } return rmb; } From 0808df68203d76ad6d156e5dffe9f0fa7eec0c8b Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Tue, 28 Jul 2026 11:11:22 -0400 Subject: [PATCH 10/16] Fix exit() hang and false-deadlock report mc_transparent_exit()'s TARGET_BRANCH case used the wait-only thread_await_scheduler() instead of thread_wake_scheduler_and_wait(), unlike every other wrapper. A thread that reaches this case directly (e.g. main() calling exit() as its own first wrapped call, in classic mode or after a restart) never posts model_side_sem, so the coordinator's execute_runner() hangs forever. Verified this is not --multithreaded-fork-specific: reverting just this hunk hangs plain classic mode identically, no DMTCP involved. Also mark the executor exited in process_exit::modify(), mirroring thread_exit: otherwise is_active() keeps reporting true and classic_dpor -- shared across every mode -- either re-selects the transition forever (when program_exit_code() > 0 doesn't already stop exploration, e.g. exit code 0) or reports a false DEADLOCK (confirmed with a trivial single-threaded exit(0) program in classic mode). Adds producer-consumer-park(-tsan), modeled on multithreaded-fork-tsan-2.0's test_park.c, to exercise main() exiting explicitly while other threads are still parked -- the scenario that exposed both bugs under --multithreaded-fork. Verified: a full --multithreaded-fork restart cycle now completes in under a second with a correct DEADLOCK verdict, instead of hanging. Regression-checked producer-consumer(-safe/-exit)-tsan and cv-producer-consumer(-safe)-tsan restart cycles, all still clean. --- .../mcmini/model/transitions/process/exit.hpp | 11 +- src/examples/CMakeLists.txt | 17 ++ src/examples/producer-consumer-park.c | 152 ++++++++++++++++++ src/lib/wrappers.c | 11 +- 4 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 src/examples/producer-consumer-park.c diff --git a/include/mcmini/model/transitions/process/exit.hpp b/include/mcmini/model/transitions/process/exit.hpp index e822bc20..6e69804d 100644 --- a/include/mcmini/model/transitions/process/exit.hpp +++ b/include/mcmini/model/transitions/process/exit.hpp @@ -1,5 +1,6 @@ #pragma once +#include "mcmini/model/objects/thread.hpp" #include "mcmini/model/transition.hpp" namespace model { @@ -14,8 +15,14 @@ struct process_exit : public model::transition { ~process_exit() = default; status modify(model::mutable_state& s) const override { - // We ensure that exiting is never enabled. This ensures that it will never - // be explored by any model checking algorithm + // Mark the executor exited, like thread_exit does (minus its + // RID_MAIN_THREAD restriction, since exit()/abort() can end the process + // from any thread). Without this, is_active() keeps reporting true, so + // classic_dpor re-selects this same "enabled" transition forever for any + // exit code its program_exit_code() > 0 check doesn't already catch + // (i.e. exit code 0). + using namespace model::objects; + s.add_state_for_runner(executor, new thread(thread::exited)); return status::exists; } diff --git a/src/examples/CMakeLists.txt b/src/examples/CMakeLists.txt index 5c056b14..9d0c9e24 100644 --- a/src/examples/CMakeLists.txt +++ b/src/examples/CMakeLists.txt @@ -4,8 +4,25 @@ add_executable(cv-test cv-test.c) add_executable(deadly-embrace deadly-embrace.c) add_executable(fifo-example fifo.cpp) add_executable(producer-consumer producer-consumer.c) +add_executable(producer-consumer-park producer-consumer-park.c) target_link_libraries(hello-world PUBLIC -pthread) target_link_libraries(cv-hello-world PUBLIC -pthread) target_link_libraries(cv-test PUBLIC -pthread) target_link_libraries(deadly-embrace PUBLIC -pthread) target_link_libraries(producer-consumer PUBLIC -pthread) +target_link_libraries(producer-consumer-park PUBLIC -pthread) + +# producer-consumer-park.c never calls pthread_join() or pthread_exit() -- +# main returns while both workers are still alive, parked forever on an +# unposted semaphore (see that file's own comment, modeled on +# multithreaded-fork-tsan-2.0's test_park.c). No --wrap needed: neither +# pthread_join nor pthread_cond_wait is ever called here. +add_executable(producer-consumer-park-tsan producer-consumer-park.c) +target_compile_options(producer-consumer-park-tsan PUBLIC -fsanitize=thread) +target_link_options(producer-consumer-park-tsan PUBLIC -fsanitize=thread) +target_link_libraries(producer-consumer-park-tsan PUBLIC -pthread libmcmini) + +add_custom_command(TARGET producer-consumer-park-tsan POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy + $ + ${CMAKE_BINARY_DIR}/producer-consumer-park-tsan) diff --git a/src/examples/producer-consumer-park.c b/src/examples/producer-consumer-park.c new file mode 100644 index 00000000..b66420f9 --- /dev/null +++ b/src/examples/producer-consumer-park.c @@ -0,0 +1,152 @@ +#include +#include +#include +#include +#include +#include +#include + +#define MaxItems 1 +#define BufferSize 2 + +// Unlike sleep(), a busy-wait can't be cut short by DMTCP's own +// pre-checkpoint signal (which interrupts blocking syscalls, returning +// early with time remaining) -- needed below so main's own delay reliably +// lasts the full duration instead of racing main's return/exit() against +// an in-progress checkpoint write. +static void busy_wait_seconds(int seconds) { + struct timespec start, now; + clock_gettime(CLOCK_MONOTONIC, &start); + do { + clock_gettime(CLOCK_MONOTONIC, &now); + } while (now.tv_sec - start.tv_sec < seconds); +} + +// Same as producer-consumer-safe.c, except main never calls pthread_join(): +// each worker signals `done` once its real work finishes, then parks forever +// on an unposted semaphore, and main returns as soon as both `done`s are +// posted -- exercising a real, alive-but-parked (never joined/exited) thread +// still present when the process ends, matching multithreaded-fork-tsan-2.0's +// test_park.c ("thread resurrection ... without pthread_join/pthread_exit"). +// +// NOTE: main() returning here (rather than e.g. pthread_exit()) triggers +// POSIX exit() semantics -- the whole process tears down immediately, +// including TSan's own exit-time Finalize()/ThreadCount() bookkeeping, which +// reuses main()'s now-dead stack slots (confirmed live via a gdb hardware +// watchpoint: the exact address that held prod_ids[0] gets overwritten by +// TSan's internal "alive thread count" a moment later). This can show up as +// an apparently-garbled producer/consumer id in RECORD-mode's printf output +// on a rare timing -- harmless (pure RECORD-mode artifact, before any +// checkpoint/restart), not a libmcmini/DMTCP bug, and not the subject of +// this file's actual test (multithreaded_fork's recreated-but-unjoined +// thread handling under --multithreaded-fork restart). +sem_t empty; +sem_t full; +sem_t producer_done; +sem_t consumer_done; +sem_t park; // never posted +int in = 0; +int out = 0; +int buffer[BufferSize]; +pthread_mutex_t mutex; +int DEBUG; + +static void sem_wait_retry(sem_t *s) { while (sem_wait(s) != 0) /* EINTR */; } + +void *producer(void *pno) +{ + int item; + for(int i = 0; i < MaxItems; i++) { + sleep(3); + item = rand(); // Produce an random item + sem_wait(&empty); + pthread_mutex_lock(&mutex); + buffer[in] = item; + if (DEBUG) { + printf("Producer %d: Insert Item %d at %d\n", + *((int *)pno),buffer[in],in); + } + in = (in+1)%BufferSize; + pthread_mutex_unlock(&mutex); + sem_post(&full); + } + sem_post(&producer_done); + sem_wait_retry(&park); // park forever; never exit/join + return NULL; +} + +void *consumer(void *cno) +{ + for(int i = 0; i < MaxItems; i++) { + sleep(3); + sem_wait(&full); + pthread_mutex_lock(&mutex); + int item = buffer[out]; + if (DEBUG) { + printf("Consumer %d: Remove Item %d from %d\n", + *((int *)cno),item, out); + } + out = (out+1)%BufferSize; + pthread_mutex_unlock(&mutex); + sem_post(&empty); + } + sem_post(&consumer_done); + sem_wait_retry(&park); // park forever; never exit/join + return NULL; +} + +int main(int argc, char* argv[]) +{ + int NUM_PRODUCERS = 1; + int NUM_CONSUMERS = 1; + DEBUG = 1; + + pthread_t pro[NUM_PRODUCERS],con[NUM_CONSUMERS]; + + pthread_mutex_init(&mutex, NULL); + sem_init(&empty,0,BufferSize); + sem_init(&full,0,0); + sem_init(&producer_done,0,0); + sem_init(&consumer_done,0,0); + sem_init(&park,0,0); + + int prod_ids[NUM_PRODUCERS]; + int cons_ids[NUM_CONSUMERS]; + + for(int i = 0; i < NUM_PRODUCERS; i++) { + prod_ids[i] = i+1; + pthread_create(&pro[i], NULL, producer, (void *)&prod_ids[i]); + } + for(int i = 0; i < NUM_CONSUMERS; i++) { + cons_ids[i] = i+1; + pthread_create(&con[i], NULL, consumer, (void *)&cons_ids[i]); + } + + for(int i = 0; i < NUM_PRODUCERS; i++) { + sem_wait(&producer_done); + } + for(int i = 0; i < NUM_CONSUMERS; i++) { + sem_wait(&consumer_done); + } + // Widen the window where both workers are safely parked (on `park`) + // but main hasn't yet returned/exited -- gives a checkpoint interval + // (mcmini -i N) a reliable target to land on, instead of racing main's + // own exit() against the workers' last bit of real work (see this + // file's top-of-file comment on the TSan-Finalize()/stack-reuse + // artifact that showed up when that race was too tight). + busy_wait_seconds(10); + // Deliberately no pthread_join(): producer/consumer are still alive, + // parked on `park`, when the process ends. + + // Explicit exit() call, NOT `return 0;`: a plain return makes glibc's + // own __libc_start_call_main call __GI_exit -- a glibc-internal alias + // resolved at glibc's own compile time, invisible to ANY interposition + // technique (--wrap, LD_PRELOAD, or a strong-symbol override). Confirmed + // live via gdb: that's exactly what happens on a plain return. Since + // main's own restart-quiescence check-in (mc_transparent_exit(), which + // exit() routes to) never runs in that case, the model checker never + // sees main check in after a restart -- it just watches the whole + // process really exit out from under it. An explicit exit() call is a + // normal, interposable function call, so it checks in correctly. + exit(0); +} diff --git a/src/lib/wrappers.c b/src/lib/wrappers.c index e2299bac..f31019ee 100644 --- a/src/lib/wrappers.c +++ b/src/lib/wrappers.c @@ -419,7 +419,14 @@ MCMINI_NO_RETURN void mc_transparent_exit(int status) { volatile runner_mailbox *mb = thread_get_mailbox(); mb->type = PROCESS_EXIT_TYPE; memcpy_v(mb->cnts, &status, sizeof(status)); - thread_await_scheduler(); + // Unlike every other wrapper's TARGET_BRANCH* case, this used to call + // the wait-only thread_await_scheduler(), which never posts + // model_side_sem. That's harmless for a thread that fell through from + // DMTCP_RESTART_INTO_BRANCH/TEMPLATE above (thread_handle_after_dmtcp_restart() + // already posted it), but a thread already in TARGET_BRANCH_AFTER_RESTART + // (e.g. main(), calling exit() for the first time since restart) jumps + // straight here and hangs the coordinator's execute_runner() forever. + thread_wake_scheduler_and_wait(); // After "exiting", don't actually exit yet: // the model checker will prevent the process @@ -427,7 +434,7 @@ MCMINI_NO_RETURN void mc_transparent_exit(int status) { // branch as "useless" since at this point mb->type = PROCESS_EXIT_TYPE; memcpy_v(mb->cnts, &status, sizeof(status)); - thread_await_scheduler(); + thread_wake_scheduler_and_wait(); } default: { libc_exit(status); From aa3b2b0d1e511d4e826541a985632e814ee633d0 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Tue, 28 Jul 2026 14:22:03 -0400 Subject: [PATCH 11/16] Wire up nonzero_exit_code callback Two separate, previously-noted gaps in the same area: 1. callbacks.nonzero_exit_code was never wired up in mcmini.cpp (in either classic or DMTCP mode), so a target program exiting with a nonzero code -- exactly the kind of bug the model checker exists to catch -- was caught internally in classic_dpor.cpp, then silently discarded: `if (callbacks.nonzero_exit_code)` was always false, so the run just stopped with no verdict printed at all. Added found_nonzero_exit_code(), mirroring found_abnormal_termination()'s existing trace-printing pattern, and wired it into both callback setups. Verified live with a trivial `exit(1);`-only classic-mode target: now prints "NONZERO EXIT CODE (1)" plus the trace instead of silently completing. 2. local_linux_process.cpp's SIGCHLD-based dead-child detection threw nonzero_exit_code_error unconditionally on WIFEXITED(status), regardless of whether WEXITSTATUS(status) was actually nonzero -- misleading given the exception's own name/contract. A clean (code 0) exit reaching this path is a different situation entirely: it means the whole process fully terminated on its own while a transition was still pending on this runner, bypassing the model-driven exit protocol (mc_transparent_exit(), 6bed56a) entirely -- a McMini-side protocol violation to investigate, not a target-program bug. Now throws execution_error for that case instead of mislabeling it. Regression-checked clean against all 6 existing TSan targets. --- src/mcmini/mcmini.cpp | 43 +++++++++++++++++++ src/mcmini/real_world/local_linux_process.cpp | 22 ++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/mcmini/mcmini.cpp b/src/mcmini/mcmini.cpp index 26da2c67..05b41a8b 100644 --- a/src/mcmini/mcmini.cpp +++ b/src/mcmini/mcmini.cpp @@ -172,6 +172,47 @@ void found_abnormal_termination( std::cout.flush(); } +void found_nonzero_exit_code( + const coordinator& c, const stats& stats, + const real_world::process::nonzero_exit_code_error& nzec) { + std::cerr << "NONZERO EXIT CODE (" << nzec.exit_code << "):\n" + << nzec.what() << std::endl; + + std::stringstream ss; + const auto& program_model = c.get_current_program_model(); + ss << "TRACE " << stats.trace_id << "\n"; + for (const auto& t : program_model.get_trace()) { + ss << "thread " << t->get_executor() << ": " << t->to_string() << "\n"; + } + // See found_abnormal_termination()'s identical comment: `nzec.culprit` may + // no longer have a pending transition in the model's current view if this + // fired while `coordinator::return_to_depth()` was replaying history. + const transition* culprit_transition = + program_model.get_pending_transition_for(nzec.culprit); + if (culprit_transition != nullptr) { + ss << "thread " << culprit_transition->get_executor() << ": " + << culprit_transition->to_string() << "\n"; + } else { + ss << "thread " << nzec.culprit << ": (no longer pending)\n"; + } + + ss << "\nNEXT THREAD OPERATIONS\n"; + for (const auto& tpair : program_model.get_pending_transitions()) { + if (culprit_transition != nullptr && + tpair.first == culprit_transition->get_executor()) { + ss << "thread " << tpair.first << ": executing" + << "\n"; + } else { + ss << "thread " << tpair.first << ": " << tpair.second->to_string() + << "\n"; + } + } + ss << stats.total_transitions + 1 << " total transitions executed" + << "\n"; + std::cout << ss.str(); + std::cout.flush(); +} + void found_deadlock(const coordinator& c, const stats& stats) { std::cerr << "DEADLOCK" << std::endl; std::stringstream ss; @@ -204,6 +245,7 @@ void do_model_checking(const config& config) { c.deadlock = &found_deadlock; c.undefined_behavior = &found_undefined_behavior; c.abnormal_termination = &found_abnormal_termination; + c.nonzero_exit_code = &found_nonzero_exit_code; classic_dpor_checker.verify_using(coordinator, c); std::cout << "Model checking completed!" << std::endl; } @@ -328,6 +370,7 @@ void do_model_checking_from_dmtcp_ckpt_file(const config& config) { c.undefined_behavior = &found_undefined_behavior; c.deadlock = &found_deadlock; c.abnormal_termination = &found_abnormal_termination; + c.nonzero_exit_code = &found_nonzero_exit_code; classic_dpor_checker.verify_using(coordinator, c); std::cerr << "Deep debugging completed!" << std::endl; } diff --git a/src/mcmini/real_world/local_linux_process.cpp b/src/mcmini/real_world/local_linux_process.cpp index 9102b4ec..c784a7f8 100644 --- a/src/mcmini/real_world/local_linux_process.cpp +++ b/src/mcmini/real_world/local_linux_process.cpp @@ -133,9 +133,25 @@ volatile runner_mailbox *local_linux_process::execute_runner(runner_id_t id) { "process to abnormally exit (or possibly an internal error of " "McMini): " + std::string(strerror(errno))); } else if (WIFEXITED(status)) { - throw process::nonzero_exit_code_error( - WEXITSTATUS(status), id, - "The program exited with code " + std::to_string(WEXITSTATUS(status))); + const int exit_code = WEXITSTATUS(status); + if (exit_code != 0) { + throw process::nonzero_exit_code_error( + exit_code, id, "The program exited with code " + std::to_string(exit_code)); + } + // A clean (code 0) exit reaching this SIGCHLD-based detection path is + // not the same situation nonzero_exit_code_error reports: that + // exception means the *target program* exited abnormally, a bug for + // the user to fix. Here, the whole process fully terminated on its + // own while this runner still had a transition pending on it -- i.e. + // it bypassed the model-driven exit protocol (mc_transparent_exit(), + // fixed in commit 6bed56a, deliberately keeps the process alive + // across both of its mailbox rounds before ever calling the real + // exit(2)) rather than checking in normally. That is a McMini-side + // protocol violation to investigate, not a target-program bug. + throw process::execution_error( + "Runner " + std::to_string(id) + + "'s process exited normally (code 0) while a transition was " + "still pending on it, bypassing the model-driven exit protocol."); } else if (WIFSIGNALED(status)) { throw process::termination_error( WTERMSIG(status), id, From 0a7a5f9ad55e04f08db5d9944c11b982201a43f1 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Thu, 30 Jul 2026 23:43:22 -0400 Subject: [PATCH 12/16] Sem desync fix: switch to a plain futex word 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 --- doc/glibc-sem-desync.txt | 147 ++++++++++++++++++ .../real_world/mailbox/runner_mailbox.h | 15 +- src/common/runner_mailbox.c | 57 +++++-- 3 files changed, 202 insertions(+), 17 deletions(-) create mode 100644 doc/glibc-sem-desync.txt diff --git a/doc/glibc-sem-desync.txt b/doc/glibc-sem-desync.txt new file mode 100644 index 00000000..1498c1a4 --- /dev/null +++ b/doc/glibc-sem-desync.txt @@ -0,0 +1,147 @@ +glibc sem_t "nwaiters" desync, and why child_side_sem is a raw futex instead +============================================================================= + +What "glibc desync" means +-------------------------- +glibc's NPTL sem_t packs two things into one 64-bit atomic word: the actual +semaphore count, and `nwaiters` -- a count of how many threads are currently +blocked inside sem_wait(). sem_post() uses `nwaiters` purely as an +optimization: it increments the count, then checks `nwaiters`, and only +makes the FUTEX_WAKE syscall if `nwaiters > 0`. If `nwaiters == 0`, it +assumes nobody's waiting and skips the syscall entirely. + +"Desync" means `nwaiters` (glibc's userspace belief about who's waiting) no +longer matches the kernel's actual futex wait queue for that address. When +that happens, sem_post() can wrongly conclude "nobody is waiting" and +silently skip the wake -- even though a real thread is genuinely parked in +the kernel on that exact word. That is a lost wakeup, and the thread hangs +forever. + +Does this happen only across checkpoint-restart, or anywhere? +--------------------------------------------------------------- +Only in scenarios like McMini's, not in ordinary programs. + +In normal execution, `nwaiters` can never drift from reality, because the +only code that ever touches it is sem_wait()/sem_post() themselves, via +atomic read-modify-write on that same packed word -- it is a closed, +self-consistent system. The only way to break it is for something *outside* +the semaphore's own API to reset that memory (sem_destroy() + sem_init(), or +an equivalent memset) while a thread is still validly blocked on it -- which +POSIX explicitly documents as undefined behavior ("it is safe to destroy a +semaphore only once no thread is blocked on it"). + +Ordinary programs don't hit this because they only ever destroy a semaphore +once they've confirmed nothing is using it. McMini's situation is different: +DMTCP's checkpoint/restart transparently preserves a thread's real +kernel-level "I'm blocked in this exact FUTEX_WAIT" state across the +restart (that's just how checkpointing a blocked syscall works), while +McMini's own code (mc_runner_mailbox_init()/mc_runner_mailbox_destroy()) +separately, explicitly reinitializes that same shared-memory semaphore for +the new branch -- invisible to, and inconsistent with, a thread that (from +the kernel's point of view) never actually left its old wait. + +CRIU's own documentation names this exact rule: everything sharing a +futex/shared-memory region must be checkpointed and restored together as +one atomic unit. McMini's architecture -- a permanent, never-checkpointed +verifier sharing memory with a repeatedly-restarted target -- inherently +breaks that rule. + +So: this is not a general glibc footgun waiting anywhere in a normal +program; it is specific to externally reinitializing a semaphore's memory +while a checkpoint/restart mechanism has silently kept a thread genuinely +blocked on it underneath. Take away either half (no checkpoint/restart, or +no external reinitialization) and it cannot occur. + +The fix +-------- +Commit 1a4b3d9 ("Replace child_side_sem's glibc sem_t with a plain futex +word") replaces `child_side_sem` -- the mailbox semaphore a DMTCP-restored +target thread waits on, posted by the verifier -- with a bare futex word +that has no separate userspace bookkeeping to desync in the first place. + +Files and functions: + + include/mcmini/real_world/mailbox/runner_mailbox.h + - `child_side_sem` changed from `sem_t` to `uint32_t`. + + src/common/runner_mailbox.c + - `mc_futex()` -- thin wrapper around syscall(SYS_futex, ...). + - `mc_raw_sem_wait()` -- replaces sem_wait() on child_side_sem: spins + on an atomic compare-and-swap against the + counter, blocking via FUTEX_WAIT only when + the counter is 0. + - `mc_raw_sem_post()` -- replaces sem_post() on child_side_sem: + atomically increments the counter, then + calls FUTEX_WAKE *unconditionally* -- no + "is anyone really waiting" check, so there + is nothing to desync. + - `mc_wait_for_scheduler()` -- now calls mc_raw_sem_wait(). + - `mc_wake_thread()` -- now calls mc_raw_sem_post(). + - `mc_runner_mailbox_init()`/`mc_runner_mailbox_destroy()` -- initialize/ + no-op-destroy the futex word directly instead of calling sem_init()/ + sem_destroy() on it. + +`model_side_sem` (the verifier waits, the target posts) is untouched and +remains a real sem_t: the verifier process is never checkpointed, so its +side of the bookkeeping can never desync. + +Not yet fixed +--------------- +Condition variables (pthread_cond_wait/pthread_cond_signal) have the same +class of vulnerability via glibc's G1/G2 waiter-group bookkeeping, for the +same reason (checkpoint/restart + externally-managed reinitialization). An +analogous fix has not yet been applied there. + +Does this require pshared=1, or can it happen with pshared=0 too? +--------------------------------------------------------------------- +`pshared` is the second parameter of sem_init() (`int sem_init(sem_t *sem, +int pshared, unsigned int value)`): pshared=0 means the semaphore may only +be used among threads of the single process that created it; pshared=1 +means it may be shared across process boundaries (typically by placing it +in memory obtained via shm_open()/mmap(), as McMini does here). + +This bug requires pshared=1. It cannot happen with pshared=0, and the +reason follows directly from the mechanism above: the desync needs an +entity *outside the checkpointed unit* to reinitialize the semaphore's +memory while a thread inside that unit is still genuinely blocked on it. In +McMini's architecture that outside entity is the verifier (mcmini) -- a +separate, permanent, never-checkpointed process -- while the target (with +the blocked thread) gets checkpoint/restarted. DMTCP transparently +preserves the target's kernel-level block across the restart, but the +verifier's reinitialization of that shared memory has no way to know about, +or wait for, that survival, because it is not part of the same checkpoint +image at all. + +If pshared=0, the semaphore is only valid for use among threads of a +single process. DMTCP checkpoints and restores that whole process -- +every thread, and all of the semaphore's own memory (nwaiters and value +together) -- as one atomic, consistent snapshot. There is no outsider who +could reinitialize the memory independently of the blocked thread's +restore, because everything that touches it moves through the +checkpoint/restart together. + +Two more reasons this is specifically a pshared=1 problem: + + 1. Using a pshared=0 semaphore across two processes is undefined + behavior under POSIX regardless of checkpointing -- Linux's NPTL + implementation uses FUTEX_PRIVATE_FLAG for private (pshared=0) futex + operations, keyed off the process's own memory descriptor, which + specifically assumes same-process access. "verifier process + target + process share a pshared=0 semaphore" is not a valid configuration to + begin with. + 2. model_side_sem (still a real sem_t, pshared=1, in this same mailbox) + is the control case that proves the point: it is just as + cross-process-shared as child_side_sem was, but it is fine, because + the verifier's side of it is never checkpointed -- only the target + side (posting) is. The bug needs the *waiting* side to be the one + that gets checkpoint/restarted while the memory gets reinitialized + out from under it; that combination cannot arise for a purely + intra-process, pshared=0 semaphore. + +Separately: an ordinary program could still hit undefined behavior by +calling sem_destroy() on any semaphore -- pshared=0 or 1 -- while a thread +is genuinely blocked on it. That is always illegal per POSIX. But that is +a plain application bug, not "the checkpoint/restart desync": the +McMini-specific failure mode is that the reinitialization happens from a +vantage point that literally cannot see the blocked thread at all, which +is only possible across the process boundary that pshared=1 implies. diff --git a/include/mcmini/real_world/mailbox/runner_mailbox.h b/include/mcmini/real_world/mailbox/runner_mailbox.h index 8f864b55..d3bd644b 100644 --- a/include/mcmini/real_world/mailbox/runner_mailbox.h +++ b/include/mcmini/real_world/mailbox/runner_mailbox.h @@ -8,8 +8,21 @@ extern "C" { #include typedef struct { + // Waited on by the verifier (mcmini), posted by the target thread. The + // verifier is never checkpointed, so a plain glibc sem_t is safe here. sem_t model_side_sem; - sem_t child_side_sem; + // Waited on by the target thread, posted by the verifier. A DMTCP-restored + // target thread can be genuinely blocked (kernel-level) on this exact + // futex word while glibc's own userspace "is anyone really waiting" + // bookkeeping is desynced from that -- because this memory gets + // reinitialized (see mc_runner_mailbox_init()/_destroy()) before every new + // DMTCP-restarted branch, and glibc's NPTL sem_t packs that bookkeeping + // into the same memory sem_post() checks to decide whether to skip the + // underlying futex(FUTEX_WAKE) syscall. A plain futex word (see + // mc_wake_thread()/mc_wait_for_scheduler() in runner_mailbox.c, which + // always call FUTEX_WAKE unconditionally) has no such bookkeeping to + // desync. + uint32_t child_side_sem; uint32_t type; uint8_t cnts[64]; // TODO: How much space should each thread have to write // payloads? diff --git a/src/common/runner_mailbox.c b/src/common/runner_mailbox.c index e11af991..25d9ec57 100644 --- a/src/common/runner_mailbox.c +++ b/src/common/runner_mailbox.c @@ -1,22 +1,55 @@ #include "mcmini/real_world/mailbox/runner_mailbox.h" -#include +#include +#include +#include #include -#include "mcmini/lib/log.h" #include "mcmini/defines.h" #include "mcmini/spy/intercept/interception.h" #include "string.h" +// child_side_sem's own raw-futex protocol (see runner_mailbox.h for why it +// isn't a glibc sem_t): a plain futex word used as a counting semaphore, +// always waking unconditionally on post, so there's no userspace "is anyone +// really waiting" bookkeeping to desync. +static long mc_futex(volatile uint32_t *uaddr, int futex_op, uint32_t val) { + return syscall(SYS_futex, uaddr, futex_op, val, NULL, NULL, 0); +} + +static int mc_raw_sem_wait(volatile uint32_t *sem) { + while (1) { + uint32_t cur = __atomic_load_n(sem, __ATOMIC_SEQ_CST); + if (cur > 0) { + uint32_t expected = cur; + if (__atomic_compare_exchange_n(sem, &expected, cur - 1, 0, + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) { + return 0; + } + continue; + } + errno = 0; + long rc = mc_futex(sem, FUTEX_WAIT, 0); + if (rc == -1 && errno != EAGAIN && errno != EINTR) { + return -1; + } + } +} + +static int mc_raw_sem_post(volatile uint32_t *sem) { + __atomic_fetch_add(sem, 1, __ATOMIC_SEQ_CST); + mc_futex(sem, FUTEX_WAKE, 1); + return 0; +} + void mc_runner_mailbox_init(volatile runner_mailbox* r) { runner_mailbox_ref ref = (runner_mailbox_ref)(r); #ifdef MC_SHARED_LIBRARY libpthread_sem_init(&ref->model_side_sem, SEM_FLAG_SHARED, 0); - libpthread_sem_init(&ref->child_side_sem, SEM_FLAG_SHARED, 0); #else sem_init(&ref->model_side_sem, SEM_FLAG_SHARED, 0); - sem_init(&ref->child_side_sem, SEM_FLAG_SHARED, 0); #endif + __atomic_store_n(&ref->child_side_sem, 0, __ATOMIC_SEQ_CST); memset(ref->cnts, 0, sizeof(ref->cnts)); } @@ -24,11 +57,11 @@ void mc_runner_mailbox_destroy(volatile runner_mailbox* r) { runner_mailbox_ref ref = (runner_mailbox_ref)(r); #ifdef MC_SHARED_LIBRARY libpthread_sem_destroy(&ref->model_side_sem); - libpthread_sem_destroy(&ref->child_side_sem); #else sem_destroy(&ref->model_side_sem); - sem_destroy(&ref->child_side_sem); #endif + // child_side_sem is a plain futex word, not a glibc sem_t: nothing to + // destroy. } int mc_wait_for_thread(volatile runner_mailbox* r) { @@ -42,20 +75,12 @@ int mc_wait_for_thread(volatile runner_mailbox* r) { int mc_wait_for_scheduler(volatile runner_mailbox* r) { runner_mailbox_ref ref = (runner_mailbox_ref)(r); -#ifdef MC_SHARED_LIBRARY - return libpthread_sem_wait(&ref->child_side_sem); -#else - return sem_wait(&ref->child_side_sem); -#endif + return mc_raw_sem_wait(&ref->child_side_sem); } int mc_wake_thread(volatile runner_mailbox* r) { runner_mailbox_ref ref = (runner_mailbox_ref)(r); -#ifdef MC_SHARED_LIBRARY - return libpthread_sem_post(&ref->child_side_sem); -#else - return sem_post(&ref->child_side_sem); -#endif + return mc_raw_sem_post(&ref->child_side_sem); } int mc_wake_scheduler(volatile runner_mailbox* r) { From a74fb0b7de88086d67b782972adf5d6c82667a1e Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Tue, 28 Jul 2026 18:26:49 -0400 Subject: [PATCH 13/16] Fix restart-quiescence bypass on plain return A target whose main() returns via `return N;` (rather than an explicit exit()/pthread_exit() call) never checked in with mc_transparent_exit()'s restart-quiescence protocol: glibc's __libc_start_call_main calls exit() internally via the hidden __GI_exit alias, bypassing exit()'s own interposition entirely -- confirmed this also holds for _exit() one level deeper (test_implicit_exit_interposition.c). __libc_start_main()'s own call site, in contrast, is in _start (crt1.o, not glibc-internal code), so it resolves through ordinary dynamic symbol resolution and is interposable (test_libc_start_main_hook.c). Wraps the target's main with a version that, after the real main() returns, calls mc_transparent_exit() directly instead of returning to glibc's own (uninterposable) exit machinery -- treating a plain return exactly like an explicit exit(rc) call, in every mode. Verified live: producer-consumer-park(-tsan), whose main() now returns plainly instead of calling exit(0) explicitly (removing that previously- required workaround), correctly reaches INITIAL STATE after a --multithreaded-fork restart with all 3 threads represented (main's pending exit(2), both workers' pending sem_wait), finds the expected DEADLOCK, and completes cleanly -- instead of the previous "Failed to create a new process (template process died)" failure. Co-Authored-By: Claude Sonnet 5 --- src/examples/producer-consumer-park.c | 17 ++++------- src/lib/interception.c | 41 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/examples/producer-consumer-park.c b/src/examples/producer-consumer-park.c index b66420f9..67e0c26d 100644 --- a/src/examples/producer-consumer-park.c +++ b/src/examples/producer-consumer-park.c @@ -138,15 +138,10 @@ int main(int argc, char* argv[]) // Deliberately no pthread_join(): producer/consumer are still alive, // parked on `park`, when the process ends. - // Explicit exit() call, NOT `return 0;`: a plain return makes glibc's - // own __libc_start_call_main call __GI_exit -- a glibc-internal alias - // resolved at glibc's own compile time, invisible to ANY interposition - // technique (--wrap, LD_PRELOAD, or a strong-symbol override). Confirmed - // live via gdb: that's exactly what happens on a plain return. Since - // main's own restart-quiescence check-in (mc_transparent_exit(), which - // exit() routes to) never runs in that case, the model checker never - // sees main check in after a restart -- it just watches the whole - // process really exit out from under it. An explicit exit() call is a - // normal, interposable function call, so it checks in correctly. - exit(0); + // Plain return, deliberately NOT an explicit exit(0) call: this is now + // a live regression test for interception.c's __libc_start_main hook, + // which runs mc_transparent_exit() after main() returns however it + // returns, catching exactly this case -- see that file for why a plain + // return used to bypass every exit()/_exit() interposition technique. + return 0; } diff --git a/src/lib/interception.c b/src/lib/interception.c index 26c0cab1..0d29e955 100644 --- a/src/lib/interception.c +++ b/src/lib/interception.c @@ -308,3 +308,44 @@ int libpthread_sem_wait_loop(sem_t *sem) { rc = libpthread_sem_wait(sem); return rc; } + +// __libc_start_main() is called from _start (crt1.o), not glibc-internal +// code, so unlike exit()/_exit() -- whose implicit post-main() call chain +// resolves through glibc-internal hidden aliases that bypass any public- +// symbol interposition (see test/tsan_support/test_implicit_exit_interposition.c) +// -- this call site goes through ordinary dynamic symbol resolution and can +// be intercepted like any other public symbol. Used to guarantee +// mc_transparent_exit()'s exit-transition/restart-quiescence handling runs +// after the target's main() returns, even when it returns plainly +// (`return N;`) rather than calling exit()/pthread_exit() explicitly -- +// verified in test/tsan_support/test_libc_start_main_hook.c. +typedef int (*main_fn)(int, char **, char **); +typedef int (*libc_start_main_fn)(main_fn, int, char **, void (*)(void), + void (*)(void), void (*)(void), void *); + +static main_fn real_main; + +static int wrapped_main(int argc, char **argv, char **envp) { + int rc = real_main(argc, argv, envp); + // libmcmini_init() itself is safe to call any time after main() has run + // (pthreads are certainly initialized by now), unlike at the top of + // __libc_start_main() below, which runs before glibc's own internal + // pthread-subsystem setup and must stay free of any pthread_once/mutex + // use until the real __libc_start_main() has had a chance to run. + libmcmini_init(); + // Route a plain return from main() through the exact same model-checking + // exit protocol as an explicit exit(rc) call: mc_transparent_exit() + // itself performs the real, final process termination in every mode + // (see wrappers.c), so this call never returns. + mc_transparent_exit(rc); +} + +int __libc_start_main(main_fn main, int argc, char **argv, void (*init)(void), + void (*fini)(void), void (*rtld_fini)(void), + void *stack_end) { + real_main = main; + libc_start_main_fn real_start_main = + (libc_start_main_fn)dlsym(RTLD_NEXT, "__libc_start_main"); + return real_start_main(wrapped_main, argc, argv, init, fini, rtld_fini, + stack_end); +} From 0440193031bda182ac4e2b9ed880df7b8bfc7e00 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Tue, 28 Jul 2026 13:38:08 -0400 Subject: [PATCH 14/16] Fix CV restart deadlock: mutex drops location condition_variable_enqueue_thread::modify() also transitions the mutex to unlocked (cond_wait atomically releases it), but did so via mutex(state) -- the 1-arg constructor, which has no default member initializer for `location` (unlike condition_variable's policy field), so it was left completely uninitialized. Every later mutex_lock/unlock faithfully forwards whatever garbage ms->get_location() reads back, corrupting the mutex's identity for the rest of the run. This is what made condition_variable_wait::modify()'s `m->get_location() == cv->get_mutex()` check fail forever, even after the earlier policy-cloning and mutex-association fixes -- traced by adding a diagnostic directly at state_sequence::follow()'s commit point, the one unambiguous place that distinguishes a genuine commit from transition::is_enabled_in()'s throwaway speculative check. Fixed the same way as condition_variable_signal's associated_mutex fix: pass the existing location through explicitly. Also fixed the identical pattern in mutex_init::modify() and its callback (mutex.cpp) for consistency, though neither is exercised by the CV repro that found this (mutex_init only ever runs once, before any checkpoint, in every target tested so far). With this fix, a checkpoint taken mid-pthread_cond_wait() finally restarts and completes correctly under --multithreaded-fork: DPOR explores all 8 valid interleavings of a producer/consumer/main scenario with zero deadlocks, down from an immediate false DEADLOCK before this session's whole condition-variable investigation started. The core diff_state/state_sequence replay machinery investigated along the way (element indexing, slice()/consume_into_subsequence(), follow()'s commit path) turned out to be architecturally sound -- every symptom traced back to CV/mutex-specific constructors, not the shared state machinery itself. Regression-checked clean against all 6 existing TSan targets. --- .../condition_variable_enqueue_thread.hpp | 10 ++++++++-- include/mcmini/model/transitions/mutex/mutex_init.hpp | 9 ++++++++- src/mcmini/model/transitions/mutex.cpp | 3 ++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/include/mcmini/model/transitions/condition_variables/condition_variable_enqueue_thread.hpp b/include/mcmini/model/transitions/condition_variables/condition_variable_enqueue_thread.hpp index 73f2b1b1..e0acf25d 100644 --- a/include/mcmini/model/transitions/condition_variables/condition_variable_enqueue_thread.hpp +++ b/include/mcmini/model/transitions/condition_variables/condition_variable_enqueue_thread.hpp @@ -40,9 +40,15 @@ struct condition_variable_enqueue_thread : public model::transition{ cv->get_policy()->add_waiter_with_state(executor, CV_WAITING); const int new_waiting_count = cv->get_policy()->return_wait_queue().size(); - + s.add_state_for_obj(cond_id, new condition_variable(condition_variable::cv_waiting, executor, m->get_location(), new_waiting_count)); - s.add_state_for_obj(mutex_id, new mutex(mutex::unlocked)); + // Preserve the mutex's location: mutex(state) (1-arg) has no default + // member initializer for `location` (unlike condition_variable's + // policy), so it's left completely uninitialized -- every later + // mutex_lock/unlock just faithfully forwards whatever garbage this + // leaves behind via ms->get_location(), corrupting the mutex's identity + // for the rest of the run. + s.add_state_for_obj(mutex_id, new mutex(mutex::unlocked, m->get_location())); return status::exists; } state::objid_t get_id() const { return this->cond_id; } diff --git a/include/mcmini/model/transitions/mutex/mutex_init.hpp b/include/mcmini/model/transitions/mutex/mutex_init.hpp index c664b2af..25dd1386 100644 --- a/include/mcmini/model/transitions/mutex/mutex_init.hpp +++ b/include/mcmini/model/transitions/mutex/mutex_init.hpp @@ -17,7 +17,14 @@ struct mutex_init : public model::transition { status modify(model::mutable_state& s) const override { using namespace model::objects; - s.add_state_for_obj(mutex_id, new mutex(mutex::unlocked)); + // Preserve the location set on the placeholder object created by + // mutex_init_callback (see mutex.cpp) -- the 1-arg mutex(state) + // constructor used below has no default member initializer for + // `location`, so it would otherwise be left uninitialized, corrupting + // every later mutex_lock/unlock that faithfully forwards whatever + // garbage ms->get_location() reads back afterward. + const mutex* ms = s.get_state_of_object(mutex_id); + s.add_state_for_obj(mutex_id, new mutex(mutex::unlocked, ms->get_location())); return status::exists; } state::objid_t get_id() const { return this->mutex_id; } diff --git a/src/mcmini/model/transitions/mutex.cpp b/src/mcmini/model/transitions/mutex.cpp index 1757a7e7..72cd074d 100644 --- a/src/mcmini/model/transitions/mutex.cpp +++ b/src/mcmini/model/transitions/mutex.cpp @@ -14,7 +14,8 @@ model::transition* mutex_init_callback(runner_id_t p, // Locate the corresponding model of this object if (!m.contains(remote_mut)) - m.observe_object(remote_mut, new mutex(mutex::state::uninitialized)); + m.observe_object(remote_mut, + new mutex(mutex::state::uninitialized, remote_mut)); state::objid_t const mut = m.get_model_of_object(remote_mut); return new transitions::mutex_init(p, mut); From fc8f26226dbec427b8cf1463ba19c1d111644c65 Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Thu, 30 Jul 2026 23:16:27 -0400 Subject: [PATCH 15/16] Fix mc_pthread_cond_wait's restart double-call abort Its restart case called the one-time-per-thread thread_handle_after_dmtcp_restart() twice; the second call aborts once mode has already advanced past DMTCP_RESTART_INTO_BRANCH/ TEMPLATE (confirmed via dmesg: real SIGABRT). Use the ordinary thread_wake_scheduler_and_wait() for the second round instead, matching every other wrapper's second-and-later round with the coordinator. --- src/lib/wrappers.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/lib/wrappers.c b/src/lib/wrappers.c index f31019ee..a6a1373d 100644 --- a/src/lib/wrappers.c +++ b/src/lib/wrappers.c @@ -1012,7 +1012,18 @@ int mc_pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) { mb->type = COND_WAIT_TYPE; memcpy_v(mb->cnts, &cond, sizeof(cond)); memcpy_v(mb->cnts + sizeof(cond), &mutex, sizeof(mutex)); - thread_handle_after_dmtcp_restart(); + // thread_handle_after_dmtcp_restart() (used for the COND_ENQUEUE_TYPE + // round above) is a one-time-per-thread restart check-in: it captures + // a getcontext() snapshot and posts the quiescence barrier that the + // background template thread waits on exactly once per real thread. By + // the time this second round runs, that barrier has already released + // and set mode to TARGET_BRANCH_AFTER_RESTART, so a second call falls + // into thread_handle_after_dmtcp_restart()'s `default: libc_abort()` + // (its mode_on_entry no longer matches DMTCP_RESTART_INTO_BRANCH/ + // TEMPLATE) -- confirmed live via dmesg showing a SIGABRT here. Use the + // ordinary wake+wait call instead, matching every other wrapper's + // second-and-later round with the coordinator. + thread_wake_scheduler_and_wait(); libpthread_mutex_lock(mutex); return 0; } From 9d6404246f8db81b27af834c54e6cbb560bf816e Mon Sep 17 00:00:00 2001 From: Gene Cooperman Date: Tue, 28 Jul 2026 14:00:48 -0400 Subject: [PATCH 16/16] Harden mutex constructor against dropped location Removed the 1-arg mutex(state) and 2-arg mutex(state, location) constructors -- confirmed unused after the recent fixes (every real call site already passes location) -- leaving one 3-arg constructor with location mandatory and tid defaulted to RID_INVALID. This turns "forgot to pass location" from a silent, uninitialized-memory correctness bug (as fixed in 9bd9ecf) into a compile error, and lets mutex_init/enqueue_thread/unlock express "no specific owner" without passing a misleading literal 0 (a real, valid runner id) or leaving `owner` uninitialized. Regression-checked clean against all 6 existing TSan targets. --- include/mcmini/model/objects/mutex.hpp | 14 +++++++++++--- .../model/transitions/mutex/mutex_unlock.hpp | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/mcmini/model/objects/mutex.hpp b/include/mcmini/model/objects/mutex.hpp index 66789238..91d2fbac 100644 --- a/include/mcmini/model/objects/mutex.hpp +++ b/include/mcmini/model/objects/mutex.hpp @@ -1,5 +1,6 @@ #pragma once +#include "mcmini/defines.h" #include "mcmini/misc/extensions/unique_ptr.hpp" #include "mcmini/model/visible_object_state.hpp" @@ -20,9 +21,16 @@ struct mutex : public model::visible_object_state { mutex() = default; ~mutex() = default; mutex(const mutex &) = default; - mutex(state s) : current_state(s) {} - mutex(state s, pthread_mutex_t* loc) : current_state(s), location(loc) {} - mutex(state s, pthread_mutex_t* loc, runner_id_t tid): current_state(s), location(loc), owner(tid) {} + // `location` has no default member initializer (unlike, say, + // condition_variable's `policy`), so it must always be passed explicitly + // -- a mutex without its real address is meaningless, and every + // mutex_lock/unlock faithfully forwards whatever get_location() returns, + // so an uninitialized one corrupts every subsequent state derived from + // it (see doc history around commit 9bd9ecf). `tid` defaults to + // RID_INVALID: "unlocked, no owner" is exactly what callers that omit it + // (mutex_init, condition_variable_enqueue_thread) mean. + mutex(state s, pthread_mutex_t* loc, runner_id_t tid = RID_INVALID) + : current_state(s), location(loc), owner(tid) {} // ---- State Observation --- // bool operator==(const mutex &other) const { diff --git a/include/mcmini/model/transitions/mutex/mutex_unlock.hpp b/include/mcmini/model/transitions/mutex/mutex_unlock.hpp index b100089a..4b50aabd 100644 --- a/include/mcmini/model/transitions/mutex/mutex_unlock.hpp +++ b/include/mcmini/model/transitions/mutex/mutex_unlock.hpp @@ -25,7 +25,7 @@ struct mutex_unlock : public model::transition { return status::disabled; } - s.add_state_for_obj(mutex_id, new mutex(mutex::unlocked, ms->get_location(), 0)); + s.add_state_for_obj(mutex_id, new mutex(mutex::unlocked, ms->get_location())); return status::exists; } state::objid_t get_id() const { return this->mutex_id; }