rstsr (fix): unsafe-soundness audit follow-up — pointer provenance, broadcast-write rejection, uninitialized-allocation contract by ajz34 · Pull Request #105 · RESTGroup/rstsr

ajz34 · 2026-09-17T12:50:13Z

Behavior change

Writes through broadcast (stride-0) layouts are now rejected on every write path
(in-place assign family, mapi family, the matmul *_with_output drivers,
op_with_func drivers, vecdot_from_f, and the iter_mut/axes_iter_mut
constructors). Constructing and reading a broadcast TensorMut stays legal.
Previously a broadcast output under rayon handed overlapping offsets to different
threads (a data race); the serial path silently aliased writes. Unary in-place ops
on owned broadcasted tensors fall back to producing a fresh output, matching the
binary-op reuse policy.

Enhancement

The uninitialized_vec family in rstsr-common now has a written contract
(alloc_vec_contract.md, rustdoc-included on all three allocation functions):
hazard taxonomy by element type, the write-vs-assignment rule, and the blessed
instantiations for new code.
Layout::check_strides no longer allocates in the common case (8-pair stack
buffer, heap fallback for higher dimensionality) and raises InvalidLayout on
span overflow instead of wrapping in release mode. Semantics unchanged;
Layout::new on a 3-D layout informally measured 33.5 -> 8.0 ns/call.

API breaking changes (user should not feel that)

DeviceMatMulAPI gains the required method matmul_uninit: write-only
c = alpha * (a @ b) into MaybeUninit storage, never reads c; implemented by
all seven in-repo devices. The allocating wrapper now runs uninit_impl ->
matmul_uninit -> assume_init_impl instead of empty + beta = 0.
DimShapeAPI::unravel_index_f/unravel_index_c and Layout::index_uncheck are
no longer unsafe (pure integer arithmetic; worst case a meaningless value or a
panic).
ChangeableDefault::change_default is now a safe fn (defaults backed by
AtomicU8); breaking only for external implementors of the trait — none known.
faer owned-Mat into_rstsr gained a T: Clone bound (it now copies; see Bug
Fix).

Bug Fix

Fix pointer-provenance UB in parallel kernels: the write-through
c.as_ptr() as *mut pattern is replaced by a hoisted AtomicPtr::new(as_mut_ptr())
at all 36 sites of the rstsr-native-impl rayon kernels and in the cdist rayon
kernels of rstsr-sci-traits; the batched-broadcast gemm parallel-outer branch
(device_faer + the five BLAS device crates) and the syrk write-back now derive
per-task pointers from as_mut_ptr() (unique provenance) — previously each task
held an overlapping full-length &mut.
Fix an unsound allocation re-home in faer owned-Mat into_rstsr: mem::forget
- Vec::from_raw_parts mismatched the dealloc layout on every numeric conversion
  (faer over-aligns drop-free element types to at least 64 B and pads row capacity).
  It now copies column-wise; the zero-copy route is mat.as_ref().into_rstsr()
  (a TensorView).
Fix a data race on the process-wide TensorIterOrder default: it was a
static mut read by the safe get_default() on effectively every tensor
operation.
Fix axes_iter/axes_iter_mut (and indexed variants) panicking on an empty axes
list (0usize - 1 underflow); axes_iter(()) now yields a single whole-tensor
view, matching the NumPy ndindex() convention.
Fix naive matmul kernels violating the beta = 0 non-read convention: a
non-finite value in c could propagate through 0 * c.
Fix diag/concatenate dropping uninitialized memory as T for allocatable
element types (assign over empty storage); both now use uninit_impl +
assign_uninit + assume_init_impl. The rstsr-sci-traits distance kernels
likewise fill Vec<MaybeUninit<Out>> write-only.
Fix rstsr-sci-traits doctest compilation (E0659) and the rstsr-native-impl
rayon feature (now enables rstsr-common/rayon); both already failed on the
base commit.

Context

