Mikeo/lowmem improvements

.claude/skills/memory-hygiene-in-rust/references/case-studies.md

@@ -0,0 +1,120 @@
+# Case studies behind the rules (bc-rust, September 2026)
+
+Every number is a massif peak in bytes on the `mem_usage_benches` harness, release build, x86_64,
+measured before and after a single change. "Full" is `bouncycastle-mldsa` / `-mlkem`; "lowmem" is
+the `-lowmemory` crate.
+
+## 1. "Only 32 bytes" was not free
+
+`make_hint_row` in mldsa-lowmemory took `out: &mut HintRow` (32 bytes) and returned the weight.
+Changing it to return `(HintRow, i32)` by value, on the argument that 32 bytes cannot matter, added
+exactly **+16 bytes** of peak on all three lowmem sign benches. Returning just `HintRow` and computing
+the weight with popcount: also +16. Reverted. The sibling `unpack_h_row` on the verify path went to
+`Option<HintRow>` at measured **0** bytes, because it was already the deepest frame there. Same
+type, same size, opposite results: the frame context decides, not the type.
+
+## 2. Decaps benches were measuring keygen
+
+Every ML-KEM decaps bench called `keygen_from_seed` inside the measured binary. Hard-coding the
+encoded private key instead:
+
+| | old | in-main decode | helper decode |
+|---|---|---|---|
+| ML-KEM-512 decaps | 24136 | 28408 | 21592 |
+| ML-KEM-768 decaps | 39800 | 44072 | 33416 |
+| ML-KEM-1024 decaps | 63800 | 62792 | 49224 |
+
+The "in-main" column is the first attempt, with the key byte array and `from_bytes` in the bench
+body: worse than keygen for 512 and 768, because the array and the `Result` temporary stayed live
+under decaps. Moving the load into an `#[inline(never)]` helper gave the third column. The old table
+had over-reported decaps by up to 14.6 kB.
+
+## 3. Sign 44 went up 8.7 kB with no code change in sign
+
+After item 2's helper pattern was applied to ML-DSA, `Sign/ML-DSA-44` rose 93720 → 102456 with
+identical instruction counts. Frame remarks: the bench `main` was 73608 bytes in both builds with
+sign fully inlined into it. The key-load helper (12.5 kB) and `from_bytes` (14.6 kB) were being
+called from that `main`, so they stacked on top of the already-allocated 73.6 kB frame. Fix: run the
+operation in a separate non-inlined, non-returning closure so load and op are sibling frames.
+Residual after the fix: **+2.1 kB**, the boundary's own spills. A wrapper that *returned* the
+signature cost 4.6 kB instead: the 2.4 kB result was copied across the boundary.
+
+## 4. Plain/expanded pairs reporting identical peaks
+
+Before the harness rework, every `Encaps`/`Encaps_expanded_pk` pair and every
+`Sign`/`Sign_expanded_sk` pair reported byte-identical peaks. That only happens when the key decode
+in the bench body, not the operation, sets the peak. Treat identical numbers across variants that
+do different work as a harness bug, not a coincidence.
+
+## 5. A heap `Vec` deleted 3 to 5 kB from two table rows
+
+`bench_mldsa65_lowmemory_verify` and the 87 variant decoded their signature with `hex::decode` into
+a `Vec<u8>`. Massif `--heap=no` ignores the heap, so those rows under-reported by one signature
+each (3309 and 4627 bytes) relative to the 44 row, which used a stack array. Converting them moved
+the rows 15864 → 19096 and 17784 → 22328.
+
+## 6. `sig_decode` returned an 11 kB tuple
+
+`fn sig_decode(sig) -> Result<(SigCTilde, VecL, VecK), ()>`. When LLVM inlined it, fine. When fed a
+runtime-length slice it did not inline it, and the remarks showed three copies of the 11.3 kB tuple
+live at once (local, return slot, destructured bindings) plus `sig_decode`'s own 23.6 kB frame under
+verify: **+18 to +24 kB** depending on caller shape. Out-parameters (`c_tilde: &mut, z: &mut,
+h: &mut`, returning `Result<(), ()>`) removed it structurally: full-crate verify −6 to −20 kB across
+parameter sets, 44 sign −9 to −12 kB.
+
+## 7. `Matrix::new()` built a 56 kB temporary through `array::map`
+
+`Self { elems: [[(); l]; k].map(|_| [(); l].map(|_| Polynomial::new())) }` goes through
+`core::array::drain::drain_array_with`, which materialises the full matrix before copying it. It was
+invisible until the fix in item 6 changed inlining and a **57368-byte** `drain_array_with` frame
+appeared under `expandA`, adding 47 kB to one bench. Repeat expression `[[Polynomial::new(); l]; k]`
+(elements are `Copy`, `new` is `const`) removed it. The same pattern in the full `mlkem` crate was
+*not* being materialised (its matrix is 4 to 16 kB and LLVM folded it); changing it there moved
+three table rows up 2 to 3 kB from inlining alone, so it was reverted. Structural correctness did
+not win over measurement.
+
+## 8. An unused `match` arm cost 57 kB
+
+```rust
+match a_hat {
+    Some(a) => sign_internal(sk, a, ...),
+    None => { let mut a = Matrix::new(); sk.expand_into(&mut a); sign_internal(sk, &a, ...) }
+}
+```
+The `None` arm's local is allocated in the frame on both paths, so every expanded-key caller paid
+for a matrix it never used (`sign_mu_deterministic` frame 62 kB). Fix: the arm calls an
+`#[inline(never)]` helper that owns the local. Cost: 5 to 6 kB on the plain paths for the extra
+boundary, which was accepted.
+
+## 9. Converting a return to an out-parameter added a copy
+
+`fn A_hat(&self) -> M { expandA(&self.rho) }` was a tail call; LLVM forwarded the return slot into
+`expandA`, so only `expandA`'s own local existed. After `expandA` took `&mut M`, `A_hat` became
+`let mut m = M::new(); expandA(&self.rho, &mut m); m`, and LLVM did not elide the move: the by-value
+`A_hat()` path gained a full extra matrix. Keygen and plain verify improved by 13 to 49 kB and 12 to
+50 kB from the same change, expanded-key construction regressed by 10 to 53 kB. Constructors of the
+form `let mut s = Self { big: M::new(), .. }; fill(&mut s.big); s` showed three copies; returning a
+struct literal built from locals showed two. Getting to one copy for a by-value constructor was not
+achievable in safe Rust without an out-parameter API; that became a follow-up.
+
+## 10. The deleted reduction
+
+Not a memory case, but found with the same tools and worth keeping next to them. A plain `reduce32`
+before `inv_ntt` existed from the first ML-DSA commits, was commented out on 2026-03-20 because
+`cargo mutants` and the full bc-test-data set passed without it, and was deleted the next day. In
+September 2026 Wycheproof's `MissingReduction` vectors showed the 8-level inverse-NTT butterflies
+overflow `i32` when fed an unreduced sum of `l+1` Montgomery products: a valid signature rejected and
+a forgery accepted in release. Restored at all nine accumulation sites in both crates; Criterion
+showed the cost below the noise floor (median −0.7 % across 30 benches, one +5 % outlier that
+re-ran at −1.3 %). Bound-keeping steps are dead code to every test that uses honest inputs.
+
+## Measured noise floors on this harness
+
+| Crate family | Layout noise between builds |
+|---|---|
+| lowmem (ML-KEM, ML-DSA) | ≤ 0.2 kB |
+| full ML-KEM | 1 to 4 kB |
+| full ML-DSA | 2 to 9 kB |
+
+Massif itself is deterministic: identical binaries give identical peaks. The noise is in what the
+compiler does with a differently shaped caller, not in the measurement.

