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

Stabilize pointer-to-reference cast validity checks #3426

Conversation

artemagvanian
Copy link
Contributor

This PR stabilizes pointer-to-reference cast validity checks, so that they are run by default.

Resolves #3425

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

@github-actions github-actions bot added the Z-BenchCI Tag a PR to run benchmark CI label Aug 7, 2024
@artemagvanian artemagvanian self-assigned this Aug 7, 2024
@artemagvanian artemagvanian marked this pull request as ready for review August 7, 2024 19:13
@artemagvanian artemagvanian requested a review from a team as a code owner August 7, 2024 19:13
Copy link
Contributor

@feliperodri feliperodri left a comment

Choose a reason for hiding this comment

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

How can we know for sure that any performance issues related to --ptr-to-ref-cast-checks have been resolved?

@artemagvanian
Copy link
Contributor Author

The only performance issue that we had was that the perf check failed because one harness consistently ran out of memory on the CI. In the current run all required checks passed.

@artemagvanian artemagvanian added this pull request to the merge queue Aug 7, 2024
Merged via the queue into model-checking:main with commit 3fb3a73 Aug 7, 2024
26 of 27 checks passed
@artemagvanian artemagvanian deleted the stabilize-ptr-to-ref-cast-checks branch August 7, 2024 21:16
@zhassan-aws
Copy link
Contributor

I think it would be useful to have an option to turn those checks off. Is there such an option?

@artemagvanian
Copy link
Contributor Author

Not as of now, but it won't be too hard to implement. I wonder if it is worth it, though.

Just to give some context: the performance impact of the check itself is negligible. We encountered the issue above because the check was inserted into a place nested inside two loops, which magnified its impact.

On top of that, as far as I remember, the problematic harness ran OOM on the CI sometimes already, so the checks seemed to just be the last drop. It all makes me believe this is an isolated instance rather than a systemic problem.

github-merge-queue bot pushed a commit that referenced this pull request Aug 9, 2024
## 0.54.0

### Major Changes
* We added support for slices in the `#[kani::modifies(...)]` clauses
when using function contracts.
* We introduce an `#[safety_constraint(...)]` attribute helper for the
`Arbitrary` and `Invariant` macros.
* We enabled support for concrete playback for harness that contains
stubs or function contracts.
* We added support for log2*, log10*, powif*, fma*, and sqrt*
intrisincs.

### Breaking Changes
* The `-Z ptr-to-ref-cast-checks` option has been removed, and pointer
validity checks when casting raw pointers to references are now run by
default.

## What's Changed
* Make Kani reject mutable pointer casts if padding is incompatible and
memory initialization is checked by @artemagvanian in
#3332
* Fix visibility of some Kani intrinsics by @artemagvanian in
#3323
* Function Contracts: Modify Slices by @pi314mm in
#3295
* Support for disabling automatically generated pointer checks to avoid
reinstrumentation by @artemagvanian in
#3344
* Add support for global transformations by @artemagvanian in
#3348
* Enable an `#[safety_constraint(...)]` attribute helper for the
`Arbitrary` and `Invariant` macros by @adpaco-aws in
#3283
* Fix contract handling of promoted constants and constant static by
@celinval in #3305
* Bump CBMC Viewer to 3.9 by @tautschnig in
#3373
* Update to CBMC version 6.1.1 by @tautschnig in
#2995
* Define a struct-level `#[safety_constraint(...)]` attribute by
@adpaco-aws in #3270
* Enable concrete playback for contract and stubs by @celinval in
#3389
* Add code scanner tool by @celinval in
#3120
* Enable contracts in associated functions by @celinval in
#3363
* Enable log2*, log10* intrinsics by @tautschnig in
#3001
* Enable powif* intrinsics by @tautschnig in
#2999
* Enable fma* intrinsics by @tautschnig in
#3002
* Enable sqrt* intrinsics by @tautschnig in
#3000
* Remove assigns clause for ZST pointers by @carolynzech in
#3417
* Instrumentation for delayed UB stemming from uninitialized memory by
@artemagvanian in #3374
* Unify kani library and kani core logic by @jaisnan in
#3333
* Stabilize pointer-to-reference cast validity checks by @artemagvanian
in #3426
* Rust toolchain upgraded to `nightly-2024-08-07` by @jaisnan @qinheping
@tautschnig @feliperodri

## New Contributors
* @carolynzech made their first contribution in
#3387

**Full Changelog**:
kani-0.53.0...kani-0.54.0

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

Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
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.

Stabilize pointer-to-reference cast validity checks
3 participants