Follow-up of the unsafe-soundness audit of rstsr at acfa93e (working notes and
benchmark records: https://github.com/ajz34/rstsr-improve-trajectory). The bugs
found by the audit were fixed in rstsr-core (fix): view-only iteration API (iter* on views, iter_mut* on mut views) #103/rstsr (fix): three bugs from unsafe-soundness check #104; this branch completes the remaining
work: comment-only SAFETY annotations (61 files across rstsr-common, rstsr-core,
rstsr-native-impl, rstsr-dtype-traits), removal of unnecessary unsafe blocks,
and the audit's design-level flags (parallel-kernel pointer provenance, writes
through broadcast layouts, static mut defaults, the faer allocation re-home),
plus the uninitialized-allocation contract.
New test suite rstsr-core/tests/allocatable_dtype.rs: BigInt through the safe
API surface with exact values, plus drop-counting guards (created == dropped) for
elementwise ops, matmul, diag, concatenate; exact-value cdist tests in
rstsr-sci-traits (serial and rayon).
A-B benchmark for the AtomicPtr hoist (6 interleaved rounds, geometric mean 0.968
— no affected kernel regressed, cdist family -24%): recorded at
https://github.com/ajz34/rstsr-improve-trajectory.
IntoRSTSR is now exported via the prelude rstsr_traits, so facade users can
call into_rstsr (previously prelude_dev only).
Verified: cargo check --workspace clean; rstsr-core 125+10+302+2+185 (incl.
doctests, also with --features faer); rstsr-common, rstsr-native-impl (serial +
rayon), rstsr-sci-traits all green.

🤖 Generated with Claude Code

…review Follow-up of the T1 unsafe-soundness audit (base acfa93e). BUG 1-4 were already fixed by RESTGroup#103/RESTGroup#104; this commit ports the remaining comment-only content and applies two API-level unsafe downgrades agreed with the maintainer. Comment-only (from the audit's safety-comments.patch + fixes.patch): - SAFETY annotations across 61 files (rstsr-common, rstsr-core, rstsr-native-impl, rstsr-dtype-traits): layout machinery, storage, operators, device kernels, allocation. Iterator files follow RESTGroup#103's view-only redesign comments where the audit text was superseded. Unsafe downgrades (pure integer arithmetic, cannot cause UB; worst case is a meaningless value or a panic): - `DimShapeAPI::unravel_index_f`/`unravel_index_c` (rstsr-common/src/layout/shape.rs): `unsafe fn` -> `fn`; `# Safety` docs replaced by unchecked-behavior notes and `# Panics` sections. - `Layout::index_uncheck` (rstsr-common/src/layout/layoutbase.rs): same treatment; doc now states the offset may be negative/meaningless. - Upstream call sites updated: ~30 now-redundant `unsafe` blocks and their stale justifications removed (layout iterators, naive matmul kernels, argmin/argmax reductions, op_tri, faer symmetrize, rstsr-sci-traits distance metrics). `set_offset` call sites keep their `unsafe` blocks. Docs: - `DataForceMutAPI::force_mut` and `TensorAny::force_mut` `# Safety` sections reworded: state the actual caller obligation (unique access while the returned mutable view is alive) instead of a generic "highly unsafe" note. Verification: `cargo check --workspace` 0 warnings; rstsr-core 116+302+2+184 (+184 doc) tests, rstsr-common 36 (+4 doc), rstsr-native-impl 4, rstsr-sci-traits 2 all pass. Pre-existing on the base commit, unrelated: rstsr-sci-traits doctest E0659; rstsr-native-impl `rayon` feature E0425. Most or all contents generated by AI (model glm-5.3-flash, via Claude Code agent), from the T1 audit patch and subsequent review. Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…reakage Two pre-existing failures on the base commit (present before the soundness-audit work): - rstsr-sci-traits doctests aborted with E0659 (`rstsr_sci_traits` ambiguous): rustdoc's doctest compilation passes the crate to itself via `--extern`, and the glob-imported self-alias (`pub(crate) use crate as rstsr_sci_traits` in prelude_dev) conflicted with that outer name in files using `rstsr_sci_traits::` paths. The alias exists so that files meant to be symlinked across crates (e.g. auto_impl_rayon.rs, cf. the rstsr-openblas device crates) can use absolute crate-qualified paths unchanged. It is now an explicit `extern crate self as rstsr_sci_traits;` in lib.rs, which shadows the injected extern instead of conflicting; the portable files and their paths are untouched. - rstsr-native-impl --features rayon failed with 84 errors: the feature did not enable `rstsr-common/rayon` (par-iteration dispatch helpers and IntoParallelIterator for IterLayout), and prelude_dev did not re-export `rayon::ThreadPool`. Both now mirror rstsr-core's setup. Most or all contents generated by AI (model glm-5.3-flash, via Claude Code agent). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…nsafe-audit 4.1) Replace the write-through `c.as_ptr() as *mut` pattern (shared reborrow of the `&mut` slice, read-only provenance - UB under Stacked Borrows, Miri-flagged) at all 36 sites in rstsr-native-impl/src/cpu_rayon, plus the same pattern in rstsr-sci-traits/src/distance/native_impl.rs (cdist_rayon / cdist_weighted_rayon, 4 sites, beyond the audit's original scope). - `AtomicPtr::new(x.as_mut_ptr())` hoisted before the parallel region; `load(Ordering::Relaxed)` at the old derivation point. Relaxed suffices: publication to tasks happens via rayon spawn, and the pointer is never reassigned through the atomic. - Inner `into_par_iter` closures capture `&AtomicPtr` and load inside (raw pointers are !Send/!Sync even behind shared references). - Per-site SAFETY comments rewritten from the NOTE-as_ptr form to the established op_tri.rs AtomicPtr form. Import consolidation: `AtomicPtr`/`Ordering` and `rayon::prelude::*` join rstsr-native-impl's feature-gated prelude_dev, removing per-file boilerplate from all cpu_rayon kernels. A-B benchmark (rstsr-improve-trajectory 2026-09-15-atomicptr-hoist-ab, 6 interleaved rounds, before = aa24643): geometric mean 0.968 - no regression on any affected kernel; cdist family -24%, in-place blocked-2D -5%, naive matmul -2%, others within noise. Tests: rstsr-core rayon 116+302+2+184, rstsr-sci-traits, and workspace `cargo check --all-targets` all green. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…check_strides zero-alloc rewrite - Capability model: a TensorMut with a stride-0 (broadcasted) layout stays constructible and readable; every write path now rejects `layout.is_broadcasted()`, the same predicate `assign`/`fill` already enforced. Gated: the add_assign family (tensor and scalar impls), mapi_f/mapi_fnmut_f, the matmul output driver, iter_mut/indexed_iter_mut constructors, and axes_iter_mut/indexed_axes_iter_mut (guard on the iterated axes only; yielded items keep non-iterated broadcast axes as inert views, caught by the item-level gates on write). `index_mut` needs no gate (single-element `&mut` access is borrow-checked). - Unary in-place ops on owned broadcasted tensors fall back to producing a fresh output instead of erroring, matching the binary-op reuse policy. - check_strides: insertion sort into an 8-pair stack buffer (heap fallback for higher dimensionality), no allocation in the common case; cumulative span arithmetic is checked and raises InvalidLayout on overflow instead of wrapping in release mode. Semantics unchanged (shape-1 axes are skipped before the stride loop). - Regression tests: broadcast write rejection in each touched file; layout tests for zero-stride handling, >8-axis heap fallback, and span overflow. Informal timing of Layout::new (3-D): 33.5 -> 8.0 ns/call. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…-audit 4.3) The process-wide default of `TensorIterOrder` was a `static mut` read by the safe `get_default()` on effectively every tensor operation (creation, binary arithmetic, axes iteration). Concurrent `change_default()` would have been a data race (UB) reachable through safe readers. - replace the `static mut` with `AtomicU8` + `Relaxed` ordering (an independent policy byte that orders no other memory); - `change_default` is now a safe fn; - drop the `impl_changeable_default!` macro (single instantiation) in favor of direct impls; - `TensorIterOrder` gains `#[repr(u8)]`; discriminant round-trip via a guarded match whose fallback arm only guards memory corruption; - round-trip regression test (restores the process-global default). `core::sync::atomic` keeps `#![cfg_attr(not(test), no_std)]` intact. Note: `unsafe fn` -> `fn` on the public trait is breaking for downstream implementors of `ChangeableDefault`; there are no in-tree or known ones. Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