.claude/skills/memory-hygiene-in-rust/scripts/frame_layout.py

@@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+"""Summarise rustc/LLVM stack-frame remarks: per-function frame sizes and large stack objects.
+
+Build with (stable toolchain is fine):
+    RUSTFLAGS="-C remark=stack-frame-layout -C remark=prologepilog -C debuginfo=1" \
+      CARGO_TARGET_DIR=/tmp/remarks cargo build --release -p <crate> --bin <bin> 2> remarks.txt
+then:
+    frame_layout.py remarks.txt [name-regex] [--min-object BYTES] [--top N]
+
+Prints the largest frames ('N stack bytes in function' from the prologepilog remark) and, for each
+matching function, its stack objects at or above --min-object (from the stack-frame-layout remark).
+Names are demangled crudely from the v0 scheme. Start with NO name filter: the copy you are looking
+for is often in a std/core frame such as core::array::drain::drain_array_with.
+"""
+import re, sys
+
+def demangle(sym):
+    parts = []
+    for m in re.finditer(r"(\d+)(_?)([A-Za-z_][A-Za-z0-9_]*)", sym):
+        n = int(m.group(1)); ident = m.group(3)[:n]
+        if len(ident) == n and not ident.startswith("Cs"):
+            parts.append(ident)
+    return "::".join(parts) if parts else sym
+
+def main():
+    args = sys.argv[1:]
+    if not args:
+        print(__doc__); sys.exit(1)
+    path = args.pop(0)
+    min_obj = 4096; top = 20; pattern = None
+    while args:
+        a = args.pop(0)
+        if a == "--min-object": min_obj = int(args.pop(0))
+        elif a == "--top": top = int(args.pop(0))
+        else: pattern = re.compile(a)
+    text = open(path, errors="replace").read()
+    text = re.sub(r"_R[A-Za-z0-9_]+", lambda m: demangle(m.group(0)), text)
+    text = re.sub(r"_ZN[0-9]+_?", "", text)
+    text = re.sub(r"17h[0-9a-f]{16}E", "", text)
+    for a, b in (("$LT$", "<"), ("$GT$", ">"), ("$u20$", " "), ("$C$", ","), ("$RF$", "&"), ("..", "::")):
+        text = text.replace(a, b)
+
+    frames = {}
+    for m in re.finditer(r"(\d+) stack bytes in function '([^']+)'", text):
+        frames[m.group(2)] = max(frames.get(m.group(2), 0), int(m.group(1)))
+    objects = {}
+    cur = None
+    for line in text.splitlines():
+        m = re.match(r"\s*Function: (.*)", line)
+        if m: cur = m.group(1).strip(); objects.setdefault(cur, []); continue
+        m = re.match(r"\s*Offset: \[SP[-+]\d+\], Type: (\w+), Align: \d+, Size: (\d+)", line)
+        if m and cur is not None:
+            objects[cur].append((int(m.group(2)), m.group(1)))
+
+    noise = re.compile(r"backtrace|gimli|driftsort|panicking|rustc_demangle|std::sys|addr2line|miniz")
+    rows = sorted(((sz, n) for n, sz in frames.items() if not noise.search(n) and (pattern is None or pattern.search(n))), reverse=True)
+    print(f"largest frames (top {top}):")
+    for sz, n in rows[:top]:
+        print(f"{sz:9d}  {n[:140]}")
+    print(f"\nstack objects >= {min_obj} bytes:")
+    for sz, n in rows[:top]:
+        big = sorted((o for o in objects.get(n, []) if o[0] >= min_obj), reverse=True)
+        if big:
+            print(f"  {n[:120]}")
+            for osz, kind in big:
+                print(f"      {osz:8d}  {kind}")
+
+if __name__ == "__main__":
+    main()

