Challenge 26: Verify the safety of Rc and Weak in alloc::rc - #661
Challenge 26: Verify the safety of Rc and Weak in alloc::rc#661kasimte wants to merge 2 commits into
Conversation
feliperodri
left a comment
There was a problem hiding this comment.
Thanks @kasimte, and apologies — this was also missed in the Challenge 26 round (created after our triage snapshot). Reviewed now.
This is excellent and essentially equal on verification merit to the solution we approved: 12/12 unsafe fns with genuine safety::{requires,ensures} + matching #[kani::proof_for_contract] (21 macro sites, sized + unsized/slice; real provenance/validity preconditions and checkable postconditions, satisfied by construction not assume) and ~54/54 safe abstractions (≥75%). Clean on T1/T2/T7, symbolic inputs, kani::cover non-vacuity witnesses, should_panic refcount-overflow twins.
Requesting changes for two reasons:
- One runtime-code edit.
Weak::dropis reformatted from upstream's one-liner (let inner = if let Some(inner) = self.inner() { inner } else { return };) to a 5-line block. Semantically identical, but per CLAUDE.md this repo must not change std runtime logic / must stay in upstream sync, and it will diverge from upstream rustfmt (risk ofcheck_rustc.shformat mismatch). Please revert that line. - Prioritization: we reviewed both open Ch26 solutions and approved #582 (v3risec) — same 12/12 real proof_for_contract + ~54/54 coverage, but with no runtime-source edit and a leaner harness count (yours expands to ~1,293 harnesses = higher CI cost). We're keeping this as the strong alternative.
Minor: 5 loop-bearing safe fns use #[kani::unwind(5)] (bounded, len-3 inputs) — fine and challenge-permitted, but a weaker guarantee than an inductive proof. Superb work; revert the Weak::drop reflow and it's a clean equal to #582.
715ea53 to
27dae19
Compare
|
Thanks @feliperodri — appreciate you circling back. Addressed both points:
I've re-requested review — thanks again for the careful read. |
27dae19 to
7f134cd
Compare
Towards #382. Solves Challenge 26: Safety of
Rc.The change is purely additive — +3,459 / −0, one file, no
stdruntime-logic change — in a singlemod verify. It is complete: all 12 requiredpub unsafefunctions carrysafety::{requires, ensures}contracts verified by#[kani::proof_for_contract], and all 54 listed safe functions have Kani harnesses. Harnesses are instantiated over a representative primitive width-spread (i8/i128, u8/u128, bool, unit,[u8; 4], and[u8]/[u128]slices) to keep CI cost lean. Inputs are symbolic; every input-bearing harness carries akani::covernon-vacuity witness; there are nocfg(kani)body swaps. GenericTis limited to primitives (challenge-permitted); allocator coverage isGlobal, explicit and default (Systemlives instd, not nameable fromalloc). All harnesses pass viascripts/run-kani.sh.Coverage: required unsafe functions (12/12)
Rc<MaybeUninit<T>, A>::assume_initproof_for_contract(rc_check_assume_init, width-spread subset)Rc<[MaybeUninit<T>], A>::assume_initproof_for_contract(rc_check_assume_init_slice, width-spread subset)Rc<T>::from_rawproof_for_contract(rc_check_from_raw_{sized,unsized}, sized + unsized)Rc<T>::increment_strong_countproof_for_contract(sized + unsized)Rc<T>::decrement_strong_countproof_for_contract(sized + unsized)Rc<T, A>::from_raw_inproof_for_contract(sized + unsized)Rc<T, A>::increment_strong_count_inproof_for_contract(sized + unsized)Rc<T, A>::decrement_strong_count_inproof_for_contract(sized + unsized)Rc<T, A>::get_mut_uncheckedproof_for_contract(sized + unsized)Rc<dyn Any, A>::downcast_uncheckedproof_for_contract(width-spread subset +Vecpayloads)Weak<T>::from_rawproof_for_contract(sized + unsized)Weak<T, A>::from_raw_inproof_for_contract(sized + unsized)Coverage: safe functions (54/54)
All 54 functions from the challenge's table, grouped by API area; each entry names its harness family.
new,new_uninit,new_zeroed,try_new,try_new_uninit,try_new_zeroed,new_uninit_in,new_zeroed_in,new_cyclic_in,try_new_in,try_new_uninit_in,try_new_zeroed_in,pin,pin_in(onerc_check_*family each;pin/pin_ininclude a!Unpinsentinel type).new_uninit_slice,new_zeroed_slice,new_uninit_slice_in,new_zeroed_slice_in,into_array(one family each, symbolic lengths under layout-validity assumptions).inner,into_inner_with_allocator,into_raw_with_allocator,as_ptr,get_mut(three-state),make_mut(three-state),downcast(success + failure),from_box_in,try_unwrap(three-state).From/FromIterator/TryFrom:From<&str>,From<Vec<T>>,From<Rc<str>>,ToRcSlice::to_rc_slice(both specializations),TryFrom<Rc<[T]>> for Rc<[T; N]>(both arms, one cover per arm).RcFromSlice: theT: Copy-era specialization (today'sTrivialCloneimpl;rc_check_from_slice_copy, width-spread subset) and theT: Clonedefault (rc_check_from_slice_clone, via a manual-Clonewrapper — every primitive isTrivialClone, so no primitive can reach the default path).Drop for Rc(three-state),Clone for Rc,Default(width-spread subset +Vecpayloads),Default for Rc<str>.Weak:as_ptr(live + dangling),into_raw_with_allocator(live + dangling),upgrade(three-path),inner(both branches),Drop for Weak(three-path).RcInnerPtr:inc_strong,inc_weak(non-overflow harnesses + the twoshould_panicoverflow harnesses).UniqueRc/UniqueRcUninit:into_rc,downgrade,Deref,DerefMut,Drop for UniqueRc(both branches),UniqueRcUninit::new,data_ptr,Drop for UniqueRcUninit.Contracts
safety::{requires, ensures}. The fourmodifiesclauses usecfg_attr(kani, kani::modifies(...))— thesafetycrate exposes no modifies wrapper.ptr::addr_eq, check size and alignment viakani::mem::checked_size_of_raw/checked_align_of_raw, and requirestrong >= 1. No contract constructs or consumes anRcinternally.assume_initharness comments state exactly what is constrained (the harness-generated bytes, beforeset_len) and why the postcondition assertion is not circular.Proof soundness
kani::coverwitness per harness body (141 source lines), placed after the last assumption and before the operation under verification (inshould_panicharnesses, before the panicking operation); every harness reportsN of N cover properties satisfied.Some/None,Ok/Err, strong/weak count relations, pointer identity, post-dropupgrade()behavior,make_mut's clone-on-write and weak-disassociation effects); constructor harnesses assert value preservation.kani::assumein the suite has a reachability witness downstream of it;TryFromadditionally covers both match arms.unique/shared/weak_present,success/failure,live/strong_zero/dangling) construct each behavior-relevant state explicitly.should_panicpairing: both reachable abort guards in the file (inc_strong/inc_weakrefcount overflow) have#[kani::should_panic]harnesses. A sweep of the remainingassert!/panic!/unwrapsites found no other guard reachable under the documented preconditions: theLayout::array().unwrap()sites are unreachable under the harnesses' layout-validity assumptions, and theTrustedLencapacity overflowpanic requires an iterator longer thanusize::MAX.Loop verification
from_iter_exact's element-writing loop is verified against the unmodified upstreamfor-loop by bounded unrolling (#[kani::unwind(5)], length-3 inputs, justified in-code). Unrolling is deliberate: a loop-contract formulation cannot carry the harnesses' element-value postconditions, because under invariant havoc no invariant relates an opaqueimpl Iterator's yielded prefix to the memory the harness asserts on. The file contains nocfg(kani)code substitutions: the code Kani verifies is the code that runs.Bounds
The shared nondeterministic-vector helper bounds the symbolic length (
sz <= 100). This is a CI tractability measure, not a safety precondition; sampled harnesses re-verified locally with the bound removed:clone_rc_vec_u822.4s,from_raw_vec_u840.5s,get_mut_vec_u8_shared_none35.2s (all successful, covers satisfied). Theassume_init_sliceharnesses carry the samelen <= 100bound for CI-runner memory on wide element types: the unbounded form verifies on the Linux CI runners, but the widest variants exceed macOS runner memory. In-code comments carry the same statements. Fixed harness lengths (len 3,N = 100ininto_array) carry in-code justifications.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.