… allocation re-home The owned-Mat conversion used mem::forget + Vec::from_raw_parts to re-home faer's allocation into an owning Vec. Verified against the faer 0.22.6 source, the dealloc layout mismatched on every numeric conversion, not just hypothetically: faer over-aligns power-of-two-sized, drop-free element types to max(align, 64) and pads row capacity to the alignment multiple, while the Vec deallocates with align_of::<T> and the logical length (e.g. a 5x1 Mat<f64>: alloc 64 B / align 64 vs dealloc 40 B / align 8). Mismatched dealloc layout is UB by the GlobalAlloc contract, benign only under allocators that ignore the layout (glibc). into_rstsr now copies the logical elements column-wise via Mat::col_as_slice (safe code; new T: Clone bound) into a fresh contiguous column-major buffer (stride [1, nrows], offset 0) and lets faer drop its own allocation. Downstream eigh/cholesky/pinv call sites are unaffected; their following into_contig becomes a no-op view. Docs: the impl docstring states the copy behavior, the reason, and the zero-copy alternative mat.as_ref().into_rstsr() (non-owning TensorView); the MatRef/ColRef/MatMut impls (ManuallyDrop, never freed, already sound) got one-line ownership docs; the IntoRSTSR trait doc notes that ownership semantics are impl-specific. IntoRSTSR is now exported via prelude rstsr_traits: the facade rstsr::prelude re-exports only rstsr_traits/structs/funcs/macros (not prelude_dev), so facade users previously could not call into_rstsr. Tests: test_mat_owned_into_rstsr (content, contiguous layout, padded 5x1 capacity case, zero-copy route) and a doctest. cargo test -p rstsr-core (lib 123, doc 185) and cargo check --workspace green. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…cache behavior

