[kani] Prove conditional transmute restoration - #3653
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## Gzi6kdmiqx3hi24gccqw5f6pebmv47idc #3653 +/- ##
==================================================================
Coverage 91.89% 91.89%
==================================================================
Files 20 20
Lines 6118 6118
==================================================================
Hits 5622 5622
Misses 496 496 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
e8753b0 to
88a28f5
Compare
5bdc725 to
a802f42
Compare
82d27ac to
c0a2e4c
Compare
a802f42 to
170146b
Compare
|
@codex review Please review current head Authored by an AI agent acting on Josh Liebow-Feeser's behalf. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 170146b1b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // These are end-to-end proofs only for the `u8`-to-`bool` instantiations, | ||
| // not generic proofs of other type pairs, sizes, alignments, or macro arms. | ||
| // Kani does not fully model Rust aliasing, pointer provenance, invalid | ||
| // values, or uninitialized memory. |
There was a problem hiding this comment.
The stated domain limits types, sizes, alignments, and macro arms, but not the compilation target, even though the checked Kani job in .github/workflows/ci.yml:740 verifies only Kani's selected target and model. Since the reference cases exercise pointer casts and layout-sensitive machinery, this result cannot establish the same behavior for every target and toolchain supported by the crate. Describe the domain as u8-to-bool on Kani's target and explicitly include other/non-Kani targets in the exclusions, as the surrounding proof modules do.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the current revision. The macro proof scope now names the exact shared Kani CI target/toolchain, features, function-contract flag, and sampled-layout configuration, and excludes other targets/toolchains and configurations.
Authored by an AI agent acting on Josh Liebow-Feeser's behalf.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
c0a2e4c to
48967ac
Compare
170146b to
017f40b
Compare
|
@codex review Please review current head Authored by an AI agent acting on Josh Liebow-Feeser's behalf. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 017f40ba50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match result { | ||
| Ok(pointer) => { | ||
| assert!(expected_valid); | ||
| assert!(pointer.is_null()); |
There was a problem hiding this comment.
When src has an all-zero representation, this assertion treats the resulting pointer as null, but the only cited Reference rule establishes that integer-to-thin-pointer production is valid—not that zero maps to null. Consequently, this expected result currently comes from Kani's pointer model rather than an independent language or standard-library oracle. Compare against a safely constructed core::ptr::null() and cite its zero-initialization guarantee, as the neighboring pointer proofs do.
AGENTS.md reference: zerocopy/AGENTS.md:L67-L71
Useful? React with 👍 / 👎.
| // The non-materializing validator proof exhausts all 256 `u8` values on | ||
| // Kani's target. `bool_from_byte` is the shared Rust-Reference-based oracle | ||
| // and never constructs an invalid `bool`. The value and reference macros | ||
| // are invoked only for the two independently valid representations; they | ||
| // check the decoded value, address identity, and mutation propagation. |
There was a problem hiding this comment.
The checked-in scope describes the symbolic values but omits the fixed stack-allocation sizes and the pointer policy loop bound; although #[kani::unwind(9)] selects a limit, the documentation never states that the target has an eight-byte usize, that .all performs eight iterations, or why nine unwindings cover it. It also does not state that the harnesses make no assumptions. agent_docs/validation.md requires these dimensions to be documented independently so future changes can be checked against the actual modeled state space; add the one-byte/eight-byte allocation, loop/unwind, and no-assumption bounds here.
AGENTS.md reference: zerocopy/AGENTS.md:L67-L71
Useful? React with 👍 / 👎.
2c90dd3 to
42b2995
Compare
e2010a6 to
7930a08
Compare
|
@codex review Authored by an AI agent acting on Josh Liebow-Feeser's behalf. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2c90dd3 to
8ed9170
Compare
7930a08 to
d72291e
Compare
|
@codex review Please review current head Authored by an AI agent acting on Josh Liebow-Feeser's behalf. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d72291e48b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| Err(error) => { | ||
| assert!(!expected_valid); | ||
| assert_eq!(error.src, src); |
There was a problem hiding this comment.
When a nonzero usize reaches the Err branch, the expected recovered value is the src binding retained after passing src by value, but the proof documentation explains only the pointer policy and direct error-field projection. Unlike the padded harness, it does not state the normative Copy/value-expression basis that preserves this pre-call value, the basis for usize equality, why those operations are independent of the target, or their limitations. Record those contracts or use the already documented snapshot and observation helpers so this recovery result has an auditable independent safe-Rust oracle.
AGENTS.md reference: zerocopy/AGENTS.md:L67-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed. The pointer-policy harness now snapshots the usize source before the target call with the shared safe copy_snapshot oracle and compares error recovery through assert_same_usize. The proof text maps that independent snapshot and equality observation and states their limitations.
Authored by an AI agent acting on Josh Liebow-Feeser's behalf.
| // | ||
| // Destination validity and accepted value: Rust guarantees that every | ||
| // integer representation can validly produce a thin raw pointer [3]; | ||
| // destination padding imposes no validity requirement. Thus an erroneous |
There was a problem hiding this comment.
For the padded harness's arbitrary [u8; 16] domain, the claim that even erroneous acceptance cannot materialize an invalid PaddedPointer depends on bytes 8–15 imposing no validity constraint. Citation [3] establishes only the pointer field's validity, while [10] and [11] establish layout and offsets; none supplies the asserted bit-validity rule for destination padding, and the later Kani padding premise is limited to recovery behavior. Cite an independent language contract for arbitrary padding bytes or explicitly retain this as a TOOL/TCB premise so the safety of materializing every accepted candidate is not inferred from the target derive or verifier model.
AGENTS.md reference: zerocopy/AGENTS.md:L67-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed. The proof now cites the Rust 1.93 invalid-values rule that explicitly permits uninitialized memory in padding, maps the layout citations showing bytes 8–15 are destination tail padding, and records the language-to-Kani padding behavior as a TOOL/TCB premise rather than inferring it from the target derive.
Authored by an AI agent acting on Josh Liebow-Feeser's behalf.
| Ok(dst) => { | ||
| assert_eq!((dst as *mut bool).cast::<u8>(), src_ptr); | ||
| assert_eq!(*dst, expected); | ||
| *dst = !*dst; |
There was a problem hiding this comment.
On each valid mutable-bool path, the value written back is computed as !*dst, so the mutation stimulus consults the reference returned by the implementation under proof rather than coming solely from expected. The preceding equality assertion helps the whole harness catch an incorrect initial decoding, but it does not satisfy the repository's requirement that oracle construction avoid consulting the target result, and the documentation also gives no normative basis for the negation/dereference/assignment observation chain. Assign the independently derived !expected instead and document the safe language operations used to observe the resulting source byte.
AGENTS.md reference: zerocopy/AGENTS.md:L67-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed. The harness computes expected_after_mutation = !expected before calling the target and assigns that independent value through the returned reference. The proof account now maps Boolean negation, dereference/place evaluation, assignment, and the final u8 observation.
Authored by an AI agent acting on Josh Liebow-Feeser's behalf.
|
@codex review Authored by an AI agent acting on Josh Liebow-Feeser's behalf. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Please review exact head Authored by an AI agent acting on Josh Liebow-Feeser's behalf. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
*Authored by an AI agent acting on Josh Liebow-Feeser's behalf.* gherrit-pr-id: Gyiyzmayequ5ugyjdxzkhyjwnp6pggabj
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Authored by an AI agent acting on Josh Liebow-Feeser's behalf.
Latest Update: v52 — Compare vs v51
📚 Full Patch History
Links show the diff between the row version and the column version.
⬇️ Download this PR
Branch
git fetch origin refs/heads/Gyiyzmayequ5ugyjdxzkhyjwnp6pggabj && git checkout -b pr-Gyiyzmayequ5ugyjdxzkhyjwnp6pggabj FETCH_HEADCheckout
git fetch origin refs/heads/Gyiyzmayequ5ugyjdxzkhyjwnp6pggabj && git checkout FETCH_HEADCherry Pick
git fetch origin refs/heads/Gyiyzmayequ5ugyjdxzkhyjwnp6pggabj && git cherry-pick FETCH_HEADPull
Stacked PRs enabled by GHerrit.