Skip to content

Instrumentation for delayed UB stemming from uninitialized memory - #3374

Merged
artemagvanian merged 64 commits into
model-checking:mainfrom
artemagvanian:delayed-ub
Aug 5, 2024
Merged

Instrumentation for delayed UB stemming from uninitialized memory#3374
artemagvanian merged 64 commits into
model-checking:mainfrom
artemagvanian:delayed-ub

Conversation

@artemagvanian

@artemagvanianartemagvanian commented Jul 23, 2024

Copy link
Copy Markdown
Contributor

As #3324 mentioned, delayed UB triggered by accessing uninitialized memory was previously mitigated by injecting assert!(false) for every possible source of it; this PR adds all the necessary infrastructure and minimal support for detecting it properly.

The process of detecting and instrumenting places is as follows:

  • Find all sources of delayed uninitialized memory UB (mutable pointer casts with different pointee padding and copy intrinsics);
  • Compute conservative aliasing graph between all memory places reachable from each harness;
  • Instrument all places pointed to by each source we found in step 1.

Not all language constructs are currently supported -- see the tracking issue (#3300) for the full list of limitations.

Resolves#3324

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

@artemagvanianartemagvanian self-assigned this Jul 23, 2024
@github-actionsgithub-actionsBot added the Z-EndToEndBenchCI Tag a PR to run benchmark CI label Jul 23, 2024
@artemagvanianartemagvanian mentioned this pull request Jul 23, 2024
Open
14 tasks
@artemagvanian
artemagvanian marked this pull request as ready for review July 26, 2024 16:31
@artemagvanian
artemagvanian requested a review from a team as a code ownerJuly 26, 2024 16:31
zhassan-aws
zhassan-aws reviewed Jul 27, 2024
View reviewed changes

@zhassan-awszhassan-aws left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Having a points-to analysis in Rust is awesome!

Comment threadtests/expected/uninit/delayed-ub/delayed-ub.rs
Comment threadtests/expected/uninit/delayed-ub/expected Outdated
Comment threadtests/expected/uninit/intrinsics/expected Outdated
Comment threadkani-compiler/src/codegen_cprover_gotoc/compiler_interface.rs
Comment threadkani-compiler/src/codegen_cprover_gotoc/compiler_interface.rs
Comment threadkani-compiler/src/kani_middle/transform/check_uninit/delayed_ub/instrumentation_visitor.rs Outdated
celinval
celinval reviewed Aug 2, 2024
View reviewed changes

@celinvalcelinval left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs Outdated
Comment threadkani-compiler/src/kani_middle/points_to/points_to_analysis.rs
@artemagvanian
artemagvanian added this pull request to the merge queueAug 5, 2024
Merged via the queue into model-checking:main with commit f2831f4Aug 5, 2024
@artemagvanian
artemagvanian deleted the delayed-ub branch August 5, 2024 17:21
This was referenced Aug 5, 2024
Merged
Merged
@feliperodrifeliperodri mentioned this pull request Aug 8, 2024
Merged
github-merge-queueBot pushed a commit that referenced this pull request Aug 9, 2024
@feliperodri
Bump Kani version to 0.54.0 (#3430)
7a6f1a4
## 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>
@artemagvanianartemagvanian mentioned this pull request Aug 14, 2024
Merged
github-merge-queueBot pushed a commit that referenced this pull request Aug 22, 2024
@artemagvanian
Avoid corner-cases by grouping instrumentation into basic blocks and …
0ed9a62
…using backward iteration (#3438)
This PR is a follow-up to
#3374.
Its main purpose is to properly handle a corner-case when multiple
instrumentation instructions are added to the same instruction and not
all of them are skipped during subsequent instrumentation.
For example, if instrumentation is added after a terminator, a new basic
block will be created, containing the added instrumentation. However, we
currently only mark the first statement (or the terminator, if there are
none) of the new basic block as skipped for subsequent instrumentation.
That means that if instrumentation in this case contains some
instrumentation targets itself, it will never terminate.
Coincidentally, this is not currently the case, but could lead to subtle
bugs if we decide to change instrumentation. In fact, this bug was only
surfaced when I experimented with checking all memory accesses, which
introduced significantly more checks. Ultimately, this shows that the
current way to avoiding instrumentation is limited and needs to be
changed.
The PR introduces the following changes:
- Group instrumentation into separate basic blocks instead of adding
instructions one-by-one.
- Use backward iteration to avoid having to reason about which
instructions need to be skipped.
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
github-merge-queueBot pushed a commit that referenced this pull request Aug 28, 2024
@artemagvanian
Extra tests and bug fixes to the delayed UB instrumentation (#3419)
e28f3db
This PR is a follow-up to #3374.
It introduces the following changes:
- Instrument more writes to avoid the case when points-to analysis
overapproximates too much.
- Add extra tests featuring safe abstractions from standard library.
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
@AlexanderPortlandAlexanderPortland mentioned this pull request Jun 16, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@celinvalcelinvalcelinval left review comments
+1 more reviewer
@zhassan-awszhassan-awszhassan-aws approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@artemagvanianartemagvanian

Labels

Z-EndToEndBenchCITag a PR to run benchmark CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uninitialized memory detection mitigates delayed UB, but does not support it fully

3 participants

@artemagvanian@celinval@zhassan-aws