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