The duplicate-axes check `for i in 0..axes_check.len() - 1` underflowed `0usize - 1` for an empty axes list, panicking (obscurely in release) instead of iterating. Replaced with an adjacent-pairs `windows(2)` scan at all four constructor sites (axes_iter / axes_iter_mut / indexed_* variants). Semantics come from the existing machinery, no new code path: an empty selection gives a 0-d `layout_axes` (IterLayout yields exactly one element) and the full `layout_inner`, so `axes_iter(())` / `axes_iter(vec![])` now yield a single whole-tensor view — the product over an empty axes set is 1, matching NumPy `ndindex()` convention. `()` is a reachable spelling via `From<()> for AxesIndex`. Regression tests: test_axes_iter_empty_axes (both spellings, indexed pair, duplicate axes still rejected) and test_axes_iter_mut_empty_axes; verified under row_major (default+rayon) and col_major+rayon feature sets. Pre-existing col_major `*_broadcast_err` lib-test failures are unrelated (fail on clean tree too). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

Make the uninitialized_vec family's acceptance precise and migrate the last unsafe internal users onto sound paths: - rstsr-common: new `alloc_vec_contract.md`, rustdoc-included on all three allocation functions via `#[doc = include_str!]`. States the hazard taxonomy by element type, the write-vs-assignment rule (assignment drops the old slot value; `write` does not), the three blessed instantiations (uninit_impl+assume_init_impl / statically-POD FFI buffers / unsafe `empty`), and rules for new code. - rstsr-core: `DeviceMatMulAPI::matmul_uninit` — write-only `c = alpha * (a @ b)` into MaybeUninit storage, never reads `c`; new required trait method, implemented by all seven in-repo devices (BLAS crates and faer reinterpret the storage to the POD dtype after TypeId dispatch and pass beta = 0 under the BLAS non-read convention; other dtypes run dedicated write-only naive kernels in rstsr-native-impl). The allocating wrapper `op_refa_refb_matmul` now runs uninit_impl -> matmul_uninit -> assume_init_impl instead of `empty` + beta = 0. - All naive matmul kernels gained `beta.is_zero()` guards: the beta-scaling path now honors the BLAS non-read convention (previously a non-finite value in `c` could propagate through `0 * c` on the naive fallbacks). CPU-serial kernels and the DeviceCpuSerial impl bind `TC: Clone + Zero`. - diag/concatenate migrated from `empty` + `assign` to `uninit_impl` + `assign_uninit` + `assume_init_impl` with no trait-bound changes: `assign` writes with drop-of-old semantics, so over `empty` storage it dropped uninitialized memory as `T` on every call for allocatable types. `concatenate` sizes storage via `bounds_index()`. - rstsr-sci-traits: distance kernels allocate `Vec<MaybeUninit<M::Out>>` with write-only fills and one localized re-view (was plain-`T` uninitialized_vec with unconstrained `MetricDistAPI::Out`). - matmul_from/matmul_with_output docs note the beta = 0 non-read convention. Tests: `rstsr-core/tests/allocatable_dtype.rs` (BigInt through the safe API surface with exact values, plus drop-counting guards asserting created == dropped for element-wise ops, matmul, diag, concatenate) and in-crate cdist tests in rstsr-sci-traits (exact euclidean values, both orders; non-POD Out guard, serial and rayon). All green: cargo check --workspace; rstsr-core 125+10+302+2+185 (also with --features faer); rstsr-common 40+4; rstsr-native-impl serial and rayon features; rstsr-sci-traits 5; rstsr-core doctests 185. Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…e-provenance task pointers in batched matmul - Output-write entry points that lacked the broadcast-layout gate now reject stride-0 output layouts, completing the capability model (holding a broadcast TensorMut stays legal; every write path rejects it): the *_with_output family (op_mutc_refa_refb), the op_with_func drivers (op_mutc_refa_refb_func / op_muta_refb_func / op_muta_func), and vecdot_from_f. A broadcast output under rayon previously handed overlapping offsets to different threads (data race); the serial path silently aliased writes. - Batched broadcast gemm, parallel-outer branch (device_faer and the five BLAS device crates): the per-task slice handle now derives from a hoisted AtomicPtr of c.as_mut_ptr() (unique provenance) instead of c.as_ptr() cast to *mut (a shared reborrow; writing through it is provenance UB, and each task held an overlapping full-length &mut). The incorrect SAFETY comment in device_faer is corrected. - syrk lower-triangle write-back (five BLAS device crates): write pointers now derive from an AtomicPtr hoist of as_mut_ptr() instead of as_ptr().add(..) as *mut. - Tests: broadcast rejection next to each gate; test_matmul_rule7_broadcast_parallel_outer (DeviceFaer, 64 batches, crosses the parallel-outer threshold; matmul and matmul_from with beta != 0). Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