.claude/skills/memory-hygiene-in-rust/scripts/massif_peak.sh

@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+# Peak stack of one or more mem_usage_benches functions, via valgrind massif.
+#
+#   massif_peak.sh <bin> <label> <bench_fn>...
+#   e.g. massif_peak.sh bench_mldsa_mem_usage after bench_mldsa44_sign bench_mldsa44_verify
+#
+# For each function it rewrites main() of mem_usage_benches/<bin>.rs to call only that function,
+# builds --release, runs the binary under massif with --heap=no --stacks=yes, and prints
+# "<label> <fn> <peak_bytes>". The bench source is restored afterwards (also on failure).
+# Per-run artifacts go to $MASSIF_OUT (default /tmp/massif_peak): the massif file, stdout and stderr
+# of the bench, so outputs can be diffed between labels (signatures must be identical, verifies
+# must succeed) and the massif time series can be inspected:
+#   awk -F= '/^time=/{t=$2} /^mem_stacks_B=/{print t, $2}' $MASSIF_OUT/massif_<label>_<fn>.out
+#
+# Run from the repository root. Do not edit the bench source while this is running.
+set -euo pipefail
+BIN=$1; LABEL=$2; shift 2
+OUT=${MASSIF_OUT:-/tmp/massif_peak}; mkdir -p "$OUT"
+F="mem_usage_benches/$BIN.rs"
+SAVED="$OUT/${BIN}_saved_${LABEL}.rs"
+cp "$F" "$SAVED"
+trap 'cp "$SAVED" "$F"' EXIT
+for fn in "$@"; do
+  cp "$SAVED" "$F"
+  python3 - "$F" "$fn" <<'PY'
+import sys
+path, fn = sys.argv[1], sys.argv[2]
+src = open(path).read()
+i = src.index("fn main() {")
+open(path, "w").write(src[:i] + "fn main() {\n    " + fn + "()\n}\n")
+PY
+  cargo build -q --release -p mem_usage_benches --bin "$BIN"
+  massif="$OUT/massif_${LABEL}_${fn}.out"
+  valgrind --tool=massif --heap=no --stacks=yes --massif-out-file="$massif" \
+    -- "target/release/$BIN" >"$OUT/out_${LABEL}_${fn}.txt" 2>"$OUT/err_${LABEL}_${fn}.txt" || true
+  peak=$(grep '^mem_stacks_B=' "$massif" | cut -d= -f2 | sort -n | tail -1)
+  echo "$LABEL $fn $peak"
+done

