Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement validity checks #3085

Merged
merged 12 commits into from
Mar 25, 2024
Merged

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Mar 16, 2024

This is still incomplete, but hopefully it can be merged as an unstable feature. I'll publish an RFC shortly.

This instruments the function body with assertion checks to see if users are generating invalid values. This covers:

  • Union access
  • Raw pointer dereference
  • Transmute value
  • Field assignment of struct with invalid values
  • Aggregate assignment

Things not covered today should trigger ICE or a delayed verification failure due to unsupported feature.

Design

This change has two main design changes which are inside the new kani_compiler::kani_middle::transform module:
1- Instance body should now be retrieved from the BodyTransformation structure. This structure will run transformation passes on instance bodies (i.e.: monomorphic instances) and cache the result.
2- Create a new transformation pass that instruments the body of a function for every potential invalid value generation.
3- Create a body builder which contains all elements of a function body and mutable functions to modify them accordingly.

Call-outs

  1. I am planning to add expected tests in a follow up PR.
  2. This PR is currently blocked since it requires toolchain nightly-2024-03-15.
  3. One potential improvement for later is to replace most of the build check logic by an internal Kani library function that handle the comparison.

Related to #2998
Fixes #301

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

This is still incomplete, but hopefully it can be merged as an unstable
feature.

This instruments the function body with assertion checks to see if users
are generating invalid values. This covers:
  - Union access
  - Raw pointer dereference
  - Transmute value
  - Field assignment of struct with invalid values
  - Aggregate assignment

Things not covered today should trigger ICE or verification failure.
@celinval celinval requested a review from a team as a code owner March 16, 2024 06:38
@celinval celinval marked this pull request as draft March 16, 2024 06:38
@github-actions github-actions bot added the Z-BenchCI Tag a PR to run benchmark CI label Mar 16, 2024
@celinval celinval marked this pull request as ready for review March 18, 2024 22:32
@celinval celinval linked an issue Mar 19, 2024 that may be closed by this pull request
@celinval celinval self-assigned this Mar 19, 2024
Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first round of comments. I haven't finished reviewing the entire PR yet.

kani-compiler/src/args.rs Outdated Show resolved Hide resolved
kani_metadata/src/unstable.rs Outdated Show resolved Hide resolved
tests/kani/ValidValues/custom_niche.rs Show resolved Hide resolved
tests/kani/ValidValues/custom_niche.rs Outdated Show resolved Hide resolved
tests/kani/ValidValues/custom_niche.rs Outdated Show resolved Hide resolved
tests/kani/ValidValues/write_invalid.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits

kani-compiler/src/kani_middle/transform/check_values.rs Outdated Show resolved Hide resolved
kani-compiler/src/kani_middle/transform/check_values.rs Outdated Show resolved Hide resolved
kani-compiler/src/kani_middle/transform/check_values.rs Outdated Show resolved Hide resolved
celinval and others added 6 commits March 21, 2024 12:13
Co-authored-by: Zyad Hassan <88045115+zhassan-aws@users.noreply.github.com>
This check today was relying on the tuple layout, which is not
guaranteed. Instead, only check for the first operand.

This is actually simpler.
Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, @celinval!

kani-compiler/src/kani_middle/transform/mod.rs Outdated Show resolved Hide resolved
@celinval celinval enabled auto-merge (squash) March 22, 2024 23:09
@celinval celinval merged commit e4a90e9 into model-checking:main Mar 25, 2024
19 of 20 checks passed
tautschnig added a commit that referenced this pull request Apr 5, 2024
Updated version in all `Cargo.toml` files (via
`find . -name Cargo.toml -exec sed -i 's/version = "0.48.0"/version =
"0.49.0"/' {} \;`) and ran `cargo build-dev` to have `Cargo.lock` files
updated.

GitHub generated release notes:

