Skip to content

Add a few intrinsics contracts - #37

Merged
celinval merged 18 commits into
model-checking:mainfrom
celinval:verify-intrinsics
Dec 7, 2024
Merged

Add a few intrinsics contracts#37
celinval merged 18 commits into
model-checking:mainfrom
celinval:verify-intrinsics

Conversation

@celinval

@celinvalcelinval commented Jul 16, 2024

Copy link
Copy Markdown

Here are a few limitations:

  1. Harness forwrite_bytes was disabled due to:
  2. The harnesses explicitly disable cases where a pointer is dangling.
  3. Actual intrinsics are very hard to verify with Kani. The cases we can verify are those that have wrappers around the actual intrinsic.

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 not working due to a Kani limitation
The intrinsics `copy` and `copy_nonoverlapping` are untyped copies,
so they don't respect the validity requirements of `T`.
@rahulku

Copy link
Copy Markdown

is this waiting for something?

@celinval

Copy link
Copy Markdown
Author

Yes, I need to update the branch and tests.

# Conflicts:
#	library/core/src/intrinsics.rs
@celinval

Copy link
Copy Markdown
Author

FYI, this is blocked until we merge the PointerGenerator into Kani: model-checking/kani#3538

@celinval
celinval marked this pull request as ready for review October 16, 2024 23:41
@celinval
celinval requested a review from a team as a code ownerOctober 16, 2024 23:41
@celinval

Copy link
Copy Markdown
Author

Just an update. I haven't been able to figure out yet why the copy_nonoverlapping harness is failing yet. The error is:

Checking harness intrinsics::verify::check_copy_nonoverlapping...
VERIFICATION RESULT:
** 1 of 856 failed (3 unreachable)
Failed Checks: Only a single top-level call to function _RNCNCINvNtCsf4mLL5B4Lhf_4core10intrinsics19copy_nonoverlappinghEs0_0s_0B8_ when checking contract _RNCNCINvNtCsf4mLL5B4Lhf_4core10intrinsics19copy_nonoverlappinghEs0_0s_0B8_
2024-11-02T01:43:52.2508870Z File: "/Users/runner/work/verify-rust-std/verify-rust-std/head/library/core/src/intrinsics.rs", line 3311, in _RNCNCINvNtCsf4mLL5B4Lhf_4core10intrinsics19copy_nonoverlappinghEs0_0s_0B8_
VERIFICATION:- FAILED

which doesn't make sense to me. The harness structure is exactly the same as copy, and so is the function implementation. 🤷‍♀️

@qinheping
qinheping self-requested a review November 6, 2024 18:03
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated

@qinhepingqinheping left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, thank you!

Comment threadlibrary/core/src/intrinsics.rs Outdated
@tautschnig

Copy link
Copy Markdown
Member

Just an update. I haven't been able to figure out yet why the copy_nonoverlapping harness is failing yet. The error is:

Checking harness intrinsics::verify::check_copy_nonoverlapping...
VERIFICATION RESULT:
** 1 of 856 failed (3 unreachable)
Failed Checks: Only a single top-level call to function _RNCNCINvNtCsf4mLL5B4Lhf_4core10intrinsics19copy_nonoverlappinghEs0_0s_0B8_ when checking contract _RNCNCINvNtCsf4mLL5B4Lhf_4core10intrinsics19copy_nonoverlappinghEs0_0s_0B8_
2024-11-02T01:43:52.2508870Z File: "/Users/runner/work/verify-rust-std/verify-rust-std/head/library/core/src/intrinsics.rs", line 3311, in _RNCNCINvNtCsf4mLL5B4Lhf_4core10intrinsics19copy_nonoverlappinghEs0_0s_0B8_
VERIFICATION:- FAILED

which doesn't make sense to me. The harness structure is exactly the same as copy, and so is the function implementation. 🤷‍♀️

I have seen the very same problem on #120, and I haven't yet managed to understand why.

@tautschnig

Copy link
Copy Markdown
Member

Here are a few problems that I bumped into: [...]

Before merning, would you mind adding to the PR description also what actually does work?

Comment threaddoc/src/challenges/0002-intrinsics-memory.md Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
Comment threadlibrary/core/src/intrinsics.rs Outdated
@celinval

Copy link
Copy Markdown
Author

@tautschnig can you please see my answer to your comments? Is it OK if I move ahead and merge these changes?

@celinval

Copy link
Copy Markdown
Author

I need to solve the new conflicts. After that, I'll merge this PR

Since intrinsics file has been deleted, we will have to reapply the
patch.
Conflicts:
- library/core/src/intrinsics.rs
Co-authored-by: Michael Tautschnig <mt@debian.org>
@celinval

Copy link
Copy Markdown
Author

I updated this PR to account to the fact that the intrinsics.rs file was moved to intrinsics/mod.rs. I have also disabled the write_bytes harness because of model-checking/kani#90.

@tautschnig can you please take a look and see if this is good to go?

@celinvalcelinval assigned tautschnig and unassigned celinvalDec 6, 2024
@celinval
celinval enabled auto-merge (squash) December 6, 2024 23:45
@celinval
celinval merged commit d92a7ea into model-checking:mainDec 7, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@celinval@rahulku@tautschnig@qinheping