diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 650dd6c49c..a0e2292c71 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -934,6 +934,20 @@ jobs: - name: Build compiler + runtime + Windows UI crates (perry-dev) run: cargo build --profile perry-dev -p perry -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static -p perry-ui-windows -p perry-ui-windows-winui + # #7356: the full perry-runtime --lib suite is green on Windows for the + # first time (the SEH/longjmp transport, setjmp alignment, TZ, trim and + # spawnSync fixes). Before that, unguarded eh_walker calls meant the + # crate did not even COMPILE here and nothing noticed — this step is the + # arm that keeps the suite green rather than letting it rot back to + # "unmeasurable". Single-threaded for the same #1444 process-global-state + # reason as the ubuntu leg (both its invocations set RUST_TEST_THREADS=1); + # perry-dev profile so the test build shares the dependency artifacts the + # build step above already produced instead of paying a second cold + # dev-profile build of a ~340k-line crate. + - name: perry-runtime unit tests (single-threaded, #7356) + shell: bash + run: RUST_TEST_THREADS=1 cargo test --profile perry-dev --lib -p perry-runtime + # Small deterministic subset: verifies the Git Bash driver itself, # `.exe`/`.lib` discovery, native TEMP paths, compilation, execution, # and Node/Perry comparison on a real Windows host. diff --git a/changelog.d/7419-windows-runtime-suite-green.md b/changelog.d/7419-windows-runtime-suite-green.md new file mode 100644 index 0000000000..17cedb021b --- /dev/null +++ b/changelog.d/7419-windows-runtime-suite-green.md @@ -0,0 +1,33 @@ +**fix(runtime): the full `perry-runtime --lib` suite is green on Windows — SEH-safe longjmp, setjmp alignment, and four platform-shape test fixes (#7356)** + +#7355 made perry-runtime *compile* on Windows; running the suite then surfaced the pre-existing failures inventoried in #7356. This lands the fixes and the CI arm that keeps them fixed. `cargo test -p perry-runtime --lib -- --test-threads=1` on Windows 11: **1635 passed, 0 failed** (previously: three process-killing stoppers truncated every run). + +Two of the fixes are production bugs, not test bugs: + +- **`js_throw`'s `longjmp` was undefined behavior on windows-msvc** (`exception.rs`). MSVC's `longjmp` reads `_JUMP_BUFFER.Frame` (the jmp_buf's first 8 bytes) and, when nonzero, performs a REAL `RtlUnwindEx` stack unwind — and our one-arg `setjmp` extern leaves that slot holding garbage (the CRT `_setjmp` stores its *second* parameter there; we pass one). Measured: STATUS_BAD_STACK (0xC0000028) in a release probe, GS-cookie aborts (`_report_gsfailure`) under the panic=unwind test harness — the `dyn_eval`/`native_abi` stoppers. Every Rust-side boundary-trap catch on Windows (microtask pump, `js_call_catching`, iterator trampolines, promise combinators) rode this path. Fix: zero the Frame slot before the jump, forcing the non-unwinding POSIX-style `longjmp` whose skipped-cleanup semantics the savepoint restores in `js_throw` already assume. End-to-end validated with a compiled probe (throwing `.then`, `Array.from` mapper, `Promise.all` member, plus a 1000-iteration churn loop) byte-identical to the Node oracle. +- **The conservative-scan register snapshot buffer was under-aligned** (`gc/roots.rs`). MSVC's `_setjmp` saves XMM registers with aligned stores; the `[u64; 32]` buffer is 8-aligned, an immediate access violation whenever it lands 8-mod-16 (measured; this was the `ffi::setjmp` smoke-test AV, same root cause). The snapshot buffer and the three test buffers are now `repr(align(16))`; the extern's docs record both MSVC contracts. + +Test-shape fixes, each keeping the subject live on Windows rather than skipping: + +- `date`: the TZ-isolation child now uses `TZ=PST8PDT` on Windows — the UCRT's `TZ` parser silently degrades IANA ids to UTC, which failed the test's own subject-is-live guard. +- `gc` malloc-trim: the test counter now records that budgeted reclaim *reached* the trim call (the #6180 subject) rather than only counting the glibc/macOS executing arms, which made the gate unsatisfiable on platforms with no trim primitive. +- `child_process`: `spawnSync` result-shape test spawns `cmd /c echo hi` on Windows (`echo` is a shell builtin there, ENOENT under Node too). +- `ffi::setjmp` smoke tests: aligned buffers (above). + +CI: the `windows-build` job now runs `RUST_TEST_THREADS=1 cargo test --profile perry-dev --lib -p perry-runtime` — same single-threaded invocation as the ubuntu leg (#1444), perry-dev profile so it shares the job's build artifacts. Before #7355 there was zero Windows CI to notice the crate didn't compile; this step is what keeps the suite from rotting back to "unmeasurable". + +Out of scope, recorded for honesty: the suite has 5 pre-existing failures in *parallel* mode (`closure::dynamic_props`, `gc teardown`, `prop_plan`, `global_this_webassembly`) — cross-thread interference that CI already sidesteps on every platform by running single-threaded. + +**Review follow-up (audit of this PR).** Moving the malloc-trim counter to the +top of `run_malloc_trim` made the gate satisfiable on Windows/musl, but it also +silently dropped the stronger property on glibc/macOS: the portable counter +witnesses only that reclaim *reached* the call, so it would pass even if the +platform arm were deleted. The assertion still read "must invoke allocator trim", +which is not what it proved. + +Both claims are now asserted separately — a portable `..._CALLS` counter for +"reached" (#6180's actual subject) and a `cfg`-gated `..._EXECUTED` counter, +incremented in both the glibc and Darwin arms, for "a trim primitive actually +ran". Verified the new assertion can fail: removing the Darwin instrumentation +fails the test with "on a target with a trim primitive, budgeted reclaim must +EXECUTE it". diff --git a/crates/perry-runtime/src/child_process/mod.rs b/crates/perry-runtime/src/child_process/mod.rs index 846f0bfd6d..26bdeda8bd 100644 --- a/crates/perry-runtime/src/child_process/mod.rs +++ b/crates/perry-runtime/src/child_process/mod.rs @@ -157,9 +157,22 @@ mod tests { fn test_spawn_sync_result_fields() { // #1936: spawnSync result carries pid / output / stdout / stderr / // status / signal. - let cmd = "echo"; + // + // `echo` is a real executable on unix but a cmd.exe BUILTIN on + // Windows — spawnSync (no shell) can never launch it there, in Node + // too (ENOENT). The subject is the result shape of a SUCCESSFUL + // spawn, so spawn `cmd /c echo hi` on Windows instead (#7356). + #[cfg(windows)] + let (cmd, extra_args): (&str, &[&[u8]]) = ("cmd", &[b"/c", b"echo"]); + #[cfg(not(windows))] + let (cmd, extra_args): (&str, &[&[u8]]) = ("echo", &[]); + let cmd_ptr = js_string_from_bytes(cmd.as_ptr(), cmd.len() as u32); - let args = crate::array::js_array_alloc(1); + let args = crate::array::js_array_alloc((extra_args.len() + 1) as u32); + for a in extra_args { + let s = js_string_from_bytes(a.as_ptr(), a.len() as u32); + crate::array::js_array_push_f64(args, crate::value::js_nanbox_string(s as i64)); + } let hi = js_string_from_bytes(b"hi".as_ptr(), 2); crate::array::js_array_push_f64(args, crate::value::js_nanbox_string(hi as i64)); diff --git a/crates/perry-runtime/src/date.rs b/crates/perry-runtime/src/date.rs index 640c40d23f..01933c6bef 100644 --- a/crates/perry-runtime/src/date.rs +++ b/crates/perry-runtime/src/date.rs @@ -1716,6 +1716,8 @@ mod tests { // 2025-06-20T00:00:00.000Z is still June 19 in Los Angeles. // The three UTC calendar getters must nevertheless keep the UTC // date, while the old delegation to local getters returned 19. + // The `get_date == 19` assertion proves the child's TZ actually + // took effect (subject-is-live guard, not part of the regression). let timestamp = 1_750_377_600_000.0; assert_eq!(js_date_get_date(timestamp), 19.0); assert_eq!(js_date_get_utc_full_year(timestamp), 2025.0); @@ -1724,10 +1726,20 @@ mod tests { return; } + // The UCRT's `TZ` parser understands only the POSIX `tzn[+-]hh dzn` + // shape — an IANA id like `America/Los_Angeles` silently degrades to + // UTC on Windows (#7356), which would fail the subject-is-live guard + // above. Use the spelling each platform's localtime honors; both put + // the child in US-Pacific time (June ⇒ UTC-7 under DST). + #[cfg(windows)] + let tz = "PST8PDT"; + #[cfg(not(windows))] + let tz = "America/Los_Angeles"; + let status = std::process::Command::new(std::env::current_exe().expect("current test exe")) .arg("date::tests::utc_getters_ignore_process_timezone") .arg("--exact") - .env("TZ", "America/Los_Angeles") + .env("TZ", tz) .env(CHILD_MARKER, "1") .status() .expect("spawn timezone-isolated date getter test"); diff --git a/crates/perry-runtime/src/exception.rs b/crates/perry-runtime/src/exception.rs index dbcbb48671..e87c81181b 100644 --- a/crates/perry-runtime/src/exception.rs +++ b/crates/perry-runtime/src/exception.rs @@ -328,6 +328,21 @@ pub extern "C" fn js_throw(value: f64) -> ! { } }); if !jb_ptr.is_null() { + // Windows MSVC: `longjmp` inspects `_JUMP_BUFFER.Frame` (the first + // 8 bytes of the jmp_buf) and, when it is nonzero, performs a REAL + // stack unwind via `RtlUnwindEx` instead of a register restore. Our + // one-arg `setjmp` extern leaves that slot holding whatever was in + // RDX at the call (the CRT `_setjmp` stores its second parameter), + // so the unwind target is garbage — measured 0xC0000028 + // (STATUS_BAD_STACK) in a release binary, and GS-cookie aborts via + // `_report_gsfailure` under the panic=unwind test harness (#7356). + // Zero the slot to force the non-unwinding POSIX-style `longjmp`; + // that is exactly the semantics the savepoint restores above + // assume (skipped cleanups are replayed manually). + #[cfg(windows)] + unsafe { + (jb_ptr as *mut u64).write(0); + } unsafe { longjmp(jb_ptr, 1) } } // Invoke/landingpad handler: raise. The unwinder transfers control to diff --git a/crates/perry-runtime/src/ffi/setjmp.rs b/crates/perry-runtime/src/ffi/setjmp.rs index a9fc7c6c43..f49542ec22 100644 --- a/crates/perry-runtime/src/ffi/setjmp.rs +++ b/crates/perry-runtime/src/ffi/setjmp.rs @@ -55,6 +55,26 @@ extern "C" { extern "C" { /// `setjmp(3)`. On glibc Linux this already doesn't save the /// signal mask, so it's the same fast path we want. + /// + /// ## Windows MSVC caveats (#7356) + /// + /// The symbol resolves to the CRT `_setjmp`, which imposes two extra + /// contracts the POSIX shape doesn't: + /// + /// 1. **16-byte alignment.** `_JUMP_BUFFER` saves Xmm6–Xmm15 with + /// aligned stores; an 8-mod-16 buffer is an immediate + /// STATUS_ACCESS_VIOLATION (measured). Every caller's buffer must + /// be 16-aligned (`exception.rs`'s `JmpBuf` is `repr(align(16))`; + /// tests here use `AlignedJmpBufBytes`). + /// 2. **`Frame` (the first 8 bytes) is garbage after this extern.** + /// The CRT `_setjmp` stores its *second* parameter there, which a + /// one-arg call leaves as whatever RDX held. `longjmp` treats a + /// nonzero `Frame` as a request for a REAL `RtlUnwindEx` stack + /// unwind — with a garbage target: STATUS_BAD_STACK in release + /// binaries, GS-cookie aborts under the panic=unwind test + /// harness. `js_throw` (the only `longjmp` site) zeroes the slot + /// before jumping to force the non-unwinding POSIX semantics the + /// runtime's savepoint restores assume. pub fn setjmp(env: *mut c_int) -> c_int; } @@ -66,7 +86,8 @@ extern "C" { /// - Linux x86_64 glibc: `__jmp_buf` is 8 `i64` = 64 bytes plus /// ~12 bytes of signal-state fields = ~152 bytes for `jmp_buf`. /// - Windows x64 MSVC: 16 doubles = 128 bytes for `_JBLEN`, padded -/// to 256 bytes of `_JUMP_BUFFER`. +/// to 256 bytes of `_JUMP_BUFFER` — and the buffer must be +/// **16-byte aligned** (aligned XMM stores; see the extern's docs). /// /// We surface 192 here so callers can `const_assert!` against it. pub const JMP_BUF_MIN_BYTES: usize = 192; @@ -75,6 +96,18 @@ pub const JMP_BUF_MIN_BYTES: usize = 192; mod tests { use super::*; + /// 256 bytes at the 16-byte alignment MSVC's `_setjmp` requires + /// (aligned XMM stores — an under-aligned buffer AVs on Windows, + /// #7356). Mirrors `exception.rs`'s `JmpBuf` / gc's snapshot buffer. + #[repr(C, align(16))] + struct AlignedJmpBufBytes([u8; 256]); + + impl AlignedJmpBufBytes { + fn new() -> Self { + AlignedJmpBufBytes([0u8; 256]) + } + } + /// Round-trip test: call `setjmp` against a buffer that satisfies /// the minimum size requirement. We never `longjmp` here — the /// goal is just to confirm the extern signature matches libc and @@ -83,12 +116,12 @@ mod tests { /// "first call, not coming from a longjmp." #[test] fn setjmp_smoke_via_c_int_buffer() { - // 64 `c_int`s = 256 bytes, well above `JMP_BUF_MIN_BYTES`. - let mut buf = [0 as c_int; 64]; + // 256 bytes, well above `JMP_BUF_MIN_BYTES`, 16-aligned. + let mut buf = AlignedJmpBufBytes::new(); // SAFETY: `buf` is exclusively owned, lives for the duration // of this call, and exceeds `JMP_BUF_MIN_BYTES`. We never // longjmp into it, so the saved state is never read. - let rv = unsafe { setjmp(buf.as_mut_ptr()) }; + let rv = unsafe { setjmp(buf.0.as_mut_ptr() as *mut c_int) }; assert_eq!(rv, 0, "first-time setjmp must return 0"); } @@ -101,8 +134,8 @@ mod tests { /// warning. #[test] fn setjmp_via_u64_buffer_cast() { - let mut buf = [0u64; 32]; // 256 bytes — matches gc.rs - let rv = unsafe { setjmp(buf.as_mut_ptr() as *mut c_int) }; + let mut buf = AlignedJmpBufBytes::new(); // 256 bytes — matches gc.rs + let rv = unsafe { setjmp(buf.0.as_mut_ptr() as *mut u64 as *mut c_int) }; assert_eq!(rv, 0); } @@ -111,8 +144,8 @@ mod tests { /// `exception.rs::js_try_push`'s `JmpBuf { data: [i32; 64] }`). #[test] fn setjmp_via_i32_buffer_cast() { - let mut buf = [0i32; 64]; // 256 bytes — matches exception.rs - let rv = unsafe { setjmp(buf.as_mut_ptr() as *mut c_int) }; + let mut buf = AlignedJmpBufBytes::new(); // 256 bytes — matches exception.rs + let rv = unsafe { setjmp(buf.0.as_mut_ptr() as *mut i32 as *mut c_int) }; assert_eq!(rv, 0); } diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index e2bfc8c762..c7cbcf1c22 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -743,11 +743,43 @@ pub(super) fn test_malloc_trim_call_count() -> usize { TEST_MALLOC_TRIM_CALLS.with(Cell::get) } -#[cfg(all(test, any(target_env = "gnu", target_os = "macos")))] +#[cfg(test)] fn record_test_malloc_trim_call() { TEST_MALLOC_TRIM_CALLS.with(|calls| calls.set(calls.get().saturating_add(1))); } +// Two counters, because they are two different claims and only one of them is +// portable. `..._CALLS` witnesses that budgeted reclaim REACHED the trim call — +// #6180's actual subject, since the bug was `ordinary_budgeted` skipping it — +// and holds on every target. `..._EXECUTED` witnesses that a trim primitive +// actually ran, which is only meaningful where one exists. +// +// Counting only the executing arms made the gate unsatisfiable on Windows and +// musl (#7356). Counting only reaches would have quietly dropped the stronger +// property on glibc/macOS, where nothing today separates reaching from +// executing but a future early return would. Keeping both means neither +// platform's gate asserts something it cannot see, and neither asserts less +// than it could. +#[cfg(all(test, any(target_env = "gnu", target_os = "macos")))] +thread_local! { + static TEST_MALLOC_TRIM_EXECUTED: Cell = const { Cell::new(0) }; +} + +#[cfg(all(test, any(target_env = "gnu", target_os = "macos")))] +pub(super) fn reset_test_malloc_trim_executed_count() { + TEST_MALLOC_TRIM_EXECUTED.with(|calls| calls.set(0)); +} + +#[cfg(all(test, any(target_env = "gnu", target_os = "macos")))] +pub(super) fn test_malloc_trim_executed_count() -> usize { + TEST_MALLOC_TRIM_EXECUTED.with(Cell::get) +} + +#[cfg(all(test, any(target_env = "gnu", target_os = "macos")))] +fn record_test_malloc_trim_executed() { + TEST_MALLOC_TRIM_EXECUTED.with(|calls| calls.set(calls.get().saturating_add(1))); +} + fn run_malloc_trim(_progress_kind: GcProgressKind) -> MallocTrimOutcome { // #6179/#6180 RSS floor: budgeted cycles are the DEFAULT-path collector // once incremental graduates — skipping allocator trim there meant a @@ -755,10 +787,19 @@ fn run_malloc_trim(_progress_kind: GcProgressKind) -> MallocTrimOutcome { // the OS (2026-07-09 audit finding). Trim runs at Reclaim, outside the // atomic tail, and is itself bounded allocator maintenance. + // The test counter records that budgeted reclaim REACHED this call (the + // #6180 subject — the old bug was skipping it with `ordinary_budgeted`), + // not that the platform executed a trim: on targets with no trim + // primitive (Windows, musl) the outcome below is `Unsupported`, and + // counting only the executing arms made the gate impossible to satisfy + // there (#7356). + #[cfg(test)] + record_test_malloc_trim_call(); + #[cfg(target_env = "gnu")] { #[cfg(test)] - record_test_malloc_trim_call(); + record_test_malloc_trim_executed(); let start = Instant::now(); unsafe { @@ -774,7 +815,7 @@ fn run_malloc_trim(_progress_kind: GcProgressKind) -> MallocTrimOutcome { #[cfg(target_os = "macos")] { #[cfg(test)] - record_test_malloc_trim_call(); + record_test_malloc_trim_executed(); // Darwin counterpart of glibc's malloc_trim: ask every malloc zone // to return clean pages to the OS. Bounded allocator maintenance — diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index 2086639bb2..a4e6ab190d 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -374,13 +374,17 @@ pub(super) fn mark_stack_roots_unchecked( // Size check: 32 * 8 = 256 bytes, which exceeds the darwin arm64 // `jmp_buf` (48 * 4 = 192 bytes) and every other platform we // currently support — see `crate::ffi::setjmp::JMP_BUF_MIN_BYTES`. - let mut jmp_buf = [0u64; 32]; // oversized for safety + // 16-aligned: MSVC's `_setjmp` saves XMM registers with aligned + // stores, and a bare `[u64; 32]` is only 8-aligned (#7356). + #[repr(C, align(16))] + struct JmpBufWords([u64; 32]); + let mut jmp_buf = JmpBufWords([0u64; 32]); // oversized for safety unsafe { - crate::ffi::setjmp::setjmp(jmp_buf.as_mut_ptr() as *mut std::os::raw::c_int); + crate::ffi::setjmp::setjmp(jmp_buf.0.as_mut_ptr() as *mut std::os::raw::c_int); } // Scan the register buffer (covers callee-saved regs: x19-x28 on AArch64, rbx/rbp/r12-r15 on x86_64) - for &word in &jmp_buf { + for &word in &jmp_buf.0 { if try_mark_conservative_word(word, valid_ptrs, pin_only_old) { stats.root_count += 1; } diff --git a/crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs b/crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs index 8bb8161a95..093adfd461 100644 --- a/crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs +++ b/crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs @@ -618,13 +618,26 @@ fn budgeted_reclaim_runs_process_malloc_trim() { let _status = budgeted_step_until_phase(GcCyclePhase::Reclaim); reset_test_malloc_trim_call_count(); + #[cfg(any(target_env = "gnu", target_os = "macos"))] + reset_test_malloc_trim_executed_count(); let before = gc_collection_count(); let completed = complete_budgeted_gc_cycle(); assert_eq!(completed.status, JS_GC_STEP_STATUS_COMPLETED); + // Portable claim: budgeted reclaim REACHED the trim call. That is #6180's + // subject -- the bug was `ordinary_budgeted` skipping it -- and it holds on + // every target, including those with no trim primitive. assert!( test_malloc_trim_call_count() >= 1, - "budgeted reclaim must invoke allocator trim (#6180 RSS floor)" + "budgeted reclaim must REACH allocator trim (#6180 RSS floor)" + ); + // Stronger claim, only where a trim primitive exists: one actually ran. + // Without this the portable counter alone would pass even if the platform + // arm were removed, which is the property #6180 ultimately cares about. + #[cfg(any(target_env = "gnu", target_os = "macos"))] + assert!( + test_malloc_trim_executed_count() >= 1, + "on a target with a trim primitive, budgeted reclaim must EXECUTE it" ); assert!(gc_collection_count() > before); assert_eq!(tracked_malloc_headers_matching(&dead_headers), 0);