CLAUDE.md

@@ -142,6 +142,13 @@ Repo mechanics behind those rules, which the documents don't spell out:
   covers) -- there, read the whole input once and process it in place, rather than adding a second
   buffer the size of the input on top of it; see `aes_ccm_cmd.rs`.
 - Trait → factory → CLI is the wiring path for a new primitive; see [the workspace architecture](#the-core--core-test-framework--factory-spine) above for the crates involved.
+- **Comments describe the code that is there, not the road that led to it.** Do not add a comment
+  explaining a transient design decision — an approach that was tried and abandoned, what an earlier
+  version did, why one formulation was chosen over another that is no longer present — or describing
+  a design the code does not use. Such comments are noise: they age badly, and a reader has to work
+  out that they describe nothing in front of them. A comment that explains why the present code is
+  the way it is, and that a naive edit would break it (a spec step, an invariant, a constraint), is
+  wanted; a comment narrating how it got that way is not.
 
 ## Scope of changes
 

QUALITY_AND_STYLE.md

@@ -16,6 +16,13 @@ All algorithms with a state that is exercised across multiple API calls -- typic
 `do_final()` pattern -- should implement SerializableState so that the user can pause the execution of this algorithm to
 a cache and resume later.
 
+The library in general is meant to serve both embedded systems as well as workstations and servers. In a no std, no
+alloc build all variables are stack variables which warrants particular attention to memory hygiene. A Claude Fable
+skill is available in `.claude/skills/memory-hygiene-in-rust` to assist with this. Note that "memory hygiene" (good
+coding practices that reduce stack usage at no performance cost) can apply to all crates and is different from "low
+memory" (such as the mldsa-lowmemory crate), which uses a different algorithm that trades significant performance loss
+for significant reduction in memory footprint.
+
 # Quality
 
 ## Tests

alpha_0.1.3_release_notes.md

@@ -21,3 +21,4 @@
       existing test used
       `0xFF`, which masked the second error.
     * Changed the order of bits when absorbing a final partial byte to match ASN.1 DER BIT_STRING bit ordering.
+* Further reductions to the memory usage of the mldsa-lowmemory and mlkem-lowmemory crates. 

crypto/mldsa-lowmemory/src/aux_functions.rs

@@ -6,7 +6,7 @@ use crate::params::{
     GAMMA1_2_POW_17, GAMMA1_2_POW_19, GAMMA2_Q_MINUS_1_OVER_32, GAMMA2_Q_MINUS_1_OVER_88,
     MLDSAParams,
 };
-use crate::polynomial::Polynomial;
+use crate::polynomial::{HintRow, Polynomial, ZEROED_HINT_ROW, hint_set};
 use bouncycastle_core::traits::{Hash, XOF, XOFSqueezer};
 use bouncycastle_utils::secret::ZeroizablePrimitive;
 
@@ -358,13 +358,15 @@ pub(crate) fn unpack_z_row<P: MLDSAParams, const SIG_LEN: usize>(
     if z.check_norm(P::gamma1_minus_beta) { Err(()) } else { Ok(z) }
 }
 /// Part of unpacking the sig value
+///
+/// Returns the decoded row, or `None` if the encoded hint is malformed.
 pub(crate) fn unpack_h_row<P: MLDSAParams, const SIG_LEN: usize>(
     row: usize,
     sig: &[u8; SIG_LEN],
-) -> Option<Polynomial> {
+) -> Option<HintRow> {
     debug_assert!(row < P::k);
 
-    let mut h = Polynomial::new();
+    let mut out = ZEROED_HINT_ROW;
 
     // skip over the other stuff in the encoded sig value
     let pos = P::C_TILDE_LEN + P::l * P::POLY_Z_PACKED_LEN;
@@ -401,7 +403,7 @@ pub(crate) fn unpack_h_row<P: MLDSAParams, const SIG_LEN: usize>(
             return None;
         }
         // 12: 𝐡[𝑖]_𝑦[Index] ← 1
-        h[sig[pos + j] as usize] = 1;
+        hint_set(&mut out, sig[pos + j] as usize);
 
         // 13: Index ← Index + 1
         //  > done by for loop
@@ -418,7 +420,7 @@ pub(crate) fn unpack_h_row<P: MLDSAParams, const SIG_LEN: usize>(
         }
     }
 
-    Some(h)
+    Some(out)
 }
 
 /// Algorithm 29 SampleInBall(𝜌)
