Conversation
|
I would like to report on the two CI failures from this PR. For
Both failures were reported as
Of these two, For
For the other failing check,
For reference, my local verification environment is:
Based on the current evidence, my impression is that these failures are more likely related to CI environment instability or resource constraints than to a semantic issue in the harnesses introduced by this PR. Would it make sense to investigate whether the CI runners are hitting memory limits, and if so, whether the memory budget or other CI resource constraints for these Kani jobs should be adjusted? |
|
Update on the CI timeout: The previous failure in To keep the macOS CI job within its time/memory budget, the latest commit adds a
This is intended as a CI-resource guard for macOS only. The general capacity and |
feliperodri
left a comment
There was a problem hiding this comment.
Challenge 27 (Arc/Weak) — Kani verification review of PR #587
This is a substantial, genuinely sound effort — vastly stronger than the competing #575 (which was rejected for zero contracts + concrete inputs). #587 uses symbolic inputs throughout (kani::any, verifier_nondet_vec), exercises every behavior-relevant branch explicitly, and adds real pointer-provenance + refcount preconditions. However, two criterion-linked issues block approval.
What's correct (credited)
- No fatal vacuity. No
cfg(kani)body-swaps. The only source changes are additive#[cfg_attr(kani, kani::requires/ensures/modifies(...))]attributes plus the#[cfg(kani)] mod verifyblock. Nokani::assume(false), no assume-the-conclusion. - All 12 required unsafe functions have harnesses. 10 are verified via
#[kani::proof_for_contract]:from_raw,increment_strong_count,decrement_strong_count,from_raw_in,increment_strong_count_in,decrement_strong_count_in,get_mut_unchecked,downcast_unchecked,Weak::from_raw,Weak::from_raw_in. - Contracts are faithful (not decorative or over-constrained).
library/alloc/src/sync.rsdiff lines 233–245 (from_raw), 277–302 (decrement_strong_count), 310–327 (from_raw_in) encode the documented safety preconditions:ptrreconstructs to&raw const (*inner).data(ptr == rebuilt_ptr),checked_size_of_raw/checked_align_of_rawmatch, andstrong.load(Relaxed) >= 1. TheWeak::from_raw/from_raw_incontracts (diff lines 434–530) correctly branch on theis_dangling(ptr)sentinel and preserve theweakcount. Preconditions are satisfiable and non-vacuous because harnesses build valid Arcs viainto_raw/into_raw_with_allocatorround-trips (e.g. diff lines 692–710, 843–861). - Symbolic, not concrete. Values are
kani::any; thedecrement_*harnessesclone()first to keepstrong >= 1;try_unwrap(diff 1824–1850) covers unique/shared/weak-present states;drop,downcast,Weak::upgrade/inner/dropall cover dangling-vs-live and multi-owner states. - Safe-function coverage well above the 75% bar. ~56 of the ~58 listed safe functions have harnesses (missing at most
TryFrom<Arc<[T]>>::try_fromandToArcSlice::to_arc_slice). - Bounded appropriately per the challenge: primitive types +
Globalallocator only, as explicitly permitted; macOSlen <= 1024guards are for CI performance.
Blocking issues
-
Contracts bypass the mandated tool-agnostic
safetycrate. Every contract is written as#[cfg_attr(kani, kani::requires(...))](e.g.sync.rsdiff lines 191–204, 253–269, 335–363). The repo convention — used by every merged solution — isuse safety::{requires, ensures};then bare#[requires(...)]/#[ensures(...)](seelibrary/core/src/ptr/non_null.rs:1,175,244).grepfinds zero othercfg_attr(kani, kani::requires)on main. The Kani-only form defeats the repo's tool-agnostic contract design, won't be seen by the runtime backend or future tools, and is why the triage counted "0 added contracts." These should be ported to thesafetycrate attributes before merge. -
The two required
assume_initcontracts are not actually verified as contracts.Arc::<MaybeUninit<T>,A>::assume_init(diff 191–205) andArc::<[MaybeUninit<T>],A>::assume_init(diff 212–226) carryrequires/ensures, but their harnesses use#[kani::proof], not#[kani::proof_for_contract](diff 615–623, 652–672). The code comment "the requires clause is still checked at the call site" (diff 604, 643) is incorrect — a plainkani::proofdoes not enforce a callee'srequires/ensures; the attributes are inert there. The functions' absence of UB is genuinely verified (the real body runs under Kani and the harness assertsstrong_count == 1/ data equality), so this is not a soundness hole — but the success criterion "the contracts have been verified" is not literally met for these two, and the misleading comment should be corrected. If theproof_for_contractpath-resolution limitation is real for Kani 0.65, note it explicitly and, ideally, add manualassert!s mirroring the contract clauses.
Non-blocking
- Data-race / atomic obligation not addressed. Kani is single-threaded, so the challenge's data-race obligation is out of scope for these proofs. This is acknowledged in the challenge text as a shared difficulty with Challenge 7; fine to scope out, but the PR should state it explicitly rather than leave it implicit.
- Spurious edits to std source: blank lines inserted between
#[cfg(...sanitize...)]and theacquire!macro (sync.rsdiff lines 175, 183). Harmless (whitespace doesn't break attribute association) but unnecessary noise in the verification target; please drop them. - Consider adding the two missing safe-fn harnesses (
TryFrom<Arc<[T]>>::try_from,ToArcSlice::to_arc_slice) for completeness.
Direction
Convert all contracts to the safety crate attributes (issue 1); either verify the two assume_init contracts via proof_for_contract or, if blocked by tooling, add explicit assertions mirroring the clauses and fix the inaccurate comment (issue 2); revert the stray blank-line edits; and note the data-race scoping. Once the contract mechanism matches repo convention and the assume_init contracts are actually exercised, this is on track for approval.
- Migrate Arc contracts to the tool-agnostic safety attributes. - Mirror assume_init contract conditions in regular Kani proofs and clarify that proof does not activate callee contracts. - Remove stray whitespace around the acquire macro. - Revert unintended Cargo.lock changes.
|
@feliperodri Thanks for the detailed review. I’ve addressed the two blocking issues and the related cleanup:
The shared nondeterministic vector helper now bounds the symbolic length to The data-race obligation remains explicitly out of scope for these single-threaded Kani proofs. Please let me know if there are any other changes you would like me to make. |
- Bound unsized Weak::upgrade inputs for stable CI verification. Not a verification limitation. - Remove commented-out dyn Any harness invocations.
|
@feliperodri All CI checks are green now, and I’ve addressed the blocking and non-blocking issues. This should be ready for another look. Thanks! |
|
Update Challenge 27 Arc/Weak verification coverage in several areas. It adds harnesses for Reachability witnesses were also added after all This change also addresses verify-rust-std#673. The repository is still pinned to a Kani version affected by kani#4537, which is fixed upstream by kani#4542, but Kani cannot be updated as part of this change. Therefore, the expected assertions for the affected ownership-sensitive branches are kept explicitly in the harness comments and temporarily disabled:
These branches are not currently reliable under the pinned Kani model. The assertions should be re-enabled once the repository adopts a Kani version containing the upstream fix. |
feliperodri
left a comment
There was a problem hiding this comment.
Approving — leading solution for Challenge 27
Thanks @v3risec. After reviewing both open Challenge 27 (Arc/Weak) solutions with our vacuity tooling and local Kani (pinned 0.67.0 / CBMC 6.8.0), this is complete and sound. Prioritizing it.
Coverage against Ch27 criteria:
- A: 12/12 unsafe pub fns. Each has real
#[requires]/#[ensures](not trivially true) with a matching#[kani::proof_for_contract]at the exact fn path — assume_init (both sized+slice), from_raw / from_raw_in, increment_strong_count(_in), decrement_strong_count(_in), get_mut_unchecked, Arc<dyn Any+Send+Sync,A>::downcast_unchecked, Weak::from_raw(_in). Contracts includechecked_align_of_raw/checked_size_of_raw,can_dereference, byte-sub round-trip,strong.load(Relaxed) >= 1,is_dangling-sentinel disjuncts, andmodifiesframes. - B: 57/58 safe abstractions ≥ 75% (98.3%). Primitive-mono over the full integer/bool/unit/[u8;4] matrix;
Globalallocator only (challenge-allowed). Only missing:Default<str>::default.
Soundness (all clean):
- T1: no cfg body swaps — bodies of the 12 unsafe fns are unmodified; only outer attributes added.
- T2: no
invariant(true), noloop_invariant(true). - T7: every contracted fn has its matching
proof_for_contractat the correct instantiation;.github/workflows/kani.ymlunchanged (no autoharness dependency). - Not assume-the-conclusion —
kani::assumeuses are limited to layout well-formedness, slice-length CI budget, and caller-sidecan_dereference(initialized)obligations. None assume the fn's own postcondition. - Inputs symbolic (
kani::any::<T>()); slice length capped at 100 (documented CI budget, "can be removed for local verification"). kani::unwind(6)only on twoCloned<slice::Iter>harnesses over a fixed[T;4]source — appropriate.- No runtime std logic changes.
Local Kani sample (CBMC 6.8.0): 2/2 so far VERIFICATION SUCCESSFUL — harness_arc_assume_init_i8 (proof_for_contract), harness_arc_downcast_unchecked_i8 (proof_for_contract). Two more (harness_arc_increment_strong_count_i8, harness_arc_clone_i8) still running; will update if any fail. All static evidence supports approval.
Minor notes for the record (non-blocking):
Weak::from_raw/from_raw_insentinel-is_dangling(ptr)==truebranch coverage: harnesses always downgrade a live Arc beforeinto_raw, so the dangling-path leg of the disjunctive precondition isn't exercised even though the contract admits it.downcast_uncheckedcontract is precondition-only (noensures) — weak but faithful to the fn's docs.- Data-race obligation (challenge line 64) explicitly out of scope: "single-threaded, no concurrent thread interleavings." Challenge 27 sidesteps this by pointing to Challenge 7; noted for the record.
Solid, complete, sound. Bounded + primitive-mono is challenge-allowed.
|
@HuStmpHrrr @rajath-mk @rafaelsamenezes could you review this proposed solution? |
| @@ -1607,6 +1639,23 @@ impl<T: ?Sized> Arc<T> { | |||
| /// ``` | |||
| #[inline] | |||
| #[stable(feature = "arc_mutate_strong_count", since = "1.51.0")] | |||
| #[requires(!ptr.is_null())] | |||
| #[requires(kani::mem::can_dereference(ptr))] | |||
| #[requires({ | |||
There was a problem hiding this comment.
Fixed. I refactored the overlapping requirements into the shared arc_raw_valid(ptr) predicate, which captures the raw layout validity, dereferenceable strong-count field, and strong >= 1. The separate redundant pointer requirements are no longer needed.
| @@ -1647,6 +1696,29 @@ impl<T: ?Sized> Arc<T> { | |||
| /// ``` | |||
| #[inline] | |||
| #[stable(feature = "arc_mutate_strong_count", since = "1.51.0")] | |||
| #[requires({ | |||
There was a problem hiding this comment.
Fixed. I updated this to follow the same refactoring pattern as Challenge 26, adapted to Arc's atomic reference counts. The repeated checks are now shared through the arc_raw_* and weak_raw_* helpers.
|
@feliperodri @HuStmpHrrr Thanks for the detailed review. I went through the comments and updated the contracts and harnesses accordingly.
I reran the updated harnesses locally, and the verification passes. |




Summary
This PR adds Kani-based verification artifacts for
Arc/Weaksafety inlibrary/alloc/src/sync.rsfor Challenge 27.The change introduces:
Arc/Weakfunctions listed in the challenge, and reachability witnesses after allproof_for_contractcalls, so contract preconditions are exercised by explicitkani::coverchecks#[requires]/#[ensures]contracts provided by thesafetycrate, while retaining Kani-specificmodifiesclauses as frame conditions#[cfg(kani)]for those unsafe functions and a broad safe-function subsetArc<[T]>/Weak<[T]>paths can be exercised in a reusable wayNo non-verification runtime behavior is changed in normal builds.
Verification Coverage Report
Unsafe functions (required by Challenge 27)
Coverage: 12 / 12 (100%)
Verified set includes:
Arc<mem::MaybeUninit<T>,A>::assume_initArc<[mem::MaybeUninit<T>],A>::assume_initArc<T:?Sized>::from_rawArc<T:?Sized>::increment_strong_countArc<T:?Sized>::decrement_strong_countArc<T:?Sized,A:Allocator>::from_raw_inArc<T:?Sized,A:Allocator>::increment_strong_count_inArc<T:?Sized,A:Allocator>::decrement_strong_count_inArc<T:?Sized,A:Allocator>::get_mut_uncheckedArc<dyn Any+Send+Sync,A:Allocator>::downcast_uncheckedWeak<T:?Sized>::from_rawWeak<T:?Sized,A:Allocator>::from_raw_inSafe functions (Challenge 27 list)
Stable passing coverage: 57 / 58 (98.3%)
This exceeds the challenge threshold (>= 75%).
Covered safe functions (57/58), grouped by API category:
Allocation
Arc<T>::newArc<T>::new_uninitArc<T>::new_zeroedArc<T>::pinArc<T>::try_pinArc<T>::try_newArc<T>::try_new_uninitArc<T>::try_new_zeroedArc<T,A:Allocator>::new_inArc<T,A:Allocator>::new_uninit_inArc<T,A:Allocator>::new_zeroed_inArc<T,A:Allocator>::new_cyclic_inArc<T,A:Allocator>::pin_inArc<T,A:Allocator>::try_pin_inArc<T,A:Allocator>::try_new_inArc<T,A:Allocator>::try_new_uninit_inArc<T,A:Allocator>::try_new_zeroed_inArc<T,A:Allocator>::try_unwrapArc<T,A:Allocator>::into_innerArc<T:?Sized,A:Allocator>::into_inner_with_allocatorSlice and conversion
Arc<[T]>::new_uninit_sliceArc<[T]>::new_zeroed_sliceArc<[T]>::into_arrayArc<[T],A:Allocator>::new_uninit_slice_inArc<[T],A:Allocator>::new_zeroed_slice_inArcFromSlice<T: Copy>::from_sliceArcFromSlice<T: Clone>::from_sliceTryFrom<Arc<[T],A:Allocator>>::try_fromToArcSlice<T, I>::to_arc_sliceConversion and pointer
Arc<T:?Sized,A:Allocator>::into_raw_with_allocatorArc<T:?Sized,A:Allocator>::as_ptrArc<T:?Sized,A:Allocator>::innerArc<T:?Sized,A:Allocator>::from_box_inClone<T:?Sized, A:Allocator>::clone for ArcArc<T:?Sized+CloneToUninit, A:Allocator+Clone>::make_mutArc<T:?Sized, A:Allocator>::get_mutDrop<T:?Sized, A:Allocator>::drop for ArcArc<dyn Any+Send+Sync,A:Allocator>::downcastWeak and trait-related operations
Weak<T:?Sized,A:Allocator>::as_ptrWeak<T:?Sized,A:Allocator>::into_raw_with_allocatorWeak<T:?Sized,A:Allocator>::upgradeWeak<T:?Sized,A:Allocator>::innerDrop<T:?Sized, A:Allocator>::drop for WeakDefault and conversions
Default<T:Default>::defaultDefault<core::ffi::CStr>::defaultDefault<[T]>::defaultFrom<&str>::fromFrom<Vec<T,A:Allocator+Clone>>::fromFrom<Arc<str>>::fromUniqueArc / UniqueArcUninit and traits
UniqueArcUninit<T:?Sized, A:Allocator>::newUniqueArcUninit<T:?Sized, A:Allocator>::data_ptrDrop<T:?Sized, A:Allocator>::drop for UniqueArcUninitUniqueArc<T:?Sized,A:Allocator>::into_arcUniqueArc<T:?Sized,A:Allocator+Clone>::downgradeDeref<T:?Sized,A:Allocator>::derefDerefMut<T:?Sized,A:Allocator>::deref_mutDrop<T:?Sized, A:Allocator>::drop for UniqueArcNot yet listed as standalone harness targets (1/58)
Default<str>::defaultCurrent Criteria Met
#[kani::proof_for_contract].Tis instantiated with representative concrete types allowed by the challenge, and allocator-focused proofs are limited to standard-library allocator scope (Global).Approach
The verification strategy combines contracts for unsafe entry points with executable proof harnesses:
safety::{requires, ensures}attributes for preconditions and postconditions on all 12 required unsafe functions.modifiesclauses as frame conditions, since thesafetycrate currently providesrequiresandensuresbut not a correspondingmodifiesattribute.#[kani::proof]/#[kani::proof_for_contract]modules for required unsafe functions and safe-function coverage targets.Arc<[T]>/Weak<[T]>harnesses.?Sizedslice cases without duplicating setup logic across harnesses.cfg(kani)so normal std behavior is unchanged.Scope assumptions (per challenge allowance)
i8..i128,u8..u128),bool,(), arrays, vectors, slices,str,CStr, and trait objects (dyn Any,dyn Any + Send + Syncwhere required by the API).Global.Data-race scope
The current Kani harnesses are single-threaded and do not model concurrent thread interleavings. Consequently, this PR does not claim to discharge the Challenge 27 data-race obligation or prove the absence of races under concurrent
Arc/Weakoperations.Within this scope, the proofs check memory safety for the explored executions, including pointer validity and provenance-related conditions, layout and allocation consistency, and strong/weak reference-count invariants. Atomic loads and updates are exercised as part of those sequential executions, but their behavior under concurrent interleavings, synchronization, and memory-ordering interactions remains outside the scope of these proofs.
Verification
All passing harnesses listed in this report pass locally with the current Kani setup used for this repository.
Resolves #383
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.