From d3e8ab41ef49a7be77e0bdeee30e97a7819667af Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 28 Oct 2026 00:36:18 -0500 Subject: [PATCH] Publish unwind plans without a plain write to their pc A new plan was copied into its slot as a whole struct, which wrote the slot's pc with a plain store while other threads' lookups read it atomically: a data race. The insert now copies the fields after pc and publishes pc only with the release store. The unwinder test also runs its threaded cases under ThreadSanitizer, which reports the race without this change. --- packages/runtime/src/scr_unwind.c | 6 ++++-- packages/runtime/test/unwind.test.ts | 13 ++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/runtime/src/scr_unwind.c b/packages/runtime/src/scr_unwind.c index f75ee3990..339f85088 100644 --- a/packages/runtime/src/scr_unwind.c +++ b/packages/runtime/src/scr_unwind.c @@ -566,8 +566,10 @@ static __attribute__((noinline)) const ScrUnwindPlan *scr_unwind_insert(uint64_t scr_unwind_plan_count++; ScrUnwindPlan built; scr_unwind_build(&built, ra); - built.pc = 0; - *slot = built; + /* Lookups read pc atomically, so the plan's other fields are written + * first and pc only by the release store that publishes them. */ + memcpy((char *)slot + sizeof slot->pc, (const char *)&built + sizeof built.pc, + sizeof *slot - sizeof slot->pc); __atomic_store_n(&slot->pc, ra, __ATOMIC_RELEASE); result = slot; } diff --git a/packages/runtime/test/unwind.test.ts b/packages/runtime/test/unwind.test.ts index 2e75de1eb..95fc5a0fd 100644 --- a/packages/runtime/test/unwind.test.ts +++ b/packages/runtime/test/unwind.test.ts @@ -18,7 +18,7 @@ test.runIf(process.platform === "linux" && process.arch === "x64")( // .eh_frame_hdr. const library = join(buildDir, "libtest_unwind_frames.so"); await execFileAsync("clang", ["-O1", "-shared", "-fPIC", "-o", library, ...frames]); - for (const build of ["plain", "asan", "shared"] as const) { + for (const build of ["plain", "asan", "tsan", "shared"] as const) { const bin = join(buildDir, `test_unwind_${build}`); await execFileAsync("clang", [ "-std=c11", @@ -28,6 +28,7 @@ test.runIf(process.platform === "linux" && process.arch === "x64")( "-D_GNU_SOURCE", "-DSCR_UNWIND_TESTING", ...(build === "asan" ? ["-fsanitize=address"] : []), + ...(build === "tsan" ? ["-fsanitize=thread"] : []), "-pthread", "-o", bin, @@ -37,8 +38,14 @@ test.runIf(process.platform === "linux" && process.arch === "x64")( // The library's landing pads and the program share libgcc_s's unwinder. ...(build === "shared" ? ["-shared-libgcc"] : ["-lgcc_eh"]), ]); - const env = { ...process.env, ASAN_OPTIONS: "detect_leaks=0:halt_on_error=1" }; - for (const args of [[], ["threads"]]) { + const env = { + ...process.env, + ASAN_OPTIONS: "detect_leaks=0:halt_on_error=1", + TSAN_OPTIONS: "halt_on_error=1", + }; + // ThreadSanitizer checks the threads' concurrent plan inserts and + // lookups; the libgcc fallback cases run in the other builds. + for (const args of build === "tsan" ? [["threads"]] : [[], ["threads"]]) { const run = await execFileAsync(bin, args, { env }); expect(run.stdout).toBe("unwind checks passed\n"); }