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

added harnesses for c_str: is_empty #195

Closed
wants to merge 93 commits into from

Conversation

lanfeima
Copy link

@lanfeima lanfeima commented Nov 28, 2024

Update:

I noticed that PR #194 leverages arbitrary_cstr (introduced in #189) to simplify the harness CStr::is_empty. Please also check that PR for additional context.

Towards: Issue #150

Parent branch: main


Changes

  • added harnesses. for CStr:: is_empty

This version of C

To revalidate the verification results, run ./scripts/run-kani.sh --kani-args --harness ffi::c_str::verify::check_is_empty

Example output:

Checking harness ffi::c_str::verify::check_is_empty...

VERIFICATION RESULT:
 ** 0 of 216 failed (5 unreachable)

VERIFICATION:- SUCCESSFUL
Verification Time: 1121.6111s

Complete - 1 successfully verified harnesses, 0 failures, 1 total.

yew005 and others added 30 commits September 10, 2024 17:11
…ints' into c-0011-core-nums-junfengj-unchecked-shl
@lanfeima lanfeima requested a review from a team as a code owner November 28, 2024 12:33
@lanfeima lanfeima changed the title added harnesses for C_str: is_empty added harnesses for c_str: is_empty Nov 28, 2024
@Yenyun035
Copy link

Duplicate with #194. Will resolve later.

@lanfeima lanfeima marked this pull request as draft November 28, 2024 20:24
@lanfeima
Copy link
Author

lanfeima commented Nov 29, 2024

I see that PR #194 takes an efficient approach by leveraging arbitrary_cstr. The main difference is that PR #195 explores handling invalid cases during the construction phase (e.g., how is_empty behaves under invalid input), which takes a long time and might not be necessary given that is_empty can only be invoked on valid CStr instances. I now realize that focusing on valid inputs may be more aligned with the project goals.

Would it make sense to address invalid cases elsewhere, or should we focus solely on valid instances here? Feedback from reviewers would be helpful. I’d be happy to close this PR or contribute to refining #194 if needed. Thank you!

@lanfeima lanfeima marked this pull request as ready for review November 29, 2024 08:54
@celinval
Copy link

Can we merge the ideas by optimizing the arbitrary_str by adding byte 0 to the end of the slice?

@Yenyun035
Copy link

Yenyun035 commented Nov 30, 2024

Can we merge the ideas by optimizing the arbitrary_str by adding byte 0 to the end of the slice?

@celinval If I understand correctly, do you mean that we ensure the input slice is null-terminated before passing it to arbitrary_cstr? This sounds equivalent to the idea that we assume the input slice is null-terminated within arbitrary_cstr, e.g.

fn arbitrary_cstr(slice: &[u8]) -> &CStr {
    // At a minimum, the slice contains a null byte to form a valid CStr.
    kani::assume(slice.len() > 0 && slice[slice.len() - 1] == 0);
    let result = CStr::from_bytes_until_nul(&slice);
    // Given the assumption, from_bytes_until_nul should never fail
    kani::assume(result.is_ok()); // maybe change to assert!(result.is_ok());
    let c_str = result.unwrap();
    assert!(c_str.is_safe());
    c_str
}

I updated PR#194 accordingly.

@Yenyun035
Copy link

PR #194 was merged. This PR can be closed. @lanfeima

@tautschnig
Copy link
Member

Closing per #195 (comment).

@tautschnig tautschnig closed this Dec 2, 2024
Sign up for free to 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.

6 participants