Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical validity-checking issues reject valid address-only raw borrows, partial reads, and nested assignments by validating values that are not actually read.
2 open issues
What changed in this PR
This PR addresses #5022 by extending Kani’s -Z valid-value-checks instrumentation to check indexed slice elements, whose validity is not checked by a whole-slice dereference.
Changes:
- Add element validity checks for slice
IndexandConstantIndexprojections. - Add regression harnesses for invalid
boolreads at concrete and symbolic indices, plus a valid-element control. - Record expected verification results for all three harnesses.
| File | Description |
|---|---|
tests/expected/valid-value-checks/slice_element.rs |
Adds slice-element validity regression harnesses. |
tests/expected/valid-value-checks/slice_element.expected |
Specifies two invalid-value failures and one successful verification. |
kani-compiler/src/kani_middle/transform/check_values.rs |
Instruments indexed slice elements with validity checks. |
Review effort: Balanced
The Index/ConstantIndex arm fired for any non-mutating use and for nested writes, reporting false positives: an address-of (`&raw const (*p)[i]`) loads no value, a partial read (`(*p)[i][j]`) reads only a subplace, and a nested write (`(*p)[i][j] = x`) validated old contents. Track whether the operand reads the value (Copy/Move) and fire only on a direct read of the whole element. Add positive regressions for each case.
This branch has not been deployed
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.

Resolves #5022.
-Z valid-value-checksdid not report an invalid value read as one element through a slice pointer. A slice[T]carries no static length, so the whole-slice dereference checks no elements, and the indexed read was checked nowhere.The fix checks the element in the
Index/ConstantIndexarm ofvisit_placewhen the base is a slice. It fires only on a direct load of the whole element: areading_valueflag (set invisit_operand) restricts the check toCopy/Moveoperands, and the index must be the last projection. So an address-of, a partial sub-element read, and a nested write are not falsely rejected. A symbolic index makes CBMC cover every element, so no length loop is needed.A nested invalid sub-element read (e.g.
(*p)[i][j]) is still not reported; this matchesmain, where theIndexarm checked nothing.Testing
tests/expected/valid-value-checks/slice_element.rs: an invalidboolread through*const [bool]failsInvalid value of type `bool`at a concrete and a symbolic index; valid elements, an address-only&raw constborrow, a partial read, and a nested write all verify.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.