Skip to content

Add Miri and cargo-mutants gates; NEON decode kernel takes fixed-size arrays - #10

Open
coderdan wants to merge 6 commits into
mainfrom
miri-mutants
Open

coderdan wants to merge 6 commits into
mainfrom
miri-mutants

Conversation

@coderdan

@coderdan coderdan commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Safety gates for the crate's unsafe SIMD code, and the code changes they prompted.

 .github/workflows/
+├── miri.yml        # x86_64 × {avx2, scalar}, default + strict models
+├── sanitizers.yml  # ASan on aarch64: the NEON path Miri can't run
+└── mutants.yml     # cargo-mutants gate, macOS (aarch64) + Ubuntu (x86_64)
+.cargo/mutants.toml # excludes that hold on every arch, timeouts
+scripts/mutants.sh  # per-arch excludes; CI and `mise run mutants` both call it

Miri runs on x86_64 only: it interprets the AVX2 intrinsics but stops at the NEON table lookup (vqtbx1q_u8). ASan covers the NEON path instead. Under cfg(miri) the property tests drop to 16 cases and the brute-force parity sweeps are ignored.

cargo-mutants mutates the PR's changed lines. It sweeps the whole crate instead when the mutants setup changes, or when a src/tests change yields no mutants on its own lines (a rename would otherwise pass without testing anything). cargo-mutants ignores #[cfg], so the per-arch dispatch functions get distinct names and each host excludes the arch it doesn't compile:

