From d400b53886dffce6dabcf96b8e101e4494a0db97 Mon Sep 17 00:00:00 2001 From: Hermes CI Date: Wed, 26 Aug 2026 02:47:29 -0500 Subject: [PATCH] gopher64-netplay#23: give Device-constructing unit tests an 8 MiB test-stack budget libtest runs each test on a thread with the default 2 MiB stack. In a debug build, Device::new() materialises ~301 KiB of struct literals on the stack, and postcard::from_bytes:: rebuilds the whole value on the stack, so any Device-constructing unit test (netplay savestate round trips etc.) peaks at several times that and aborts the ENTIRE test binary with a stack overflow, taking every other test down with it. Fix, both layers: - src/test_util.rs: with_big_stack() runs a test body on an explicitly sized 8 MiB thread (the budget the project already standardises on in .cargo/config.toml). Regression test: a device serialise/restore/clone round trip measured to overflow the 2 MiB libtest thread (SIGABRT) and pass at 8 MiB, plus a size_of::() < 700 KiB tripwire. - scripts/gate.sh: export RUST_MIN_STACK=8388608 around step 3 so the documented gate also covers Device-constructing tests that are not yet wrapped (the netplay cluster's !15/!18 tests land after this fix). Verified: cargo test --lib bare (no env) -> 1 passed; full gate -> GATE_OK with step 3 reporting 1 passed (was 0 on main). --- scripts/gate.sh | 10 +++++++ src/lib.rs | 2 ++ src/test_util.rs | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 src/test_util.rs diff --git a/scripts/gate.sh b/scripts/gate.sh index cca520c..83da38f 100755 --- a/scripts/gate.sh +++ b/scripts/gate.sh @@ -75,6 +75,16 @@ echo "GATE step 2/3: cargo test --no-run" cargo test --no-run echo "GATE step 3/3: cargo test --lib" + +# Issue #23: libtest runs each test on a thread with the 2 MiB default stack, +# but debug builds of Device::new() materialise ~300 KiB of struct literals on +# the stack per construction, so any Device-constructing unit test overflows +# and aborts the WHOLE test binary. Give the test threads the same 8 MiB the +# project already standardises on for the main thread (.cargo/config.toml +# /STACK:8388608, Windows). Tests should additionally route their bodies +# through test_util::with_big_stack so they also pass under a bare +# `cargo test --lib` (see src/test_util.rs). +export RUST_MIN_STACK=8388608 cargo test --lib echo "GATE_OK" diff --git a/src/lib.rs b/src/lib.rs index b3e51ae..fb9cfc2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,8 @@ mod netplay; mod retroachievements; mod savestates; mod ui; +#[cfg(test)] +mod test_util; use clap::Parser; #[cfg(target_os = "android")] use slint::ComponentHandle; diff --git a/src/test_util.rs b/src/test_util.rs new file mode 100644 index 0000000..8a3da69 --- /dev/null +++ b/src/test_util.rs @@ -0,0 +1,71 @@ +//! Test-only utilities shared by unit tests across the crate. +//! +//! Background (issue #23): [`crate::device::Device`] is the entire N64 machine +//! state — cpu, pif, cart, memory, rsp, rdp, rdram, mi, pi, vi, ai, si, ri, +//! vru, transfer paks, cheats — 307968 bytes (~301 KiB) in a debug build. +//! `Device::new()` materialises the struct literal and its nested field +//! literals on the stack before moving them into the box, and +//! `postcard::from_bytes::` rebuilds the whole value on the stack — +//! so a netplay-savestate round trip (construct, serialize, restore, clone) +//! peaks at several times that size. libtest runs each test on a spawned +//! thread whose default stack is 2 MiB (not the 8 MiB main-thread stack), so +//! such a test aborts the whole test binary with +//! `fatal runtime error: stack overflow` — taking every other test in the run +//! down with it. +//! +//! [`with_big_stack`] runs a test body on an explicitly sized thread so tests +//! are independent of the ambient stack, on any runner, with no env var. + +/// Run `f` on a thread with an 8 MiB stack — the same budget the project +/// already standardises on for the main thread (`.cargo/config.toml` ships +/// `/STACK:8388608` on Windows). Panics inside `f` are propagated to the +/// caller, so a failing test still fails normally. +pub(crate) fn with_big_stack(f: F) { + std::thread::Builder::new() + .stack_size(8 * 1024 * 1024) + .spawn(f) + .unwrap() + .join() + .unwrap(); +} + +#[cfg(test)] +mod tests { + use super::with_big_stack; + use crate::device::Device; + + /// Issue #23 acceptance-criterion 5 tripwire: if `Device` ever grows past + /// ~700 KiB the 8 MiB test-stack budget chosen here has to be revisited. + const MAX_DEVICE_SIZE: usize = 700 * 1024; + + /// Regression test for issue #23: a netplay-savestate-style round trip — + /// construct a `Device`, serialize it, rebuild it from bytes (postcard + /// deserialises the ~301 KiB struct on the stack), clone its state back — + /// needs more than the 2 MiB libtest thread stack in a debug build. + /// + /// Measured with this exact body unwrapped on the ambient libtest thread: + /// `thread '...' has overflowed its stack` / `fatal runtime error: stack + /// overflow, aborting` (SIGABRT); the same body passes with an 8 MiB + /// stack. This body is therefore run on an 8 MiB thread; if the wrapping + /// is ever removed the test binary aborts instead of reporting a failure, + /// so the mitigation cannot silently regress once step 3 of the gate has + /// real content. + #[test] + fn device_round_trip_fits_under_the_8_mib_test_stack_budget() { + with_big_stack(|| { + let device = Device::new(false); + let bytes = postcard::to_stdvec(&device).unwrap(); + let mut restored: Device = postcard::from_bytes(&bytes).unwrap(); + restored.clone_state(&device); + // Keep both devices alive so nothing is dropped early and the + // peak stack use is actually measured. + std::hint::black_box((device, restored)); + }); + + assert!( + std::mem::size_of::() < MAX_DEVICE_SIZE, + "Device is now {} bytes; the 8 MiB test-stack budget in with_big_stack must be revisited", + std::mem::size_of::() + ); + } +}