- rustfmt (`cargo fmt --all -- --check`): reflow lines and SAFETY comments in the rayon reduction kernels (rstsr-native-impl), the AtomicPtr hoist blocks of the five BLAS device crates' batched matmul and the openblas build script, rstsr-sci-traits distance kernels and tests, rstsr-common (rearrangement, tensordot_to_einsum), and the allocatable-dtype test suite; also the comment rewraps in the rstsr-core test modules touched by this branch. - clippy (`--all-targets --all-features -- -D warnings`): drop an unused test-module re-import of `crate::prelude_dev::*` (the parent module already imports it, so `use super::*` covers it) and remove an unnecessary `unsafe` block in the op_with_func broadcast-rejection test (`MaybeUninit::write` is a safe method); the closure keeps a block body since `write` returns `&mut T`. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

…(col_major CI) The broadcast-write rejection tests added in 1bc2c39 read back the untouched tensor through `view().iter()`, which follows the device default order. The test layout `Layout::new([2, 3], [0, 1], 0)` is f-contiguous, so under the col_major feature set the read-back visits elements first-axis-fastest and the row-major expectation vectors mismatched — 7 lib-test failures in the unittests-col-major CI job. The rejection gates themselves were already correct under both feature sets (the `is_err()` assertions pass). Following the established test convention, pin the device to row-major (`device.set_default_order(RowMajor)`) at the start of each of these tests; read-backs stay plain `.iter()` and assert the same element sequence regardless of the crate's default-order feature. Verified: rstsr-core lib tests green under col_major (118 passed) and default (129 passed) feature sets. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

Re-run `cargo fmt --all` with the updated stable rustfmt; comment rewrap preferences changed in the 1.100+ formatter, so a few comment blocks formatted with an older nightly toolchain (which additionally honors the nightly-only wrap_comments option) no longer matched the CI check. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

Re-run `cargo fmt --all` with rustfmt 1.100 (rustc 1.100.0-nightly, 2026-09-16), matching the CI runner's post-1.100 formatter: comment wrapping (rustfmt.toml wrap_comments/comment_width/overflow_delimited_expr) now takes effect on stable-derived toolchains, so blocks formatted by older rustfmt (1.9.0 stable / pre-1.100 nightly) diverge again — e.g. the openblas build script comment rewrap, overflow_delimited_expr-style match/array literals in rstsr-blas-traits util and rstsr-common layout iterator tests, and comment reflows in native-impl kernels. Most or all contents generated by AI (model glm-5.3-flash). Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: glm-5.3-flash <service@zhipuai.cn>

ajz34 and others added 14 commits September 15, 2026 20:29

rstsr (fix): remove unnecessary unsafe

5829b28

rstsr-common (fix): remove note on Layout::size that is incorrect on …

3086ba1

…cache behavior

ajz34 force-pushed the 260915-unsafe-soundness-3 branch from ed235ae to 6ca47ac Compare September 17, 2026 13:27

ajz34 merged commit dee8807 into RESTGroup:master Sep 17, 2026
12 checks passed

ajz34 deleted the 260915-unsafe-soundness-3 branch September 17, 2026 13:46

ajz34 mentioned this pull request Sep 17, 2026

rstsr-book (dev): add ADR-0008 uninitialized allocation contract RESTGroup/rstsr-book#8

Merged

Navigation Menu

rstsr (fix): unsafe-soundness audit follow-up — pointer provenance, broadcast-write rejection, uninitialized-allocation contract - #105

rstsr (fix): unsafe-soundness audit follow-up — pointer provenance, broadcast-write rejection, uninitialized-allocation contract#105
ajz34 merged 15 commits into
RESTGroup:masterfrom
ajz34:260915-unsafe-soundness-3

ajz34 commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ajz34 commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant