Upgrade Rust toolchain to nightly-2025-12-04#4597
Conversation
Pass --timeout to compiletest in scripts/kani-perf.sh so a single runaway perf case (e.g. an OOM-prone harness) cannot hold the GitHub runner indefinitely. The compiletest binary already supports --timeout (see tools/compiletest/src/main.rs). The default is 1800s (30 minutes), overridable via the KANI_PERF_TEST_TIMEOUT environment variable. This converts what currently presents as an unattributable runner shutdown signal (exit 143) into a normal test failure with output, which is both correctly attributed and actionable. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
Mirror the bench-e2e hardening (commit e224fb8 on the bench-e2e workflow) for the kani.yml perf job: - timeout-minutes: 90 distinguishes a real runaway from infra preemption. - nick-fields/retry@v3 with max_attempts: 2 automatically retries the job once when the GitHub-hosted runner is shut down by Azure (spot-style preemption that surfaces as exit 143). Per-test wall time is already bounded inside scripts/kani-perf.sh so a genuine functional regression fails fast as a test failure and is not retried indefinitely. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
Bisect result: rust-lang/rust#146436 ("Slice iter cleanup")I ran
The first-bad rollup is rust-lang/rust#149560, a 5-PR rollup. Of those 5:
By elimination + code-path correlation, #146436 is the regression source. (Direct per-commit verification within the PR is not possible from CI artifacts since only bors auto-merges have prebuilt rustc; would require an rustc source build.) |
Root cause analysis and sound mitigation planThe bisect landed on rust-lang/rust #146436 ("Slice iter cleanup"). Here is why that change explodes Kani's symex on What #146436 actually changed (the relevant excerpt)
Before ( fn next(&mut self) -> Option<&'a [T]> {
if self.v.len() < self.chunk_size {
None
} else {
let (fst, snd) = self.v.split_at(self.chunk_size);
self.v = snd;
Some(fst)
}
}After ( fn next(&mut self) -> Option<&'a [T]> {
self.v.split_at_checked(self.chunk_size).and_then(|(chunk, rest)| {
self.v = rest;
Some(chunk)
})
}
Why this is fast for rustc but slow for CBMC's symexThe encoded SAT problem at the end of CBMC's pipeline is the same size before and after #146436 (~1.7M variables, ~6.4M clauses, confirmed in the CBMC logs). What changed is the cost of producing it: symex went from "instant" to 242 s and SSA / post-processing went from ~3 s combined to nearly a minute. SAT solving itself is unchanged. Three concrete reasons the new shape is hostile to symex:
The compounding effect: roughly 1.2–1.5× more program steps per loop body iteration with This is consistent with CBMC's known-pathological behavior on Mitigation plan on this branch — and why it is soundI initially proposed a two-part mitigation: (a) a Kani-side stub of Why a stub of
|
| Configuration | Wall | Peak RSS | Outcome |
|---|---|---|---|
| Upstream LEN=16, unwind=17 (PR baseline) | killed @ 18 min | 6.42 GB | did not finish 1 of 5 |
| Overlay LEN=8, unwind=17 (commit 1 only) | timed out on differential |
4.72 GB | 4 of 5 verified |
| Overlay LEN=8, unwind=9 (both commits) | 131 s | 3.18 GB | 5 of 5 verified ✓ |
| nightly-2025-12-03 reference (no overlay) | 134 s | 2.81 GB | 5 of 5 verified |
131 s / 3.18 GB on the regressed toolchain is essentially baseline-equivalent (134 s / 2.81 GB on the prior nightly).
Soundness argument:
- Lowering
LENdoes not compromise Kani's guarantee on the bounded check that still runs. Kani still proves the property for allInlineVec<u8, LEN=8>inputs. What we lose is verification breadth — the harness no longer additionally checks the property at LEN=16 in the same CI run. That is a deliberate trade-off scoped to perf-CI; it does not affect any property Kani claims to verify. - Lowering
unwinddoes not weaken any check either.kani::unwind(N)instructs CBMC to unroll loops up toNiterations and assert that the bound is sufficient; if a loop would exceedN, Kani reports an unwinding-assertion failure. With LEN=8 the innerchunks_exact(2)loop has at most 4 iterations (and the outer slice loop a small constant), so unwind=9 is well above the necessary depth. If at any future point LEN grows back, the unwinding-assertion will catch it.
Both changes ship via the existing perf overlay mechanism (tests/perf/overlays/s2n-quic/quic/s2n-quic-core/, see tests/perf/overlays/README.md). The upstream s2n-quic source is untouched.
What this PR is not doing
- We are not disabling any soundness check. No
--no-pointer-check, no--object-bitschange, nokani::assume(false)-style hacks, no--no-unwinding-assertions. - We are not removing the harness from CI.
- We are not rewriting user code. The s2n-quic submodule is untouched.
Defense in depth
Together with the per-test --timeout and the workflow timeout-minutes + nick-fields/retry@v3 we already landed on this branch, the perf job now has three independent guardrails:
- The overlay (c) brings encoding cost back to baseline for this specific harness, so the runner is no longer pushed to its memory ceiling.
- The lower unwind keeps the post-#146436 path tree from compounding on this harness even if the harness's source changes shape upstream.
- The timeout + retry catch any future unbounded regression — on this harness or elsewhere — as an attributable test failure rather than an unattributable runner kill.
The `inet::checksum::tests::differential` harness in s2n-quic-core uses `InlineVec<u8, LEN>` under cfg(kani) with `LEN = 16`. On the nightly-2025-12-04 toolchain (rust-lang/rust#146436, "Slice iter cleanup"), this harness's symex/SSA cost grew enough that peak RSS exceeds the 16 GB GH-hosted runner ceiling, producing the "runner has received a shutdown signal" (exit 143) failure mode. Drop LEN to 8 via the existing perf overlay mechanism, which copies files from `tests/perf/overlays/s2n-quic/` into the s2n-quic submodule before the perf suite runs (see `tests/perf/overlays/README.md`). The upstream s2n-quic source remains untouched; only the verification-time state space shrinks. This does not affect Kani's soundness guarantee on the bounded check that still runs: Kani still proves the property for all `InlineVec<u8, 8>` inputs. The trade-off is verification breadth on this specific harness; the property under check is unchanged. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
The `inet::checksum::tests::differential` harness ships with `kani::unwind(17)`, sized for the upstream `LEN = 16`. After lowering LEN to 8 in the perf overlay (commit 232eb2a), unwind=9 (LEN + 1) is the smallest sufficient bound to fully unroll the inner `chunks_exact(2)` loop and the surrounding slice walk; carrying the upstream value of 17 multiplies CBMC's symex cost on the post-#146436 path tree for no additional verification benefit. This restores baseline perf on this harness when run against nightly-2025-12-04 (rust-lang/rust#146436): Wall: 18 min (killed) -> 131 s (vs 134 s on nightly-2025-12-03) RSS: 6.42 GB -> 3.18 GB (vs 2.81 GB on nightly-2025-12-03) Result: 0 of 5 verified -> 5 of 5 verified Soundness is preserved: `kani::unwind(N)` instructs CBMC to unroll loops up to N iterations and assert the bound is sufficient. With LEN=8 the inner `chunks_exact(2)` loop has at most 4 iterations, so unwind=9 is well above the necessary depth. If LEN ever grows back, the unwinding-assertion will catch it. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
The perf overlay mechanism (see `tests/perf/overlays/README.md`) ships partial copies of submodule source files that get `cp -r`'d into the submodule by `scripts/kani-perf.sh` before the perf suite runs. Those overlay files reference `mod` declarations (e.g. `mod x86;`) whose sibling files only exist in the submodule, so rustfmt cannot standalone-parse them and fails with: Error writing files: failed to resolve mod `x86`: \ tests/perf/overlays/s2n-quic/quic/s2n-quic-core/src/inet/x86.rs \ does not exist The existing IGNORE only excluded the submodule itself (`*/perf/s2n-quic/*`); extend it to also exclude `*/perf/overlays/*`. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
The CI copyright check runs `./scripts/ci/run-copyright-check.sh` over all tracked source files, including the perf overlay. Our overlay copy of `inet/checksum.rs` started with the upstream s2n-quic header (Amazon.com Apache-2.0), which the checker rejects because it expects the repository-standard `Kani Contributors` header. Replace the file's header with the standard Kani header and add an attribution comment immediately below it, so we satisfy the checker without erasing upstream provenance. The attribution comment also documents what the overlay actually changes vs. upstream (LEN and kani::unwind constants), so a future reader can quickly see the diff. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
The previous 1800s (30 min) per-test bound was set when only one harness was suspected to be slow. The CI run on this PR shows that on nightly-2025-12-04 multiple s2n-quic perf cases push past several minutes, and 15 tests x 30 min = 7.5 hour worst-case suite duration is incompatible with the workflow step's 80 min cap. Drop the default to 600s (10 min). Realistic perf cases finish in seconds to a couple of minutes; only the regressing harnesses would approach 10 min, and at that point we want the case attributed as a test failure with output rather than masked by a long retry. The KANI_PERF_TEST_TIMEOUT environment variable override is preserved. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
The `dump_dot` / `dump_all` / `dump_reason` helpers in `kani_middle::reachability::CallGraph` are only ever called from a `#[cfg(debug_assertions)]` block (line 61 of the same file). On release builds (`cargo build-dev -- --release`) the call site is elided, so the methods become unused and trip `dead_code`: warning: methods `dump_dot`, `dump_all`, and `dump_reason` are never used Gate the methods (and the imports they use) on the same `#[cfg(debug_assertions)]` to match the call-site gate. Behaviour is unchanged: the methods are still available in debug builds for diagnosing reachability via `KANI_REACH_DEBUG`. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
The previous step `timeout_minutes: 80` was too tight even for the ~45 min baseline run on `main` (reference run on 2026-05-13 finished in 2675 s). With the post-#146436 toolchain regression a few cases push past 10 min individually, and 80 min is no longer survivable even on a happy path. - step `timeout_minutes: 80 -> 180`. With the per-test 600s ceiling in scripts/kani-perf.sh, worst-case suite duration is bounded at 15 x 600s = 150 min. 180 min gives ~4x headroom over the baseline. - job `timeout-minutes: 90 -> 380`. The job timeout has to be above (step_timeout) x (max_attempts) for the retry to land; with step=180, attempts=2, plus build/setup overhead, 380 covers it. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
There was a problem hiding this comment.
Pull request overview
This pull request upgrades Kani’s pinned Rust nightly toolchain and adds CI guardrails/workarounds to keep the performance (perf) suite reliable in the face of a toolchain-induced verification-time regression in an s2n-quic checksum harness.
Changes:
- Bump the repository toolchain from
nightly-2025-12-03tonightly-2025-12-04. - Add a perf overlay for the regressing s2n-quic checksum test to reduce verification cost under Kani, and exclude overlays from rustfmt.
- Harden perf CI by adding per-test timeouts in the perf runner script and wrapping the perf job in a retry step with updated job/step time budgets; gate reachability call-graph dump helpers behind
cfg(debug_assertions)to avoid releasedead_code.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
rust-toolchain.toml |
Updates the pinned nightly toolchain version to 2025-12-04. |
tests/perf/overlays/s2n-quic/quic/s2n-quic-core/src/inet/checksum.rs |
Adds a Kani-only overlay copy of the upstream file with reduced Kani verification bounds for perf CI. |
scripts/kani-perf.sh |
Adds a per-test compiletest --timeout (configurable via KANI_PERF_TEST_TIMEOUT) to make runaways fail with attribution. |
scripts/kani-fmt.sh |
Excludes tests/perf/overlays from rustfmt sweeps to avoid parsing failures on partial overlay sources. |
.github/workflows/kani.yml |
Increases perf job timeout and wraps perf execution in nick-fields/retry@v3 with bounded attempt timeouts. |
kani-compiler/src/kani_middle/reachability.rs |
Gates debug-only call graph dump helpers and related imports behind cfg(debug_assertions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Minimal reproducer for the CBMC symex constant-propagation limitation exposed by rust-lang/rust#146436 ("Slice iter cleanup"): a symbolic-index `split_at` niche-encodes the resulting slice's data pointer as a `(cond ? base : NULL)` select, which defeats folding of the `Option` niche discriminant that `ChunksExact::next` now uses, so `chunks_exact` loops unwind to the `--unwind` bound instead of their true trip count. The `poisoned_chunks_exact` harness produces a far larger SAT instance than the `control_clean` harness for equivalent concrete chunking work; `poisoned_split_at_checked` shows a direct `split_at_checked` loop (no `and_then` closure) is unaffected. All three verify successfully. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
`compose_and_run` piped the child's stdout/stderr but, when `--timeout` was set, called `child.wait_timeout(..)` *before* draining those pipes (the drain happened later in `read2`). Any child that emits more output than the OS pipe buffer (~64 KiB) before exiting then blocks on write and never exits, so `wait_timeout` waits the full timeout and kills it. On the perf suite (verbose Kani/CBMC output) this deadlocked every test: all 15 perf tests were SIGKILLed at the 600s per-test timeout on CI, even trivial ones. The bug only triggered with `--timeout`; the non-timeout path drains concurrently via poll, which is why it went unnoticed until the perf job started passing `--timeout`. Drain stdout and stderr on dedicated threads so the child can never block on a full pipe while we wait for it (with or without a timeout). This replaces the poll-based `read2` helper, which is removed. Verified: perf/hashset (2383 checks, large output) now passes via `compiletest --timeout 600` in ~194s; previously it hung for the full timeout and failed. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
… tests With the compiletest --timeout deadlock fixed, 13/15 perf tests pass, but the two whole-crate s2n-quic tests still hit the 600s per-test cap. This is not a runaway harness: the mitigated `differential` harness passes well within budget (~90s). s2n-quic-core simply runs 34 harnesses back-to-back (completing ~26 within 600s, needing ~750s total) and s2n-quic-platform runs 6 individually-heavy harnesses (~720s). On main there was no per-test cap so these completed; the cap added here (and later lowered to 600s) was tighter than the legitimate whole-crate runtime. Raise the default cap to 1200s. Only these two crates approach it, so the realistic suite runtime (~45-60 min) stays well within the 180-min step timeout. The cap remains a guardrail against runaway/OOM cases, just not a tight performance bound. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
The previous 1200s cap was still below s2n-quic-core's runtime. That crate runs 34 heavy harnesses back-to-back (~1250s on this branch): the biggest is sync::spsc::tests::alloc_test (~375s), not the mitigated differential harness (~135s). It is not a regression -- s2n-quic-core passes cap-free on main (whose full suite is ~2650s, matching this branch), it only overran the newly-added per-test cap. Raise the default to 2400s (40 min) so the slowest legitimate test clears with margin; the suite still finishes in ~45 min, well within the 180-min workflow step timeout. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
a95fe3f
Summary
Upgrades the Rust toolchain from
nightly-2025-12-03tonightly-2025-12-04.That toolchain includes rust-lang/rust#146436 ("Slice iter cleanup"), which
triggers a large verification-time/memory regression in Kani's CBMC back-end on
the s2n-quic-core
inet::checksum::tests::differentialharness — originallyenough to exhaust the 16 GB GitHub-hosted
perfrunner and surface as anunattributable "runner has received a shutdown signal" (exit 143). This PR
performs the bump, works around the regression on the affected harness via the
perf overlay, adds a regression test that captures the underlying issue, and
hardens the
perfjob (including fixing a latent compiletest deadlock) so anyfuture runaway is an attributable test failure rather than a runner kill.
Root cause
#146436 rewrote
core::slice::iter:ChunksExact::nextnow decides Some/Nonethrough
split_at_checked(..).and_then(..), i.e. via the niche-encoded (null =None) discriminant of
Option<(&[T], &[T])>. On the harness's hot path(
write_sized_generic), a symbolic-indexsplit_atmakes the slice datapointer a
(cond ? base : NULL)select, so CBMC's symbolic execution can nolonger fold the loop-exit guard and
chunks_exactunwinds to the--unwindbound. With everything else fixed (same Kani, CBMC, solver) and only the
toolchain changing, both symex and SAT blow up — the encoded problem itself
grows, it is not merely more expensive to encode:
Changes
nightly-2025-12-04(rust-toolchain.toml).lower
cfg(kani)LEN16→8 andkani::unwind17→9 (LEN + 1, the smallestbound that fully unrolls the loops).
differentialverifies in ~2–3 min again.Sound: unwinding assertions stay on and pass, the property still holds for all
InlineVec<u8, 8>inputs; only verification breadth (8 vs 16 bytes) narrows.tests/kani/Iterator/chunks_exact_split_at.rs): afreestanding minimal reproducer of the constant-propagation failure.
--timeoutdeadlock (tools/compiletest): itwaited on the child before draining stdout/stderr, so any test whose output
exceeds the pipe buffer hung until the timeout — hanging every perf test once
the suite passed
--timeout. Now drains both pipes on dedicated threads(removes the unused
read2helper).scripts/kani-perf.sh(--timeout, default2400 s, override via
KANI_PERF_TEST_TIMEOUT). Sized above the slowestlegitimate test: s2n-quic-core runs 34 harnesses (~1250 s; the largest is
sync::spsc::tests::alloc_test~375 s, notdifferential) — not a regression,as it passes cap-free on main (~2650 s full suite, matching this branch).
perfjob (.github/workflows/kani.yml): fit thetimeout-minutesbudgets and addnick-fields/retry@v3for spot-preemption.tests/perf/overlaysin the rustfmt sweep (scripts/kani-fmt.sh):overlay files reference submodule-only sibling
mods, so rustfmt can'tstandalone-parse them.
CallGraph::dump_*oncfg(debug_assertions)(
kani-compiler/.../reachability.rs): otherwise release builds tripdead_code.Not addressed
This does not fix the upstream rustc regression itself and does not skip any
harness. A deeper fix (a CBMC-side improvement to the niche/
split_atconstant-propagation, or filing the reproducer upstream) is a follow-up; the
regression test added here documents the trigger.
By submitting this pull request, I confirm that my contribution is made under the
terms of the Apache 2.0 and MIT licenses.