diff --git a/src/backends/ggml/Cargo.toml b/src/backends/ggml/Cargo.toml index 60ae47c..6cf6728 100644 --- a/src/backends/ggml/Cargo.toml +++ b/src/backends/ggml/Cargo.toml @@ -33,4 +33,12 @@ thiserror = { workspace = true } # here yet because doing so would require cloning GGML's build # graph; the Q4_K layout is cross-checked separately in # `tests/q4_k_layout.rs` via a standalone C-side static_assert. -bindgen = { version = "0.69", default-features = false, features = ["runtime"] } +# +# The `=0.69.5` exact-version pin is intentional: bindgen patch +# releases have historically shifted the layout of generated +# code (alignment of `#[repr(C)]` shims, derive ordering, etc.). +# Pinning the patch keeps the binding output stable across CI +# rebuilds and across developer machines without relying on a +# committed Cargo.lock alone (Cargo.lock is the source of truth +# for binaries; libraries pinning here is belt-and-suspenders). +bindgen = { version = "=0.69.5", default-features = false, features = ["runtime"] } diff --git a/src/backends/ggml/build.rs b/src/backends/ggml/build.rs index f759d88..5d67e3b 100644 --- a/src/backends/ggml/build.rs +++ b/src/backends/ggml/build.rs @@ -48,6 +48,18 @@ fn main() { .derive_default(true) .derive_debug(true) .derive_copy(true) + // Suppress bindgen's compile-time size/align assertions. + // Enabling them would require the test runner to link + // against libclang at `cargo test` time (the generated + // assertions reference C-side `sizeof`/`alignof` values), + // which is heavier than this crate's UAPI surface + // justifies. The cross-mirror test + // `bindgen_uapi_constants_match_runtime_mirror` in + // `src/lib.rs` covers `size_of::()` + // (the only struct currently bound) at unit-test time + // instead. If WORK_SUBMIT or another non-trivial struct + // is later added to the UAPI header, revisit this and + // either flip layout_tests back on or extend the mirror. .layout_tests(false) .generate() .expect("bindgen failed to generate spanker UAPI bindings"); diff --git a/src/backends/ggml/src/lib.rs b/src/backends/ggml/src/lib.rs index dc29139..0fe736f 100644 --- a/src/backends/ggml/src/lib.rs +++ b/src/backends/ggml/src/lib.rs @@ -73,7 +73,11 @@ pub const Q4_K_BLOCK_BYTES: usize = 144; /// Bytes per element in the f32-shaped output the device returns /// for a Q4_K matmul. The device dequantizes on the way out, so /// `out` is `m × n` f32 values regardless of the input encoding. -pub(crate) const OUTPUT_ELEM_BYTES: usize = 4; +/// +/// Public so integration tests under `tests/` can compute the same +/// expected output-buffer size the production helpers use, without +/// re-declaring a local `const = 4` that could silently drift. +pub const OUTPUT_ELEM_BYTES: usize = 4; /// Errors returned by this crate. /// diff --git a/src/backends/ggml/src/sail.rs b/src/backends/ggml/src/sail.rs index d475c2c..51fce39 100644 --- a/src/backends/ggml/src/sail.rs +++ b/src/backends/ggml/src/sail.rs @@ -70,19 +70,32 @@ mod tests { /// it with the corresponding success-path assertions — do not /// just delete it. /// - /// Uses `/dev/null` for the underlying handle: it always - /// exists, opens read+write, and the test never reaches an - /// ioctl call (the stub short-circuits with `NotImplemented` - /// before any device traffic). + /// Uses a freshly-created temp file under `std::env::temp_dir()` + /// for the underlying handle. The stub short-circuits with + /// `NotImplemented` before issuing any ioctl, so the file's + /// type doesn't matter — only that `open(2)` succeeds. + /// Previous revisions used `/dev/null`, which is read-only in + /// some hermetic sandboxes and caused the test to silently + /// `return` (passing in CI summaries while never asserting + /// anything). A temp file removes that silent-skip path. #[test] fn matmul_q4_k_returns_not_implemented() { - // /dev/null may not be writable as a regular file in - // every sandbox. If we can't open it, skip — running this - // assertion is best-effort until a hermetic harness lands. - let Ok(ctl) = SpankerControl::open_path("/dev/null") else { - eprintln!("/dev/null unavailable for SailMatmul stub test; skipping"); - return; - }; + // Unique-per-process path so concurrent test runs don't + // collide. `cargo test` runs each integration binary in + // its own process, but unit tests inside one binary share + // a PID — `line!()` keeps this filename distinct from any + // sibling test that might adopt the same pattern later. + let tmp_path = std::env::temp_dir().join(format!( + "spanker-sail-stub-{}-{}.tmp", + std::process::id(), + line!() + )); + // `create` truncates if the file already exists from a + // previous run that was killed before cleanup ran. + std::fs::File::create(&tmp_path).expect("temp file should be creatable"); + + let ctl = SpankerControl::open_path(&tmp_path) + .expect("freshly-created temp file should open r+w"); let sail = SailMatmul::new(ctl); let err = sail .matmul_q4_k(&[], &[], &mut [], 0, 0, 0) @@ -91,6 +104,10 @@ mod tests { matches!(err, Error::NotImplemented), "expected NotImplemented, got {err:?}" ); + + // Best-effort cleanup; a leaked file in /tmp is harmless + // but tidiness is cheap. + let _ = std::fs::remove_file(&tmp_path); } #[test] diff --git a/src/backends/ggml/tests/mock_matmul.rs b/src/backends/ggml/tests/mock_matmul.rs index 93f1737..e3613bb 100644 --- a/src/backends/ggml/tests/mock_matmul.rs +++ b/src/backends/ggml/tests/mock_matmul.rs @@ -9,15 +9,31 @@ //! `axi4_mem_model` cocotb harness; until that lands the mock is //! the source of truth for transaction shape. -use ggml_spanker::{Error, MatmulInt4, MockSail, Transaction, Q4_K_BLOCK_BYTES, QK_K}; - -const OUTPUT_ELEM_BYTES: usize = 4; // f32 dequant on the device side +use ggml_spanker::{ + Error, MatmulInt4, MockSail, Transaction, OUTPUT_ELEM_BYTES, Q4_K_BLOCK_BYTES, QK_K, +}; fn alloc_operands(m: usize, k: usize, n: usize) -> (Vec, Vec, Vec) { + // `checked_mul` mirrors the production helpers + // (`expected_a_bytes`, `expected_b_bytes`, `expected_out_bytes`) + // so a future contributor copying this allocator inherits the + // overflow-safe idiom rather than the bare `*` shortcut. let blocks = k / QK_K; - let a = vec![0u8; m * blocks * Q4_K_BLOCK_BYTES]; - let b = vec![0u8; n * blocks * Q4_K_BLOCK_BYTES]; - let out = vec![0u8; m * n * OUTPUT_ELEM_BYTES]; + let a_len = m + .checked_mul(blocks) + .and_then(|v| v.checked_mul(Q4_K_BLOCK_BYTES)) + .expect("A bytes overflow usize"); + let b_len = n + .checked_mul(blocks) + .and_then(|v| v.checked_mul(Q4_K_BLOCK_BYTES)) + .expect("B bytes overflow usize"); + let out_len = m + .checked_mul(n) + .and_then(|v| v.checked_mul(OUTPUT_ELEM_BYTES)) + .expect("OUT bytes overflow usize"); + let a = vec![0u8; a_len]; + let b = vec![0u8; b_len]; + let out = vec![0u8; out_len]; (a, b, out) }