-fn encode_into / decode_into      (×3, cfg'd)
+fn encode_into_neon / _avx2 / _portable
+use encode_into_neon as encode_into;   // cfg'd alias; call sites unchanged

NEON decode kernel takes arrays, matching the AVX2 kernel, and is no longer unsafe fn:

-pub(crate) unsafe fn try_decode_block_x4(input: &[u8], out: &mut [u8])
+pub(crate) fn try_decode_block_x4(input: &[u8; 20], out: &mut [u8; 16])

Dispatch loops share one scalar block-and-tail loop instead of four copies:

 decode_into_neon / decode_into_avx2
   while let (Some(in_chunk), Some(out_chunk)) = …   // SIMD chunks
     on kernel error: decode_chunk_by_block   // exact error position
-  scalar block loop + tail                 (copy per path)
+  decode_blocks_from(input, out, in_off, out_off)
 decode_scalar
-  scalar block loop + tail
+  decode_blocks_from(input, out, 0, 0)

Encode is the same shape. Every NEON load and store has a // SAFETY: comment, and the two NEON lookup helpers, which had no preconditions, are safe fns.

Evidence

  • Miri (local, x86_64 target): AVX2 and scalar suites pass, no UB. CI also runs the strict models (-Zmiri-tree-borrows -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check).

  • ASan, aarch64: full suite clean, locally and in CI.

  • Mutants, aarch64 full sweep:
    Before: 6 missed, all in the NEON decode loop guards. decode() sizes out from input, so each guard's two halves were redundant and a broken one went unnoticed.
    After: 115 caught, 0 missed, 12 unviable. decode_into_stays_in_bounds_with_mismatched_buffers checks each bound on its own. The NEON kernel is mutation-tested for the first time (cargo-mutants skips unsafe fn).

  • Also missed before, caught now: DecodeError's Display, and the x86 scalar fallback, which an AVX2 runner never reached.

  • Excludes (.cargo/mutants.toml), each with its reason:

    • _portable: not compiled on any tested host.
    • The encode tail > 0 → >= 0 mutant: equivalent; no input can tell them apart.
    • The inverted is_x86_feature_detected!("avx2") check: same output on an AVX2 host, so no test can catch it. The Miri scalar job catches it, as a call to a function needing unavailable features.
  • Benchmarks (M-series, AC power, main then this branch back to back; the unchanged base85 reference moved up to ±3.5% between runs):

    16 B 64 B 256 B 1 KiB 1 MiB
    decode +0.7% +0.6% −0.5% +0.4% −0.2%
    encode +6.0% +1.1% +2.8% +0.5% −1.7%

    The comment that justified open slices claimed fixed arrays cost ~4×. That doesn't reproduce.

Merge Danger

Door: two-way

The public API is unchanged, and any commit can be reverted on its own.

Blast Radius: CI

The new gates can fail future PRs. To make them blocking, mark the two "Mutants gate" checks as required.

Miri runs the test suite on x86_64 twice: with +avx2 (the SIMD path) and
without (the scalar fallback, which also exercises the runtime AVX2
check). Miri can't interpret the NEON table lookups, so aarch64 isn't
covered. Under cfg(miri) the property tests drop to 16 cases and the
brute-force parity sweeps are ignored.

The cargo-mutants gate runs --in-diff on PRs, on both macOS (aarch64)
and Ubuntu (x86_64). cargo-mutants ignores #[cfg], so the per-arch
encode_into/decode_into variants get distinct names (*_neon, *_avx2,
*_portable, aliased back) and each job excludes the other arch's code.

Tests added for gaps the first sweep found: DecodeError's Display, the
x86_64 scalar fallback (never reached on AVX2 runners), and decode_into
with mismatched buffers, which checks each half of the loop bounds that
decode() otherwise always satisfies together.
try_decode_block_x4 now takes &[u8; 20] / &mut [u8; 16], like the AVX2
kernel, so its loads and stores are in bounds by type. It is no longer
an unsafe fn, which also brings it under mutation testing (cargo-mutants
skips unsafe fns). decode_into_neon walks the input with
while let (chunk_at, chunk_at_mut): one bounds check per buffer, and no
more lint allowances.

The open-slice form was kept for a claimed ~4x speedup that doesn't
reproduce. Medians on an M-series Mac, back to back, original vs new:

  16 B    14.4 ns ->  15.1 ns  (+5.2%)
  64 B    22.1 ns ->  23.6 ns  (+6.4%)
  256 B   56.2 ns ->  57.4 ns  (+2.1%)
  1 KiB  225.2 ns -> 226.1 ns  (+0.4%)
  1 MiB  217.4 us -> 217.5 us  (0.0%)

A rerun of the original against itself moved by at most 0.5%.
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Mutation testing — x86_64 (cargo-mutants, whole crate)

caught missed unviable timeout
108 0 2 0

Every mutant was caught by a test.

--in-diff only mutates changed lines, so a PR that edits the excludes or
the workflow can hide mutants anywhere and still pass on an empty diff.
This PR's own x86_64 run did exactly that: renaming the AVX2 dispatch
changed only the signature lines, so it generated zero mutants.

Full sweeps run at -j 2 with a 120-minute limit.
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Mutation testing — aarch64 (cargo-mutants, whole crate)

caught missed unviable timeout
115 0 12 0

Every mutant was caught by a test.

Inverting the !is_x86_feature_detected!("avx2") check on an AVX2 host
routes every call to the scalar fallback, which returns the same bytes,
so no test can catch it. The Miri scalar job builds without AVX2 and
fails if that check ever lets AVX2 code run.
Each encode/decode path repeated the scalar block loop and tail handling
after its SIMD chunks. They now end in encode_blocks_from /
decode_blocks_from, and the SIMD chunk replay in decode_chunk_by_block.
A buffer that runs short falls through to the tail again, as before the
NEON kernel change, instead of returning early.

The NEON lookup helpers had no safety preconditions, so they are safe fns
now; every NEON load and store has a SAFETY comment. The mismatched-buffers
test moves to its own section, since it runs on every arch.
- scripts/mutants.sh holds the per-arch excludes; CI and mise both call it.
- A PR that changes src/ or tests/ but whose changed lines yield no
  mutants (a rename, a moved function) sweeps the whole crate instead of
  passing without testing anything.
- sanitizers.yml runs the suite under ASan on aarch64, covering the NEON
  path Miri can't interpret.
- mise miri runs the strict models too, matching miri.yml.
- Stale comments fixed; CHANGELOG grouped with re-measured numbers.
@coderdan
coderdan requested a review from a team October 23, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants