Challenge 4: bounded Kani PROBEs on btree::node's copy, relink and rebalancing helpers - #666
Open
ivmat wants to merge 4 commits into
Open
Challenge 4: bounded Kani PROBEs on btree::node's copy, relink and rebalancing helpers#666ivmat wants to merge 4 commits into
ivmat wants to merge 4 commits into
Conversation
…link and slot-copy internal helpers (correct_childrens_parent_links, insert_fit, remove, move_suffix)
The repo's `upstream_test` CI job runs `./x fmt --check` inside a rust-lang/rust checkout, which uses that repo's rustfmt.toml (style_edition 2024, use_small_heuristics = "Max"). This crate has no rustfmt.toml of its own, so a plain `cargo fmt` does not reproduce it. Formatting only: no harness, assertion, cover string, or bound changed.
…e PROBE set Extends the existing nine-harness entry with nineteen more, all in the same `#[cfg(kani)] mod verify` block and purely additive to the module. Functional content for `Handle::move_suffix` (6): the post-state content of both nodes after the type-erasure + two-node copy sequence, read back through the proven-safe `Handle::new_kv(..).into_kv()` path, plus a raw stored-length diagnostic. These were previously disclosed as an open residual; they discharge on the toolchain this entry pins. The second success-criteria list (12): `NodeRef::new_internal`; `BalancingContext`'s `do_merge` (leaf arm over the complete occupancy domain, internal arm at a stated bound, and a full-occupancy internal-arm no-UB harness), `merge_tracking_child_edge`, `steal_left`, `steal_right`, `bulk_steal_left` and `bulk_steal_right` (leaf and internal arms). Also `Handle::split` and the relink loop over the complete occupancy domain. The two bulk-steal harnesses assert their destination/source stored length and their key-and-value shift outright rather than recording them as coverage witnesses. Formatting verified against rust-lang/rust's own rustfmt config at the pinned toolchain's commit, which is what upstream CI applies; the diff is additive with no reformatting of existing code, and every `kani::cover` message is byte-identical to its source.
…le gaps
Two independent review seats and the verification run itself converged on the same fatal
defect, which is why the run is the gate and not the review:
F1 (fatal): check_bulk_steal_left_leaf_scoped_no_ub read right_v_before, which that harness
never declared -- E0425, so the whole #[cfg(kani)] mod verify failed to build and no
harness in the file could have produced a green. Ordinary cargo builds do not see the
module, which is why it went unnoticed. right_v_before is now declared and populated
alongside right_k_before.
F2: the full-occupancy internal-arm harness carried two covers -- 'a maximal right/left
child for this bound' -- that do_merge's own precondition (old_left_len + 1 + right_len <=
CAPACITY) makes UNSATISFIABLE at IB == CAPACITY. Shipping a cover that can never witness is
the exact fault this packet cited when deleting the LOST.* covers. Replaced with the
reachable extremes the precondition admits, and the reason recorded in a comment.
Oracle gaps closed rather than narrowed, where cheap:
NI5 -- new_internal now checks the FORWARD link (edge 0 IS the child); the backlink alone
would accept a node that does not hold the child.
CA3 -- the relink harness now checks the parent POINTER, not just parent_idx; index-only
would be satisfied by a child still carrying the perturbed dangling pointer.
SP6v/SP7v -- split now checks values as well as keys for the new node's last element and
the source head.
Labelling and domain honesty: the split harness is relabelled PROBE (it was headed CONTRACT
CANDIDATE while being monomorphic and sampling two occupancies), its duplicated sentence is
removed, and its sampled {1, CAPACITY} domain is stated. steal_right's comment no longer
claims its assumed range is what the doc documents -- the doc's original-edge range is
..=old_left_len and the extra point is disclosed as a deliberate superset.
Formatting re-verified with the CI-faithful gate: fmt-ok, additive, no production code
touched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This extends the previous nine-harness entry to twenty-eight, adding the rebalancing family named in
the challenge's second success-criteria list, the functional-content companions for
Handle::move_suffix, andHandle::split.It remains a bounded PROBE set, not a discharge of the challenge: every harness is monomorphic at
K = V = i32, and tree height is bounded. What has changed is coverage — seven of the eightfunctions in the second list are now driven directly — and strength: several claims that the previous
entry recorded as open residuals are now proven.
The challenge's criterion is memory safety, and Kani emits its UB checks on every path whether or
not an assertion is written. The functional content assertions here are therefore additional to the
criterion, not a substitute for it.
What is proved
Handle::move_suffixcontentHandle::new_kv(..).into_kv(), plus a raw stored-length diagnosticNodeRef::new_internalcorrect_all_childrens_parent_linksHandle::split(leaf arm)splitis not drivendo_mergedo_merge's own precondition, memory safetybulk_steal_left/bulk_steal_rightsteal_left/steal_right/merge_tracking_child_edgeinsert_fit,remove,move_suffixsafety, the two relink harnessesSecond success-criteria list: 7 of 8 functions harnessed —
new_internal,do_merge,merge_tracking_child_edge,steal_left,steal_right,bulk_steal_left,bulk_steal_right.Six are called directly by a harness;
do_mergeis driven through its own public wrappermerge_tracking_parent, which is the real call path rather than a re-implementation.insert_recursingis not included; see residuals.The two
bulk_steal_*leaf-arm harnesses assert their destination/source stored length andtheir key-and-value shift, rather than recording them as coverage witnesses.
Scope and method
Fixtures use
K = V = i32and the module's ownCAPACITY(11, forB = 6). Content isposition-derived (
(i, 1000 + i)) where a misplaced write must be observable. Node lengths, splitindices, edge ranges and steal counts are symbolic — except in the
move_suffixfamily, whosesource length is sampled at
{0, 1, CAPACITY}, andHandle::split, sampled at{1, CAPACITY}.On "symbolic
lenover0..=CAPACITY".CAPACITYis a compile-time constant and aLeafNode<K, V>storeskeys: [MaybeUninit<K>; CAPACITY], so a node's occupancy cannot exceed it inany reachable state. A harness that leaves
lensymbolic across0..=CAPACITYis thereforeverifying the node type's complete occupancy domain, not a chosen slice of it. The same holds for
the
countargument ofbulk_steal_*and for every loop whose trip count derives from a nodelength.
Bounds that are harness-chosen are called out individually in the source and in the residuals
below: the internal-arm occupancy bound
IB(the fixture must materialiseIB + 1grandchildren byhand), and tree height.
Non-vacuity is carried by
kani::coverwitnesses on every harness — interior versus boundary splitpoints, empty versus maximal children,
count == 1versus a genuine bulk steal, and the shapes thatmake each relink loop iterate more than once. Replay-greens are defeated per harness: parent links
are perturbed to a sentinel before the relink harnesses run, and content harnesses use
position-derived values so a misplaced write is observable.
What is NOT proved
Each residual below was measured at Kani 0.67.0 / CBMC 6.8.0, the toolchain this entry pins. Most
of them — monomorphisation, bounded height, the harness-chosen
IB, the fixed parent shape, andper-index readback — are properties of the harness design and would hold at any pin. The previous
entry's two residuals, by contrast, WERE toolchain properties, and are retired below. The pin is
stated throughout so a reader can tell the two kinds apart.
That requirement heads the whole second list, not just one entry. These are bounded probes. The
occupancy axis happens to be bounded by the node type itself, which makes the child-length bounds
non-artificial, but height, parent shape and monomorphisation remain harness bounds throughout.
insert_recursingis not harnessed in this entry at all. Its loop ascends from the insertionpoint to the root, so its trip count is the tree's height — the one axis the node type does not
bound, unlike occupancy. Harnessing it at a fixed height is possible, but a bounded-height result
would not discharge the requirement for it, so it is left out rather than shipped with a bound that
reads stronger than it is.
KandVare not covered. Every harness is monomorphic ati32/i32; nothing hereis proven for arbitrary
K: OrdorV.height-2 tree. Nothing here speaks to deeper trees.
IBis harness-chosen on the content-carrying internalharnesses (
IB = 2), because the fixture materialisesIB + 1grandchildren explicitly. Incheck_do_merge_internal_full_occupancy_no_ubthe fixture cap is raised toCAPACITYand isdeliberately non-binding:
do_merge's own precondition (old_left_len + 1 + right_len <= CAPACITY) is what constrains the children there, so no single child reachesCAPACITYpre-merge —the reachable maximum is
CAPACITY - 1with the other child empty.operates at parent index 0;
do_merge's parent-side gap-close and relink therefore run at tripcounts 0–1 only.
child index per run and the content harnesses check named boundary positions; neither asserts the
property for all positions at once. Kani's memory-safety checks are all-paths regardless, which is
what the challenge's criterion asks for.
On the previous entry's two disclosed limitations — a suspected CBMC pointer-model residual on the
move_suffixcontent readback, and a deferreddo_merge/bulk_stealunwind hang — both weremeasured on Kani 0.65.0 / CBMC 6.7.1. Re-measured on this entry's pin they do not reproduce: the
move_suffixcontent family and the whole rebalancing family discharge, and those harnesses areincluded above. The earlier limitations were specific to the older toolchain and are not carried
forward.
Toolchain and reproduction
d4df833c8f8f18e632e7b0a7945bb2161f708990kani-dependencies)One run, one sha: every row below comes from a single whole-set run at the submitted commit, on a
16-vCPU / 128 GB VM. 28 of 28 SUCCESSFUL, 83/83 cover properties satisfied.
check_correct_all_childrens_parent_links_no_ubcheck_correct_childrens_parent_links_subrange_no_ubcheck_leaf_insert_fit_no_ubcheck_leaf_insert_fit_contentcheck_internal_insert_fit_no_ubcheck_internal_insert_fit_contentcheck_leaf_remove_no_ubcheck_leaf_remove_contentcheck_move_suffix_leaf_no_ubcheck_move_suffix_leaf_raw_lencheck_move_suffix_leaf_content_allcheck_move_suffix_leaf_content_check_acheck_move_suffix_leaf_content_check_bcheck_move_suffix_leaf_content_check_ccheck_move_suffix_leaf_content_check_dcheck_leaf_split_no_ubcheck_new_internal_no_ubcheck_correct_all_childrens_parent_links_full_domaincheck_steal_left_leaf_no_ubcheck_steal_right_leaf_no_ubcheck_merge_tracking_child_edge_leaf_no_ubcheck_do_merge_leaf_no_ubcheck_bulk_steal_left_leaf_scoped_no_ubcheck_bulk_steal_right_leaf_scoped_no_ubcheck_bulk_steal_left_internal_no_ubcheck_bulk_steal_right_internal_no_ubcheck_do_merge_internal_no_ubcheck_do_merge_internal_full_occupancy_no_ubControls
Four claims are backed by an observed-red mutation control: a single-token length mutation is applied
to the implementation, and the harness that owns the claim is required to go RED. All four did.
move_suffix's destination stored lengthcheck_move_suffix_leaf_raw_len,check_move_suffix_leaf_content_alldo_merge's merged-child stored lengthcheck_do_merge_leaf_no_ubbulk_steal_left's destination stored lengthcheck_bulk_steal_left_leaf_scoped_no_ubbulk_steal_right's source stored lengthcheck_bulk_steal_right_leaf_scoped_no_ubThe mutations are applied on the verification machine only and are not part of this diff: an in-tree
#[kani::proof]designed to fail would turn any whole-set run of the submission red.Acceptance manifest
A machine-readable statement of exactly what these proofs establish and do not establish — one entry
per claim, with declared bounds, evidence, and assurance band, checked by a standalone validator — is
shipped as a real-world example in the format's own repo:
examples/verify-rust-std-ch4-btree-v2/acceptance.toml(format). It records 13 claims: 12 evidence-bearing
(four graded A3 — each with a red mutation control naming the claim — the rest A0) and one
explicit gap entry for what this PR does not prove (
insert_recursingis not harnessed; and noclaim asserts any unboundedness requirement). Stamped and verified at this branch's head (
9a083f4bc12).