Skip to content

Expose CProver API version tag through to Rust API #7410

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

Merged

Conversation

NlightNFotis
Copy link
Contributor

@NlightNFotis NlightNFotis commented Dec 2, 2022

This for now sets up the build infrastructure, both in CMake (to be able to build the Rust bit as part of a CBMC build) and inside of the Rust project, with cargo (so that API tests can be run independently - by just issuing cargo test inside the src/libcprover-rust/ folder).

It also exposes the functions from the C++ API to the Rust project, which exercises them through its own test suite.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@NlightNFotis NlightNFotis added the Kani Bugs or features of importance to Kani Rust Verifier label Dec 2, 2022
@NlightNFotis NlightNFotis self-assigned this Dec 2, 2022
@NlightNFotis NlightNFotis force-pushed the expose_version_tag_to_rust branch from 4d9a81d to 7ba2cb7 Compare December 2, 2022 00:47

// A simple API version information function.
// TODO: Investigate best way to store and expose version string.
std::string get_api_version() const { return std::string{"0.1"}; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some kind of advantage to versioning the API separately from the rest of CBMC? We could just return the CBMC_VERSION constant here.

@feliperodri feliperodri added the aws Bugs or features of importance to AWS CBMC users label Dec 5, 2022
@NlightNFotis NlightNFotis force-pushed the expose_version_tag_to_rust branch from 7ba2cb7 to cb26ef2 Compare December 7, 2022 14:44
let client = ffi::new_cprover_api();
let result = ffi::get_api_version(client);
println!("{}", *result);
// assert_eq!(*result, "0.1");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we assert here?

Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ "0.1" or "0.1.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an older version of the code - we do assert now.

.flag_if_supported("-std=c++11")
.compile("cprover-rust-api");

println!("cargo:rustc-link-search=native=/Users/fotiskoutoulakis/Devel/cbmc/build/lib/");
Copy link
Contributor

Choose a reason for hiding this comment

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

hardcoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, yes - I'm aware. I will fix this once I rebase on top of some changes we expect to go in soon that will affect the C++ side of things, forcing some changes to this bit as well.

On the build side of thing, it will also need us to install a CMake module (corrosion) for building the Rust project along the C++ project, but I will do them both at the same time.

class api_optionst
{
// Options for the verification engine
bool simplify_enabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not exported to the Rust side yet - there's some thinking we need to do as to what's the best way to support some of these items (be they options that feed a verification-pipeline, or exported as calls that the user can then call when they want).

println!("cargo:rerun-if-changed=src/c_api.cc");
println!("cargo:rerun-if-changed=include/c_api.h");

println!("cargo:rustc-link-search=native=/Users/fotiskoutoulakis/Devel/cbmc/build/lib/");
Copy link
Contributor

Choose a reason for hiding this comment

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

hardcoded to your machine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto as above - we expect to fix imminently.

@NlightNFotis NlightNFotis force-pushed the expose_version_tag_to_rust branch 6 times, most recently from d5754a3 to e5a5041 Compare December 21, 2022 16:16
@NlightNFotis NlightNFotis marked this pull request as ready for review December 21, 2022 16:16

/// Validate the loaded goto model
void validate_goto_model();
void validate_goto_model() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these are void. How do we expect to get back information about the result of validating the model?

@@ -0,0 +1,2 @@
# Ignore build artefacts folder
Copy link
Contributor

Choose a reason for hiding this comment

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

🇬🇧 spelling I see :)

@@ -0,0 +1,15 @@
[package]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the plan to publish this to crates.io

type api_sessiont;

// API Functions
fn new_api_session() -> UniquePtr<api_sessiont>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this manually written, or autogenerated?

fn print_response(vec: &CxxVector<CxxString>) {
let vec: Vec<String> = vec
.iter()
.map(|s| s.to_string_lossy().into_owned())
Copy link
Contributor

Choose a reason for hiding this comment

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

What encoding does CBMC use? Do we need this, or would to_str be sufficient?

fn print_response(vec: &CxxVector<CxxString>) {
let vec: Vec<String> = vec
.iter()
.map(|s| s.to_string_lossy().into_owned())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is into_owned needed? Is this an expectation of the API that ownership move here?


// Invoke load_model_from_files and see if the model
// has been loaded.
client.load_model_from_files(vect);
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this check that the loading happened?

client.validate_goto_model();

let msgs = ffi::get_messages();
print_response(msgs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Anything to check here?

@NlightNFotis NlightNFotis force-pushed the expose_version_tag_to_rust branch from e5a5041 to 2a0f03b Compare January 10, 2023 12:18
@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Base: 78.47% // Head: 78.48% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (c2cba5b) compared to base (251c257).
Patch coverage: 97.82% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop    #7410    +/-   ##
=========================================
  Coverage    78.47%   78.48%            
=========================================
  Files         1662     1663     +1     
  Lines       191013   191188   +175     
=========================================
+ Hits        149899   150054   +155     
- Misses       41114    41134    +20     
Impacted Files Coverage Δ
src/goto-instrument/contracts/contracts.h 100.00% <ø> (ø)
...t/contracts/dynamic-frames/dfcc_contract_handler.h 0.00% <ø> (ø)
src/libcprover-cpp/api.h 100.00% <ø> (ø)
src/solvers/sat/external_sat.cpp 81.60% <ø> (ø)
src/libcprover-cpp/api.cpp 81.94% <89.47%> (+1.94%) ⬆️
regression/libcprover-cpp/call_bmc.cpp 87.50% <100.00%> (+6.25%) ⬆️
.../goto-instrument/contracts/dynamic-frames/dfcc.cpp 98.02% <100.00%> (ø)
...contracts/dynamic-frames/dfcc_contract_handler.cpp 74.32% <100.00%> (ø)
src/libcprover-cpp/options.cpp 100.00% <100.00%> (ø)
... and 41 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@NlightNFotis NlightNFotis force-pushed the expose_version_tag_to_rust branch 4 times, most recently from b58d496 to 2fb16b4 Compare January 12, 2023 14:41
At the moment this supports the capacity for

* loading a goto_model from files
* verifying the goto_model
* performing a sample instrumentation pass
@NlightNFotis NlightNFotis force-pushed the expose_version_tag_to_rust branch from 2fb16b4 to b566c4f Compare January 12, 2023 14:46
@NlightNFotis NlightNFotis merged commit 12d5324 into diffblue:develop Jan 16, 2023
@NlightNFotis NlightNFotis deleted the expose_version_tag_to_rust branch January 16, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Bugs or features of importance to AWS CBMC users Kani Bugs or features of importance to Kani Rust Verifier
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants