WIP: TSAN checkpoint restart

[gc00]

Still working on it with Claude, but mostly ready to start being reviewed. This includes the commits of PR #8.

Summary by CodeRabbit

• New Features
  • Enhanced ThreadSanitizer multithreaded fork/restart support with sanitizer-aware thread handling and safer pthread exit/join behavior for recreated threads.
• Bug Fixes
  • Reduced ThreadSanitizer-enabled hangs and crashes by improving fork barrier behavior and thread counting.
• Documentation
  • Added phased TSAN port design/implementation and hardening plans for the multithreaded fork workflow.
• Tests
  • Added standalone coverage for signal-block detection, thread-id extraction, fork fast-path behavior, pthread exit forwarding, and repeated-fork stress.
• Compatibility / API Updates
  • Updated DMTCP plugin API version and checkpoint image header format to v4.0.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds TSan-safe support for multithreaded fork/restart, including a TSAN-safe allocator, signal-based thread classification, fork/restart fiber handling, pthread_exit interception, DMTCP header and pid API updates, and matching plans, specs, and tests.

Changes

TSan Port Implementation

Layer / File(s)	Summary
Allocator and signal classifier src/common/mem.c, include/mcmini/mem.h, include/mcmini/spy/checkpointing/record.h, src/lib/record.c, include/mcmini/spy/checkpointing/tsan_support.h, src/lib/tsan_support.c, test/tsan_support/test_thread_blocks_signal.c, CMakeLists.txt	Adds mc_ts_alloc, a TSAN-safe record-list append helper, and thread_blocks_signal() with standalone coverage.
Restart barrier classification include/dmtcp.h, src/common/multithreaded_fork.c, src/lib/dmtcp-callback.c, include/mcmini/spy/intercept/wrappers.h, test/tsan_support/test_tid_from_descriptor_offset.c	Updates checkpoint/pid declarations, adds the read-only pthread-descriptor TID helper, classifies restart-barrier threads per TID, and switches internal template-thread creation to public pthread_create.
Fork and fiber restart path src/lib/dmtcp-callback.c, test/tsan_support/test_fastpath_fork_clone_fiber.c, test/tsan_support/test_repeated_fork_stress.c	Adds TSAN fork/fiber hooks, uses __clone in restart thread recreation, and switches forked/resurrected threads onto fresh TSAN fibers, with standalone harnesses for the fast path and repeated-fork stress.
pthread_exit interception src/lib/interception.c, src/lib/wrappers.c, include/mcmini/spy/intercept/interception.h, include/mcmini/spy/intercept/wrappers.h, test/tsan_support/test_pthread_exit_forwarding.c	Adds mc_pthread_exit, the real pthread_exit forwarding path, TSan-safe timed-join bypassing, and the pthread-map synchronization changes.
Plans and specs PLAN.txt, docs/superpowers/plans/*, docs/superpowers/specs/*	Adds Phase 1–4 design and implementation documents for the TSAN multithreaded-fork port.
Model/header cleanups include/mcmini/model/pending_transitions.hpp, include/mcmini/model/state.hpp, include/mcmini/model_checking/algorithms/classic_dpor/stack_item.hpp	Adds pending_transitions::empty(), un-hides base overloads in mutable_state, and reorders an include list.

Estimated code review effort: 4 (Complex) | ~75 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is related to the TSAN checkpoint/restart work, but it is too vague to convey the main change.	Use a more specific title such as "Add TSAN-safe checkpoint/restart support" or mention the key fix, e.g. "TSAN checkpoint restart: add libc_clone and pthread_exit handling".

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch tsan-checkpoint-restart

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/lib/tsan_support.c (1)

5-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Validate signo range before shifting.

sigblk >> (signo - 1) (Line 27) is undefined behavior if signo <= 0 or signo > 64 (shift by negative or by ≥ width of unsigned long long). The header doesn't document a valid range, and this is a general-purpose public API, so a future caller passing an out-of-range value would hit UB. Current call site passes a fixed macro, so this isn't triggered today, but a guard is cheap insurance.

🔧 Proposed fix

 int thread_blocks_signal(pid_t tid, int signo) {
+  if (signo < 1 || signo > 64) {
+    return 0;
+  }
   char path[64];

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/tsan_support.c` around lines 5 - 28, Validate the signo argument in
thread_blocks_signal before using it in the bit shift; the current return
expression can invoke undefined behavior for signo values outside 1..64. Add a
range check near the start of thread_blocks_signal, and return 0 for invalid
inputs before reading or shifting sigblk so the helper remains safe for all
callers.

src/common/mem.c (1)

9-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Arena isn't guaranteed to be 16-byte aligned.

mc_ts_alloc rounds n to 16-byte boundaries (Line 14) implying a malloc-like alignment guarantee, but mc_ts_arena is a plain char[] with no explicit alignment. The C standard only guarantees 1-byte alignment for char arrays; nothing forces the base address onto a 16-byte boundary, so offsets computed relative to it aren't guaranteed absolute-aligned. This could produce misaligned pointers for stricter-alignment consumers on some platforms/compilers.

🔧 Proposed fix

-static char mc_ts_arena[MC_TS_ARENA_SIZE];
+static _Alignas(16) char mc_ts_arena[MC_TS_ARENA_SIZE];

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/common/mem.c` around lines 9 - 11, The arena backing `mc_ts_alloc` is not
guaranteed to start on a 16-byte boundary, so the alignment promise made by
rounding allocations up in `mc_ts_alloc` is incomplete. Update the `mc_ts_arena`
storage in `mem.c` so the arena itself has explicit 16-byte alignment, and keep
the existing size/offset logic in `mc_ts_alloc` consistent with that guarantee.
Reference the `MC_TS_ARENA_SIZE`, `mc_ts_arena`, and `mc_ts_alloc` symbols when
making the change.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md`:
- Around line 104-107: The harness snippet in setTLSPointer is calling
arch_prctl with the wrong syscall shape and an inverted success check, so update
it to use the direct ARCH_SET_FS and ARCH_SET_GS usage on ti->fs and ti->gs and
make the assertions validate success with == 0. Keep the fix localized to
setTLSPointer so the sample reaches the TSan path as intended.

In `@src/lib/wrappers.c`:
- Around line 468-496: The RECORD/PRE_CHECKPOINT path in mc_pthread_exit
currently forwards directly to libpthread_pthread_exit without updating the
thread’s rec_list status, leaving an exited thread marked ALIVE. Update
mc_pthread_exit so that, before calling libpthread_pthread_exit in the RECORD
and PRE_CHECKPOINT cases, it follows the same rec_list_lock / thread-record
lookup pattern used by mc_thread_routine_wrapper and sets thrd_state.status to
EXITED for the current thread. Keep the restart and TARGET_BRANCH paths
unchanged.

---

Nitpick comments:
In `@src/common/mem.c`:
- Around line 9-11: The arena backing `mc_ts_alloc` is not guaranteed to start
on a 16-byte boundary, so the alignment promise made by rounding allocations up
in `mc_ts_alloc` is incomplete. Update the `mc_ts_arena` storage in `mem.c` so
the arena itself has explicit 16-byte alignment, and keep the existing
size/offset logic in `mc_ts_alloc` consistent with that guarantee. Reference the
`MC_TS_ARENA_SIZE`, `mc_ts_arena`, and `mc_ts_alloc` symbols when making the
change.

In `@src/lib/tsan_support.c`:
- Around line 5-28: Validate the signo argument in thread_blocks_signal before
using it in the bit shift; the current return expression can invoke undefined
behavior for signo values outside 1..64. Add a range check near the start of
thread_blocks_signal, and return 0 for invalid inputs before reading or shifting
sigblk so the helper remains safe for all callers.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b898a1c1-f28e-4c7e-9165-d3136b2322cd

📥 Commits

Reviewing files that changed from the base of the PR and between 7a2d504 and cfba39d.

📒 Files selected for processing (27)

• CMakeLists.txt
• docs/superpowers/plans/2026-07-04-tsan-port-phase1-r1-implementation.md
• docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md
• docs/superpowers/plans/2026-07-05-tsan-port-phase3-r5-implementation.md
• docs/superpowers/specs/2026-07-04-tsan-port-phase1-r1-design.md
• docs/superpowers/specs/2026-07-04-tsan-port-phase2-r2-r3-r4-design.md
• docs/superpowers/specs/2026-07-05-tsan-port-phase3-r5-design.md
• include/dmtcp.h
• include/mcmini/mem.h
• include/mcmini/model/pending_transitions.hpp
• include/mcmini/model/state.hpp
• include/mcmini/model_checking/algorithms/classic_dpor/stack_item.hpp
• include/mcmini/spy/checkpointing/record.h
• include/mcmini/spy/checkpointing/tsan_support.h
• include/mcmini/spy/intercept/interception.h
• include/mcmini/spy/intercept/wrappers.h
• src/common/mem.c
• src/common/multithreaded_fork.c
• src/lib/dmtcp-callback.c
• src/lib/interception.c
• src/lib/record.c
• src/lib/tsan_support.c
• src/lib/wrappers.c
• test/tsan_support/test_fastpath_fork_clone_fiber.c
• test/tsan_support/test_pthread_exit_forwarding.c
• test/tsan_support/test_thread_blocks_signal.c
• test/tsan_support/test_tid_from_descriptor_offset.c

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the reviewed file around the referenced lines.
file='docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md'
wc -l "$file"
sed -n '90,120p' "$file"

# Find any arch_prctl usage in the repo for comparison.
rg -n "arch_prctl|ARCH_SET_FS|ARCH_SET_GS|SYS_arch_prctl" -S .

Repository: mcminickpt/deep-debug

Length of output: 268

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Determine syscall convention from system headers/manpage if available in sandbox.
python3 - <<'PY'
import os, subprocess, textwrap, sys
cands = [
    "/usr/include/x86_64-linux-gnu/sys/syscall.h",
    "/usr/include/sys/syscall.h",
    "/usr/include/x86_64-linux-gnu/asm/prctl.h",
    "/usr/include/asm/prctl.h",
]
for p in cands:
    if os.path.exists(p):
        print(f"--- {p} ---")
        with open(p, 'r', encoding='utf-8', errors='ignore') as f:
            for i, line in enumerate(f, 1):
                if "ARCH_SET_FS" in line or "ARCH_SET_GS" in line or "SYS_arch_prctl" in line or "arch_prctl" in line:
                    print(f"{i}:{line.rstrip()}")
PY

Repository: mcminickpt/deep-debug

Length of output: 323

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Programmatically check the expected syscall arity/signature from headers/examples if present.
python3 - <<'PY'
import os, re

paths = [
    "/usr/include/x86_64-linux-gnu/asm/prctl.h",
    "/usr/include/asm/prctl.h",
    "/usr/include/x86_64-linux-gnu/sys/syscall.h",
    "/usr/include/sys/syscall.h",
]
for path in paths:
    if not os.path.exists(path):
        continue
    print(f"\n== {path} ==")
    with open(path, 'r', encoding='utf-8', errors='ignore') as f:
        for line in f:
            if any(tok in line for tok in ("ARCH_SET_FS", "ARCH_SET_GS", "SYS_arch_prctl", "arch_prctl")):
                print(line.rstrip())
PY

Repository: mcminickpt/deep-debug

Length of output: 317

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the document and inspect nearby lines in the actual file.
git ls-files 'docs/**' | rg 'tsan-port-phase2|implementation\.md|r2-r3-r4'

Repository: mcminickpt/deep-debug

Length of output: 159

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the document by name or unique path fragments.
find docs -type f | rg 'tsan|phase2|r2|r3|r4|implementation\.md'

Repository: mcminickpt/deep-debug

Length of output: 199

---

Fix the arch_prctl calls in the harness snippet.
syscall(SYS_arch_prctl, 2, ARCH_SET_FS, ti->fs) passes the wrong argument shape and assert(... != 0) inverts success, so this sample fails before it reaches the TSan path. Use the direct ARCH_SET_FS / ARCH_SET_GS calls with == 0 on success.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs/superpowers/plans/2026-07-04-tsan-port-phase2-r2-r3-r4-implementation.md`
around lines 104 - 107, The harness snippet in setTLSPointer is calling
arch_prctl with the wrong syscall shape and an inverted success check, so update
it to use the direct ARCH_SET_FS and ARCH_SET_GS usage on ti->fs and ti->gs and
make the assertions validate success with == 0. Keep the fix localized to
setTLSPointer so the sample reaches the TSan path as intended.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

mc_pthread_exit's RECORD-mode path doesn't mark the thread's rec_list entry EXITED.

Compare with mc_thread_routine_wrapper's normal-return exit path (RECORD/PRE_CHECKPOINT case), which explicitly locks rec_list_lock, finds the thread's record, and sets thrd_state.status = EXITED before returning. Here, when a thread calls pthread_exit() explicitly in RECORD/PRE_CHECKPOINT mode, control forwards straight to libpthread_pthread_exit(retval) and never updates the record. The status only becomes correct later if/when mc_pthread_join succeeds on that thread — until then, any code inspecting rec_list sees a stale ALIVE status for an already-exited thread.

🐛 Proposed fix: mark EXITED before forwarding in RECORD/PRE_CHECKPOINT

 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: {
+    case PRE_DMTCP_INIT:
+    case PRE_CHECKPOINT_THREAD:
+    case CHECKPOINT_THREAD: {
+      libpthread_pthread_exit(retval);
+    }
+    case RECORD:
+    case PRE_CHECKPOINT: {
+      libpthread_mutex_lock(&rec_list_lock);
+      rec_list *thread_record = find_thread_record_mode(pthread_self());
+      assert(thread_record != NULL);
+      thread_record->vo.thrd_state.status = EXITED;
+      libpthread_mutex_unlock(&rec_list_lock);
       libpthread_pthread_exit(retval);
     }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	MCMINI_NO_RETURN void mc_pthread_exit(void *retval) {
	switch (get_current_mode()) {
	case PRE_DMTCP_INIT:
	case PRE_CHECKPOINT_THREAD:
	case CHECKPOINT_THREAD:
	case RECORD:
	case PRE_CHECKPOINT: {
	libpthread_pthread_exit(retval);
	}
	case DMTCP_RESTART_INTO_BRANCH:
	case DMTCP_RESTART_INTO_TEMPLATE: {
	thread_get_mailbox()->type = THREAD_EXIT_TYPE;
	thread_handle_after_dmtcp_restart();
	// Fallthrough
	}
	case TARGET_BRANCH:
	case TARGET_BRANCH_AFTER_RESTART: {
	if (tid_self == RID_MAIN_THREAD) {
	mc_exit_main_thread_in_child();
	} else {
	mc_exit_thread_in_child();
	}
	}
	default: {
	libc_abort();
	}
	}
	}
	MCMINI_NO_RETURN void mc_pthread_exit(void *retval) {
	switch (get_current_mode()) {
	case PRE_DMTCP_INIT:
	case PRE_CHECKPOINT_THREAD:
	case CHECKPOINT_THREAD: {
	libpthread_pthread_exit(retval);
	}
	case RECORD:
	case PRE_CHECKPOINT: {
	libpthread_mutex_lock(&rec_list_lock);
	rec_list *thread_record = find_thread_record_mode(pthread_self());
	assert(thread_record != NULL);
	thread_record->vo.thrd_state.status = EXITED;
	libpthread_mutex_unlock(&rec_list_lock);
	libpthread_pthread_exit(retval);
	}
	case DMTCP_RESTART_INTO_BRANCH:
	case DMTCP_RESTART_INTO_TEMPLATE: {
	thread_get_mailbox()->type = THREAD_EXIT_TYPE;
	thread_handle_after_dmtcp_restart();
	// Fallthrough
	}
	case TARGET_BRANCH:
	case TARGET_BRANCH_AFTER_RESTART: {
	if (tid_self == RID_MAIN_THREAD) {
	mc_exit_main_thread_in_child();
	} else {
	mc_exit_thread_in_child();
	}
	}
	default: {
	libc_abort();
	}
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/wrappers.c` around lines 468 - 496, The RECORD/PRE_CHECKPOINT path in
mc_pthread_exit currently forwards directly to libpthread_pthread_exit without
updating the thread’s rec_list status, leaving an exited thread marked ALIVE.
Update mc_pthread_exit so that, before calling libpthread_pthread_exit in the
RECORD and PRE_CHECKPOINT cases, it follows the same rec_list_lock /
thread-record lookup pattern used by mc_thread_routine_wrapper and sets
thrd_state.status to EXITED for the current thread. Keep the restart and
TARGET_BRANCH paths unchanged.

[coderabbitai]

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@docs/superpowers/plans/2026-07-05-tsan-port-phase4-hardening-implementation.md`:
- Around line 271-309: The replacement fenced snippets in the plan update are
missing explicit language tags, which will keep markdownlint MD040 failing.
Update the affected fenced blocks in the document to use the appropriate labels
from the surrounding content, such as c for code snippets, bash for shell
commands, and text for prose-only replacements. Use the existing snippet
locations around the D7 and Q4 replacements to apply the fence labels
consistently.
- Around line 358-365: The DMTCP verification command in the TSAN hardening plan
is missing the required ASLR suppression, so update the launch sequence to run
the existing dmtcp_launch invocation through setarch -R. Keep the current
TSAN_OPTIONS and plugin arguments intact, and adjust the documented command so
the producer-consumer-tsan run is executed from the build directory with ASLR
disabled as required by the plan.
- Around line 103-106: The TLS restore logic in setTLSPointer is calling
syscall(SYS_arch_prctl, ...) with an extra argument and the assertion is
inverted, so the arch_prctl setup will fail. Update the setTLSPointer helper to
use the correct syscall signature for ARCH_SET_FS and ARCH_SET_GS with just the
code and pointer argument, and assert for a zero return value to confirm
success. Keep the fix localized to setTLSPointer so TLS restoration works
correctly.

In `@test/tsan_support/test_repeated_fork_stress.c`:
- Around line 106-111: The child setup in the repeated fork stress test does not
guard against allocation or clone failure, which can leave the worker path
broken. In the setup around malloc, __clone, and child_setcontext_fast, add
explicit checks for a null malloc result and a failed __clone() return, then
abort or exit the test immediately on either failure so the completion path is
never skipped.
- Around line 23-30: In test_repeated_fork_stress.c, the fork/wait handling can
fall through on _Fork() or waitpid() failure and then decode an uninitialized
status value. Update the control flow around _Fork(), waitpid(), and the
WIFEXITED/WEXITSTATUS checks so the parent only inspects status after a
successful waitpid and bails out or retries cleanly when _Fork() returns -1 or
waitpid() is interrupted/fails.
- Around line 173-178: The worker launch and synchronization in the repeated
fork stress test need error handling and interrupt-safe waits. Update the thread
startup loop around pthread_create in the worker setup to detect creation
failures and fail fast instead of continuing into sem_wait deadlock, and replace
the raw sem_wait calls in this function (including the check-in and done gate
waits) with the existing retry wrapper used elsewhere so EINTR does not let the
fork/done phase advance prematurely.
- Around line 71-74: The TLS setup in setTLSPointer is calling
syscall(SYS_arch_prctl, ...) with an extra bogus argument and the assertion is
inverted, so the arch_prctl operation is never applied correctly. Update the
setTLSPointer helper to use the real arch_prctl calling convention with the
proper ARCH_SET_FS/ARCH_SET_GS arguments and make the assertions check for
success, ensuring TLS is restored before setcontext().

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ada950e5-44ca-460f-9514-02e9e65b1a89

📥 Commits

Reviewing files that changed from the base of the PR and between cfba39d and faf71a1.

📒 Files selected for processing (5)

• PLAN.txt
• docs/superpowers/plans/2026-07-05-tsan-port-phase4-hardening-implementation.md
• docs/superpowers/specs/2026-07-05-tsan-port-phase4-hardening-design.md
• src/lib/dmtcp-callback.c
• test/tsan_support/test_repeated_fork_stress.c

✅ Files skipped from review due to trivial changes (2)

• docs/superpowers/specs/2026-07-05-tsan-port-phase4-hardening-design.md
• PLAN.txt

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/dmtcp-callback.c

[coderabbitai]

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant plan section with line numbers
sed -n '90,115p' docs/superpowers/plans/2026-07-05-tsan-port-phase4-hardening-implementation.md | cat -n

# Find other arch_prctl usages in the repo for comparison
rg -n "arch_prctl|ARCH_SET_FS|ARCH_SET_GS|SYS_arch_prctl" .

# If available, show the syscall prototype from local headers
python3 - <<'PY'
import os, glob
paths = [
    "/usr/include/x86_64-linux-gnu/sys/syscall.h",
    "/usr/include/sys/syscall.h",
    "/usr/include/asm/prctl.h",
    "/usr/include/x86_64-linux-gnu/asm/prctl.h",
]
for p in paths:
    if os.path.exists(p):
        print(f"\n== {p} ==")
        with open(p, "r", errors="ignore") as f:
            for i, line in enumerate(f, 1):
                if "ARCH_SET_FS" in line or "ARCH_SET_GS" in line or "SYS_arch_prctl" in line:
                    print(f"{i}:{line.rstrip()}")
PY

Repository: mcminickpt/deep-debug

Length of output: 4807

---

Fix the arch_prctl restore call.

syscall(SYS_arch_prctl, ...) takes (code, addr) and returns 0 on success. The current call shifts the arguments and asserts on failure, so TLS restoration will break.

Suggested fix

 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);
+  assert(syscall(SYS_arch_prctl, ARCH_SET_FS, ti->fs) == 0);
+  assert(syscall(SYS_arch_prctl, ARCH_SET_GS, ti->gs) == 0);
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	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 void setTLSPointer(struct threadinfo *ti) {
	assert(syscall(SYS_arch_prctl, ARCH_SET_FS, ti->fs) == 0);
	assert(syscall(SYS_arch_prctl, ARCH_SET_GS, ti->gs) == 0);
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs/superpowers/plans/2026-07-05-tsan-port-phase4-hardening-implementation.md`
around lines 103 - 106, The TLS restore logic in setTLSPointer is calling
syscall(SYS_arch_prctl, ...) with an extra argument and the assertion is
inverted, so the arch_prctl setup will fail. Update the setTLSPointer helper to
use the correct syscall signature for ARCH_SET_FS and ARCH_SET_GS with just the
code and pointer argument, and assert for a zero return value to confirm
success. Keep the fix localized to setTLSPointer so TLS restoration works
correctly.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add language tags to the replacement fences.

The four snippets in this block are missing fenced-code languages, so markdownlint MD040 will keep failing here. Please mark them explicitly (c, bash, or text as appropriate).

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 273-273: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

[warning] 281-281: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

[warning] 298-298: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

[warning] 305-305: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs/superpowers/plans/2026-07-05-tsan-port-phase4-hardening-implementation.md`
around lines 271 - 309, The replacement fenced snippets in the plan update are
missing explicit language tags, which will keep markdownlint MD040 failing.
Update the affected fenced blocks in the document to use the appropriate labels
from the surrounding content, such as c for code snippets, bash for shell
commands, and text for prose-only replacements. Use the existing snippet
locations around the D7 and Q4 replacements to apply the fence labels
consistently.

Source: Linters/SAST tools

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Wrap the DMTCP launch with setarch -R too.

This step launches a TSan-instrumented target without the ASLR suppression required by the plan's own global constraint, so the documented verification run can diverge or fail here.

Suggested fix

-TSAN_OPTIONS="handle_segv=0 die_after_fork=0" \
-  dmtcp_launch --disable-alloc-plugin -i 5 --with-plugin "$PWD/libmcmini.so" \
-  /tmp/producer-consumer-tsan
+setarch -R env TSAN_OPTIONS="handle_segv=0 die_after_fork=0" \
+  dmtcp_launch --disable-alloc-plugin -i 5 --with-plugin "$PWD/libmcmini.so" \
+  /tmp/producer-consumer-tsan

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	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/producer-consumer-tsan
	```
	cd build
	setarch -R env TSAN_OPTIONS="handle_segv=0 die_after_fork=0" \
	dmtcp_launch --disable-alloc-plugin -i 5 --with-plugin "$PWD/libmcmini.so" \
	/tmp/producer-consumer-tsan

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs/superpowers/plans/2026-07-05-tsan-port-phase4-hardening-implementation.md`
around lines 358 - 365, The DMTCP verification command in the TSAN hardening
plan is missing the required ASLR suppression, so update the launch sequence to
run the existing dmtcp_launch invocation through setarch -R. Keep the current
TSAN_OPTIONS and plugin arguments intact, and adjust the documented command so
the producer-consumer-tsan run is executed from the build directory with ASLR
disabled as required by the plan.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C4 '\b_Fork\(|\bwaitpid\(|WEXITSTATUS|WIFEXITED' test/tsan_support/test_repeated_fork_stress.c

Repository: mcminickpt/deep-debug

Length of output: 1695

---

🏁 Script executed:

#!/bin/bash
sed -n '110,210p' test/tsan_support/test_repeated_fork_stress.c

Repository: mcminickpt/deep-debug

Length of output: 3209

---

🏁 Script executed:

#!/bin/bash
nl -ba test/tsan_support/test_repeated_fork_stress.c | sed -n '110,210p'

Repository: mcminickpt/deep-debug

Length of output: 195

---

Guard _Fork()/waitpid() before decoding status. If _Fork() returns -1, the parent path falls through to waitpid(-1, ...); if waitpid() fails or is interrupted, status is still read with WIFEXITED/WEXITSTATUS, which makes the log use uninitialized data.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/tsan_support/test_repeated_fork_stress.c` around lines 23 - 30, In
test_repeated_fork_stress.c, the fork/wait handling can fall through on _Fork()
or waitpid() failure and then decode an uninitialized status value. Update the
control flow around _Fork(), waitpid(), and the WIFEXITED/WEXITSTATUS checks so
the parent only inspects status after a successful waitpid and bails out or
retries cleanly when _Fork() returns -1 or waitpid() is interrupted/fails.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C4 '\b(setTLSPointer|getTLSPointer|SYS_arch_prctl|ARCH_SET_[FG]S)\b' \
  src/lib/dmtcp-callback.c test/tsan_support/test_repeated_fork_stress.c

Repository: mcminickpt/deep-debug

Length of output: 6670

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test/tsan_support/test_repeated_fork_stress.c ---'
sed -n '67,74p' test/tsan_support/test_repeated_fork_stress.c

printf '\n%s\n' '--- src/lib/dmtcp-callback.c ---'
sed -n '94,101p' src/lib/dmtcp-callback.c

printf '\n%s\n' '--- arch_prctl constants from local headers ---'
rg -n 'ARCH_(SET|GET)_[FG]S' /usr/include -g 'prctl.h' -g 'arch*.h' | head -n 20

Repository: mcminickpt/deep-debug

Length of output: 1367

---

Restore TLS using the real arch_prctl arguments. SYS_arch_prctl takes (code, addr), so the extra 2 makes this call use the wrong operation, and != 0 treats the failure path as success. That leaves TLS unset before setcontext().

Suggested fix

 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);
+  assert(syscall(SYS_arch_prctl, ARCH_SET_FS, ti->fs) == 0);
+  assert(syscall(SYS_arch_prctl, ARCH_SET_GS, ti->gs) == 0);
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	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 void setTLSPointer(struct threadinfo *ti) {
	assert(syscall(SYS_arch_prctl, ARCH_SET_FS, ti->fs) == 0);
	assert(syscall(SYS_arch_prctl, ARCH_SET_GS, ti->gs) == 0);
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/tsan_support/test_repeated_fork_stress.c` around lines 71 - 74, The TLS
setup in setTLSPointer is calling syscall(SYS_arch_prctl, ...) with an extra
bogus argument and the assertion is inverted, so the arch_prctl operation is
never applied correctly. Update the setTLSPointer helper to use the real
arch_prctl calling convention with the proper ARCH_SET_FS/ARCH_SET_GS arguments
and make the assertions check for success, ensuring TLS is restored before
setcontext().

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C4 'malloc\(0x10000\)|__clone\(' \
  test/tsan_support/test_repeated_fork_stress.c src/lib/dmtcp-callback.c

Repository: mcminickpt/deep-debug

Length of output: 5229

---

Handle malloc()/__clone() failures in the child path test/tsan_support/test_repeated_fork_stress.c:106-111
malloc(0x10000) is used before any null check, and a failed __clone() can leave the test without a spawned worker to signal its completion path. Exit explicitly on either failure.

🧰 Tools

🪛 Cppcheck (2.21.0)

[error] 106-106: If memory allocation fails

(nullPointerArithmeticOutOfMemory)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/tsan_support/test_repeated_fork_stress.c` around lines 106 - 111, The
child setup in the repeated fork stress test does not guard against allocation
or clone failure, which can leave the worker path broken. In the setup around
malloc, __clone, and child_setcontext_fast, add explicit checks for a null
malloc result and a failed __clone() return, then abort or exit the test
immediately on either failure so the completion path is never skipped.

Source: Linters/SAST tools

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 '\bpthread_create\(|\bsem_wait\(' test/tsan_support/test_repeated_fork_stress.c

Repository: mcminickpt/deep-debug

Length of output: 944

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file structure and inspect the relevant ranges.
wc -l test/tsan_support/test_repeated_fork_stress.c
ast-grep outline test/tsan_support/test_repeated_fork_stress.c --view expanded

echo
sed -n '1,240p' test/tsan_support/test_repeated_fork_stress.c

Repository: mcminickpt/deep-debug

Length of output: 7618

---

Check thread creation errors and retry semaphore waits
pthread_create() failures leave the main thread waiting forever for check-ins, and the raw sem_wait() calls here and at 185-186 should use the existing retry wrapper so an interrupt doesn’t advance the fork or done gate early.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/tsan_support/test_repeated_fork_stress.c` around lines 173 - 178, The
worker launch and synchronization in the repeated fork stress test need error
handling and interrupt-safe waits. Update the thread startup loop around
pthread_create in the worker setup to detect creation failures and fail fast
instead of continuing into sem_wait deadlock, and replace the raw sem_wait calls
in this function (including the check-in and done gate waits) with the existing
retry wrapper used elsewhere so EINTR does not let the fork/done phase advance
prematurely.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/dmtcp-callback.c (1)

469-473: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Skip unreadable thread status entries src/lib/dmtcp-callback.c:469-473 — thread_blocks_signal() returns 0 on /proc/self/task/<tid>/status open/parse failure, so a transient/exited thread is treated as countable here and can overinflate thread_count, leaving the restart barrier stuck. Use a tri-state helper or explicitly skip/retry unreadable entries.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/dmtcp-callback.c` around lines 469 - 473, The restart barrier
counting in the thread scan treats unreadable /proc thread status entries as
countable because thread_blocks_signal() only returns a boolean-like 0 on
open/parse failure. Update the logic around thread_blocks_signal() and the
thread_count increment so transient or exited threads are explicitly skipped or
retried instead of being counted, and ensure the helper can distinguish “cannot
read status” from “thread does not block the signal.”

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/dmtcp-callback.c`:
- Around line 224-230: The raw clone call in the thread recreation loop is not
checked, so a failed libc_clone() can leave thread state incomplete and break
later restart/barrier logic. Update the code around libc_clone() in the thread
restore path to capture its return value, detect failure, and stop or report the
error instead of silently continuing. Use the existing child_setcontext_fast and
threadInfos[i] context to keep the failure handling localized where the thread
is recreated.

---

Outside diff comments:
In `@src/lib/dmtcp-callback.c`:
- Around line 469-473: The restart barrier counting in the thread scan treats
unreadable /proc thread status entries as countable because
thread_blocks_signal() only returns a boolean-like 0 on open/parse failure.
Update the logic around thread_blocks_signal() and the thread_count increment so
transient or exited threads are explicitly skipped or retried instead of being
counted, and ensure the helper can distinguish “cannot read status” from “thread
does not block the signal.”

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 556e9e2e-0975-44d8-8cbb-e75ba8bb0b2f

📥 Commits

Reviewing files that changed from the base of the PR and between faf71a1 and 7773e63.

📒 Files selected for processing (3)

• include/mcmini/spy/intercept/interception.h
• src/lib/dmtcp-callback.c
• src/lib/interception.c

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Check libc_clone() failure before continuing.

If raw clone fails, the loop silently skips recreating that thread and later restart/barrier logic can hang or resume with missing state.

Proposed fix

-    libc_clone(child_setcontext_fast,
-               stack,
-               clone_flags,
-               (void *)&threadInfos[i], ptid, (void *)threadInfos[i].fs, ctid);
+    int rc = libc_clone(child_setcontext_fast,
+                        stack,
+                        clone_flags,
+                        (void *)&threadInfos[i], ptid,
+                        (void *)threadInfos[i].fs, ctid);
+    if (rc < 0) {
+      perror("libc_clone");
+      libc_abort();
+    }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// R3: use libc's raw __clone (via libc_clone(), see above), bypassing
	// both libtsan's and DMTCP's own interception of the public clone()/
	// __clone symbols.
	libc_clone(child_setcontext_fast,
	stack,
	clone_flags,
	(void *)&threadInfos[i], ptid, (void *)threadInfos[i].fs, ctid);
	int rc = libc_clone(child_setcontext_fast,
	stack,
	clone_flags,
	(void *)&threadInfos[i], ptid,
	(void *)threadInfos[i].fs, ctid);
	if (rc < 0) {
	perror("libc_clone");
	libc_abort();
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/dmtcp-callback.c` around lines 224 - 230, The raw clone call in the
thread recreation loop is not checked, so a failed libc_clone() can leave thread
state incomplete and break later restart/barrier logic. Update the code around
libc_clone() in the thread restore path to capture its return value, detect
failure, and stop or report the error instead of silently continuing. Use the
existing child_setcontext_fast and threadInfos[i] context to keep the failure
handling localized where the thread is recreated.