@@ -546,12 +548,22 @@ pub(crate) fn rej_bounded_poly<P: MLDSAParams>(rho: &[u8; 64], nonce: &[u8; 2])
     h.do_update(rho);
     h.do_update(nonce);
 
-    // SHAKE is fairly inefficient if only 3 bytes are squeezed at a time, so the implementation does a block instead.
-    // size is not a limitation as long as it is a multiple of 3.
-    // 312 seems to be the sweet spot after some experimentation
-    // which is possibly also related with the average rejection rate.
-    // Also, 312 is a multiple of 8 (efficient for SHAKE)
-    let mut z_arr = [0u8; 312];
+    // Deviation from FIPS 204, Algorithm 31 step 5, which squeezes one byte per loop iteration:
+    // H is SHAKE256, which produces a whole 136-byte block per Keccak permutation, so squeezing a
+    // byte at a time wastes most of each block. The squeeze is buffered instead, and the refill
+    // below makes the byte stream — and therefore the output — identical to the spec's.
+    //
+    // 272 is exactly two SHAKE256 blocks (2 × 136), so filling the buffer costs two permutations
+    // with nothing stranded in the sponge's output queue, and it covers the whole polynomial in a
+    // single squeeze almost always. Per FIPS 204 §C, each iteration consumes one byte and yields
+    // Binomial(2, θ) coefficients, θ = 15/16 for η = 2 and 9/16 for η = 4. The worst case is
+    // η = 4 (ML-DSA-65): 228 bytes needed on average, and over 300k simulated seeds the largest
+    // requirement was 276 bytes, so the refill runs for roughly 1 seed in 100,000. For η = 2
+    // (ML-DSA-44/87) it is 137 bytes on average and never exceeded 150.
+    //
+    // This is a buffer, not the iteration cap of FIPS 204 Table 3 (481 bytes for RejBoundedPoly):
+    // the loop refills rather than giving up, so no cap is imposed.
+    let mut z_arr = [0u8; 272];
     let mut h = h.into_squeezer();
     h.do_output_out(&mut z_arr);
     let mut idx: usize = 0;