## What's Changed
* Upgrade Rust toolchain to 2024-03-14 by @zhassan-aws in
#3081
* Disable removal of storage markers by @zhassan-aws in
#3083
* Automatic cargo update to 2024-03-18 by @github-actions in
#3086
* Bump tests/perf/s2n-quic from `1a7faa8` to `9e39ca0` by @dependabot in
#3087
* Upgrade toolchain to nightly-2024-03-15 by @celinval in
#3084
* Add optional scatterplot to benchcomp output by @tautschnig in
#3077
* Benchcomp scatterplots: quote axis labels by @tautschnig in
#3097
* Expand ${var} in benchcomp variant `env` by @karkhaz in
#3090
* Add test for #3099 by @zhassan-aws in
#3100
* Automatic cargo update to 2024-03-25 by @github-actions in
#3103
* Bump tests/perf/s2n-quic from `1a7faa8` to `0a60ec1` by @dependabot in
#3104
* Implement validity checks by @celinval in
#3085
* Add `benchcomp filter` command by @karkhaz in
#3105
* Add CI test for --use-local-toolchain by @jaisnan in
#3074
* Upgrade Rust toolchain to `nightly-2024-03-21` by @adpaco-aws in
#3102
* Use `intrinsic_name` to get the intrinsic name by @adpaco-aws in
#3114
* Bump tests/perf/s2n-quic from `0a60ec1` to `2d5e891` by @dependabot in
#3118
* Allow modifies clause for verification only by @feliperodri in
#3098
* Automatic cargo update to 2024-04-01 by @github-actions in
#3117
* Automatic cargo update to 2024-04-04 by @github-actions in
#3122
* Remove bookrunner by @tautschnig in
#3123
* Upgrade Rust toolchain to nightly-2024-03-29 by @feliperodri in
#3116
* Remove unnecessary build step for some workflows by @zhassan-aws in
#3124
* Ensure storage markers are kept in std code by @zhassan-aws in
#3080


**Full Changelog**:
kani-0.48.0...kani-0.49.0
tautschnig pushed a commit that referenced this pull request Apr 5, 2024
In the previous PR #3085, we did not support checks for `write_bytes`
which is added in this PR.

I am waiting for #3092 to add expected tests.
zpzigi754 pushed a commit to zpzigi754/kani that referenced this pull request May 8, 2024
Updated version in all `Cargo.toml` files (via
`find . -name Cargo.toml -exec sed -i 's/version = "0.48.0"/version =
"0.49.0"/' {} \;`) and ran `cargo build-dev` to have `Cargo.lock` files
updated.

GitHub generated release notes:

## What's Changed
* Upgrade Rust toolchain to 2024-03-14 by @zhassan-aws in
model-checking#3081
* Disable removal of storage markers by @zhassan-aws in
model-checking#3083
* Automatic cargo update to 2024-03-18 by @github-actions in
model-checking#3086
* Bump tests/perf/s2n-quic from `1a7faa8` to `9e39ca0` by @dependabot in
model-checking#3087
* Upgrade toolchain to nightly-2024-03-15 by @celinval in
model-checking#3084
* Add optional scatterplot to benchcomp output by @tautschnig in
model-checking#3077
* Benchcomp scatterplots: quote axis labels by @tautschnig in
model-checking#3097
* Expand ${var} in benchcomp variant `env` by @karkhaz in
model-checking#3090
* Add test for model-checking#3099 by @zhassan-aws in
model-checking#3100
* Automatic cargo update to 2024-03-25 by @github-actions in
model-checking#3103
* Bump tests/perf/s2n-quic from `1a7faa8` to `0a60ec1` by @dependabot in
model-checking#3104
* Implement validity checks by @celinval in
model-checking#3085
* Add `benchcomp filter` command by @karkhaz in
model-checking#3105
* Add CI test for --use-local-toolchain by @jaisnan in
model-checking#3074
* Upgrade Rust toolchain to `nightly-2024-03-21` by @adpaco-aws in
model-checking#3102
* Use `intrinsic_name` to get the intrinsic name by @adpaco-aws in
model-checking#3114
* Bump tests/perf/s2n-quic from `0a60ec1` to `2d5e891` by @dependabot in
model-checking#3118
* Allow modifies clause for verification only by @feliperodri in
model-checking#3098
* Automatic cargo update to 2024-04-01 by @github-actions in
model-checking#3117
* Automatic cargo update to 2024-04-04 by @github-actions in
model-checking#3122
* Remove bookrunner by @tautschnig in
model-checking#3123
* Upgrade Rust toolchain to nightly-2024-03-29 by @feliperodri in
model-checking#3116
* Remove unnecessary build step for some workflows by @zhassan-aws in
model-checking#3124
* Ensure storage markers are kept in std code by @zhassan-aws in
model-checking#3080


**Full Changelog**:
model-checking/kani@kani-0.48.0...kani-0.49.0
zpzigi754 pushed a commit to zpzigi754/kani that referenced this pull request May 8, 2024
In the previous PR model-checking#3085, we did not support checks for `write_bytes`
which is added in this PR.

I am waiting for model-checking#3092 to add expected tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-BenchCI Tag a PR to run benchmark CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing check for object correctness invariants
2 participants