Repository navigation
Challenge 29: Kani contracts for Box, convert, and ThinBox - #639
sankalpsthakur wants to merge 16 commits into
Conversation
Kani contracts and harnesses for verify-rust-std challenge. Fixes rust-lang#526
Place #[cfg(kani)] use core::kani with neighboring core uses and group
use core::{fmt, kani} so the upstream_test format check passes.
Autoharness macos/ubuntu failed on check_downcast_any, check_downcast_error, and check_from_slice_clone (CBMC timeout). Match the passing sibling proofs: is_ok/is_err only, fixed-length Clone from_slice with unwind(3). No runtime stdlib change. Fixes rust-lang#526
|
Ready for committee FCFS review of Challenge 29 (in-window, end 2026-12-31). Head
Tracking Fixes #526. No second solution PR. |
Documented Box reconstruction requires more than non-null dereference: size must fit isize::MAX. Encode that with a kani-only layout check on from_raw / from_non_null / from_raw_in / from_non_null_in. Use symbolic slice lengths (capped at 2) on reconstructors and slice constructors, and add from_raw_in / from_non_null_in slice harnesses.
proof_for_contract allows one top-level from_raw_in call. Box::from already goes through from_raw_in, so macos partition 2 failed check_from_raw_in_slice. Mirror alloc_write for [u8].
feliperodri
left a comment
There was a problem hiding this comment.
Approving — leading solution for Challenge 29
Thanks @sankalpsthakur. After reviewing all three open Challenge 29 (boxed) solutions with our vacuity tooling and local Kani (pinned 0.67.0 / CBMC 6.8.0), this is complete, sound, and the most concise. Prioritizing it.
Coverage against Ch29 criteria:
- A: 9/9 unsafe fns with contracts + verified.
assume_init(sized+slice),from_raw,from_non_null,from_raw_in,from_non_null_inare verified via#[kani::proof_for_contract](6). The 3downcast_uncheckedmethods carry contracts and are exercised via plain#[kani::proof]with the precondition mirrored bykani::assume— Kani cannot resolveproof_for_contracton a trait-object generic method (Box<dyn Any>::downcast_unchecked::<T>), the documented/accepted workaround. Note the 3 challenge-listed<dyn Error>::downcast_uncheckeddon't exist in std — the real methods areBox<dyn Any(+Send)(+Sync)>::downcast_unchecked, which you correctly verified (disclosed in the PR). - B: 45/46 safe abstractions (97.8%, ≥ 75%). Only
<Box<[T;N]> as TryFrom<Box<T>>>::try_from(which doesn't exist in-tree) is uncovered.
Soundness (all clean):
- T1: no cfg body swaps — 906 lines pure additions, no
-lines; the only cfg-gated code is thebox_ptr_fits_box_layouthelper used inrequiresclauses. - T2: no trivial invariants, no
loop_invariant(true); slice iteration usesunwind(4)for len ≤ 2 (non-vacuous). - T7: 6/9 unsafe fns +
boxed_slice_as_array_uncheckedhaveproof_for_contract; the 3downcast_unchecked+WithHeader::dropare assume-mirror (Kani trait-object limitation, disclosed). Autoharness allowlist unchanged. - Not assume-the-conclusion — downcast harnesses build
Box::new(concrete)then downcast to the same type; theis::<T>()witness is real, not assumed. - Inputs symbolic (
kani::any()); primitive-mono is challenge-allowed; no unbounded clause. No runtime std logic changed.
Local Kani sample (CBMC 6.8.0): 4/4 VERIFICATION SUCCESSFUL — check_assume_init_i32 (proof_for_contract), check_from_raw_i32 (proof_for_contract), check_downcast_unchecked_any, check_leak.
Minor note (non-blocking): the 3 Box<dyn Any>::downcast_unchecked contracts and WithHeader::drop rely on assume-mirror rather than proof_for_contract (Kani limitation) — worth a tracking note so they can be upgraded if kani gains trait-object contract resolution. Complete and sound as-is.
|
Merged current main in c454b54 and resolved the boxed conversion import conflict. The alloc crate compiles on nightly-2026-02-05 and the three changed files pass rustfmt with the pinned upstream configuration. The contracts and harnesses are unchanged; full Kani verification on this updated base remains pending. |
|
CI caught two harnesses still referencing BoxFromSlice, which current main removed. Updated them to use Box::from in cdfeac6, preserving the inputs and assertions. The full proof checks are rerunning; no proof pass is claimed yet. |
|
@feliperodri Current main made |
Resolve the conflicts from the nightly-2026-09-25 subtree update in boxed.rs and boxed/thin.rs. Keep upstream's const `assume_init`, its `# Safety` doc on `WithHeader::drop`, and the new `Rng for Box` and `From<T> for ThinBox<T>` impls, with the existing contracts and `mod verify` blocks after them. The Kani build needs the follow-up commit that adapts the contracts and harnesses to the new APIs.
- `Box<MaybeUninit<T>, A>::assume_init` is now a `const fn`. With its contract attached, the Kani build fails with E0493 because the owned `Box` receiver's destructor cannot be evaluated at compile time. Keep the contract as a commented TODO, and turn `check_assume_init_i32`, `_zst` and `_bool` into plain proofs that assume its precondition and assert its postcondition around the call, like the `transmute_unchecked` harnesses in `core::intrinsics`. - `Box::into_array` now returns `Result`, so update `check_into_array`. - Remove `check_into_unique`, since `Box::into_unique` no longer exists.
The slice harnesses share the `check_assume_init_` prefix but still use `proof_for_contract`, so list the three sized ones explicitly.
|
at e5c65ce, the now-const |
|
kani metrics red is the typ.rs:777 crash in stdarch sve, same on main since oct 6, unrelated to this pr |
The three `Box<dyn Any (+ Send) (+ Sync), A>::downcast_unchecked` impls live in `boxed/convert.rs`, outside `Box`'s own module, and Kani's contract resolver rejected every spelling of them until model-checking/kani#4778 (merged 2026-09-24), which the pinned Kani `6ce55b95437a` includes. Target the three harnesses with `proof_for_contract` so the existing `requires`/`ensures` are checked rather than exercised by plain proofs, and drop the stale comment about the resolver. Harness bodies and contracts are unchanged.
|
@sankalpsthakur, re-reviewing the changes since my approval: all 71 harnesses verify locally with the pinned Kani. One request: none of them has a |
feliperodri
left a comment
There was a problem hiding this comment.
Requesting changes on the commits since my approval. Two harnesses can't reach the case they're meant to rule out, and nothing in the suite shows which paths are reached.
What I checked locally with the pinned Kani (6ce55b9): all 71 harnesses verify. Criterion 1: 8 of the 9 unsafe functions have contracts verified with proof_for_contract (from_raw, from_non_null, from_raw_in and from_non_null_in for i32 and [u8], the slice assume_init, and the three downcast_unchecked). Moving downcast_unchecked to proof_for_contract is an improvement. Criterion 2: every listed function except the removed into_unique has a harness, well above 75%. The diff is additive (+774/−0).
Mutation testing:
| Mutant | Result |
|---|---|
Box::write skips the write |
killed by check_write |
ThinBox drop frees ptr - value_offset - 1 |
killed by 10 harnesses |
into_array accepts len >= N |
survives |
TryFrom<Box<[T]>> accepts len >= N |
survives |
The two survivors are the inline comments below. With no kani::cover anywhere (see my earlier comment), this kind of gap doesn't show up in the results.
On the sized assume_init: I put the commented-out contract back and Kani fails with error[E0493]: destructor of Box<MaybeUninit<T>, A> cannot be evaluated at compile-time, so the TODO is right that this is a Kani limitation, and the mirror harnesses are the best available. Could you open a Kani issue for it and link it from the TODO, so the contract comes back when that's fixed?
What would flip this to approval: the two harnesses below reaching len > N, and covers on the cases the harnesses target.
|
@sankalpsthakur any news? |
Each harness with a symbolic length covers an empty and a full slice, each downcast harness covers its Ok and Err results, and each harness that assumes a precondition covers that it is reached. The assume_init TODO links model-checking/kani#4984.
|
working on it. |
on my fork, with main merged (Kani cd355b0) and only the
the other 33 harnesses have no length, result or precondition that picks a path, so they have no case to cover. a heads-up: on my fork |
feliperodri
left a comment
There was a problem hiding this comment.
Re-verified on 5f0e638 with the pinned Kani: all 71 harnesses pass, all 64 covers are satisfied, and both mutants from my last review (into_array and TryFrom<Box<[T]>> accepting len >= N) now fail. Thanks for the quick turnaround.
Approving. The one remaining gap is the contract on the sized Box<MaybeUninit<T>>::assume_init, which Kani can't attach today (E0493, model-checking/kani#4984); the mirror harnesses cover it in the meantime. We'll add the contract back once kani#4984 is fixed.
|
@sankalpsthakur would you be willing to write a blog post about this proof for https://model-checking.github.io/kani-verifier-blog/? |
|
thanks for the careful review, the mutation run made the harnesses better. yes, i'd be glad to write it. i'll send you a draft |
| // SAFETY: every element must be a valid `T`. Casting `[MaybeUninit<T>]` to | ||
| // `[T]` and requiring `can_dereference` encodes that for Kani. |
There was a problem hiding this comment.
Ran all 71 harnesses at the pinned Kani: all verify and all 64 covers are reached. I also re-ran the len >= N mutants for into_array, TryFrom<Box<[T]>> and TryFrom<Vec<T>>, and each is killed by its own harness. One thing about this contract: can_dereference only checks initialization under -Z uninit-checks (otherwise Kani treats is_initialized as true), and CI doesn't pass that flag. So in CI this requires doesn't rule out uninitialized elements: a proof_for_contract calling Box::<[MaybeUninit<u8>]>::new_uninit_slice(len).assume_init() without writing anything still verifies. With -Z uninit-checks, Kani reports [MaybeUninit<u8>] as unsupported (union layout), so neither setup checks it today. Not blocking, since it's a Kani limitation, but could you reword "encodes that for Kani" here and "covers aligned/initialized" on box_ptr_fits_box_layout (line 213) to say initialization stays the caller's obligation?
…racts can_dereference checks initialization only under -Z uninit-checks, which CI does not pass, so the contracts of the raw-pointer constructors and the slice assume_init do not rule out uninitialized elements.
Resolves #526 (Challenge 29: Box, convert and ThinBox).
Add safety contracts and Kani harnesses using the repository's
safety::{requires, ensures}annotations. Runtime stdlib logic is unchanged.Coverage includes assume-init methods, raw/non-null reconstructors, ThinBox operations, slice constructors, downcast paths and pinning. Slice harnesses use bounded symbolic lengths. Reconstructor contracts check dereferenceability and layout bounds; allocator provenance remains a caller obligation that these contracts cannot express.
The challenge's unchecked
dyn Errormethods do not exist in this tree. The actualBox<dyn Any>downcast_uncheckedvariants carry contracts and are proved withproof_for_contract, which needs model-checking/kani#4778 (in the pinned Kani). The listedTryFrom<Box<T>>implementation also does not exist;TryFrom<Vec<T>>is covered separately.Validation
On the current head
4712f513d5, all 27 completed GitHub checks pass (2 skipped). The earlier run 32560286046 at4da62f00d5reported 1,447 (Ubuntu) and 1,446 (macOS) verified autoharness harnesses with zero failures. These are run-level results, not counts of new harnesses in this PR.AI coding tools assisted with the contracts, harnesses and this description.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.