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

Limit FFI calls by default to explicitly supported ones #2428

Merged
merged 9 commits into from
May 5, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented May 3, 2023

Description of changes:

C-FFI is fairly unstable right now and the lack of error handling causes a poor experience when things go wrong.

In this PR, we limit its usage to only support CBMC built-in functions that are explicitly included in Kani and alloc functions from kani_lib.c. User's should still be enable to go back to previous behavior by enabling unstable C-FFI support

Resolved issues:

Resolves #1781

Related RFC:

Call-outs:

This PR is currently blocked by #2427. Once that gets merged, the regression should pass.

Testing:

  • How is this change tested? New tests

  • Is this a refactor change? Kinda

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

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

C-FFI is pretty broken right now. Limit its usage to only support
built-in functions that are explicitly included in Kani.

User's should still be enable to go back to previous behavior by
enabling unstable C-FFI support
@celinval celinval marked this pull request as ready for review May 4, 2023 02:04
@celinval celinval requested a review from a team as a code owner May 4, 2023 02:04
@feliperodri feliperodri self-requested a review May 4, 2023 19:26
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.

Looks good. Mostly minor comments.

tests/ui/missing-function/extern_c/extern_c.rs Outdated Show resolved Hide resolved
tests/kani/ForeignItems/extern_fn_ptr.rs Outdated Show resolved Hide resolved
tests/expected/foreign-function/ffi_ptr.rs Outdated Show resolved Hide resolved
tests/cargo-kani/libc/Cargo.toml Show resolved Hide resolved
Co-authored-by: Zyad Hassan <88045115+zhassan-aws@users.noreply.github.com>
  - Add / improve comments
  - Add expected file
  - Fix variadic test
  - improve test
celinval and others added 2 commits May 5, 2023 13:00
@celinval celinval enabled auto-merge (squash) May 5, 2023 20:08
@celinval celinval merged commit 6e91d13 into model-checking:main May 5, 2023
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.

function call: parameter "pthread_key_create::destructor" type mismatch when hitting thread::current()
3 participants