Skip to content

Rust crate preparation - final touches #7625

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/pull-request-check-rust-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ on:
env:
default_build_dir: "build/"
default_solver: "minisat2"
default_include_dir: "src/libcprover-cpp/"

# For now, we support two jobs: A Linux and a MacOS based one.
# Both of the jobs use CMake, as we only support building the Rust
Expand Down Expand Up @@ -64,7 +65,7 @@ jobs:
VERSION=$(cat src/config.inc | python3 -c "import sys,re;line = [line for line in sys.stdin if re.search('CBMC_VERSION = (\d+\.\d+\.\d+)', line)];sys.stdout.write(re.search('CBMC_VERSION = (\d+\.\d+\.\d+)', line[0]).group(1))")
cd src/libcprover-rust;\
cargo clean;\
CBMC_LIB_DIR=../../${{env.default_build_dir}}/lib CBMC_VERSION=$VERSION cargo test -- --test-threads=1
CBMC_INCLUDE_DIR=../../${{env.default_include_dir}} CBMC_LIB_DIR=../../${{env.default_build_dir}}/lib CBMC_VERSION=$VERSION cargo test -- --test-threads=1


check-macos-12-cmake-clang-rust:
Expand Down Expand Up @@ -104,4 +105,4 @@ jobs:
VERSION=$(cat src/config.inc | python3 -c "import sys,re;line = [line for line in sys.stdin if re.search('CBMC_VERSION = (\d+\.\d+\.\d+)', line)];sys.stdout.write(re.search('CBMC_VERSION = (\d+\.\d+\.\d+)', line[0]).group(1))")
cd src/libcprover-rust;\
cargo clean;\
CBMC_LIB_DIR=../../${{env.default_build_dir}}/lib CBMC_VERSION=$VERSION cargo test -- --test-threads=1
CBMC_INCLUDE_DIR=../../${{env.default_include_dir}} CBMC_LIB_DIR=../../${{env.default_build_dir}}/lib CBMC_VERSION=$VERSION cargo test -- --test-threads=1
2 changes: 1 addition & 1 deletion src/libcprover-rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description = "Rust API for CBMC and assorted CProver tools"
repository = "https://github.com/diffblue/cbmc"
documentation = "https://diffblue.github.io/cbmc/"
license = "BSD-4-Clause"
exclude = ["other/", "module_dependencies.txt"]
exclude = ["module_dependencies.txt"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand Down
37 changes: 32 additions & 5 deletions src/libcprover-rust/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::env;
use std::env::VarError;
use std::io::{Error, ErrorKind};
use std::path::Path;
use std::path::PathBuf;

fn get_current_working_dir() -> std::io::Result<PathBuf> {
Expand Down Expand Up @@ -37,12 +36,40 @@ fn get_library_build_dir() -> std::io::Result<PathBuf> {
))
}

// This is needed so that cargo can find the include folder for the C++
// API headers at compile time.
fn get_include_directory_envvar() -> Result<String, VarError> {
env::var("CBMC_INCLUDE_DIR")
}

fn get_include_directory() -> std::io::Result<PathBuf> {
let env_var_fetch_result = get_include_directory_envvar();
if let Ok(build_dir) = env_var_fetch_result {
let mut path = PathBuf::new();
path.push(build_dir);
return Ok(path);
}
Err(Error::new(
ErrorKind::Other,
"Environment variable `CBMC_INCLUDE_DIR' not set",
))
}

fn main() {
let cbmc_source_path = Path::new("..");
let cpp_api_path = Path::new("../libcprover-cpp/");
let cpp_api_include_path = match get_include_directory() {
Ok(path) => path,
Err(err) => {
let error_message = &format!(
"Error: {}.\n Advice: {}.",
err,
"Please set the environment variable `CBMC_INCLUDE_DIR' with the path that contains cprover/api.h on your system"
);
panic!("{}", error_message);
}
};

let _build = cxx_build::bridge("src/lib.rs")
.include(cbmc_source_path)
.include(cpp_api_path)
.include(cpp_api_include_path)
.include(get_current_working_dir().unwrap())
.file("src/c_api.cc")
.flag_if_supported("-std=c++11")
Expand Down
5 changes: 3 additions & 2 deletions src/libcprover-rust/include/c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

#pragma once

#include <util/exception_utils.h>

#include <memory>
#include <string>

// NOLINTNEXTLINE(build/include)
#include "rust/cxx.h"
// NOLINTNEXTLINE(build/include)
#include "include/c_errors.h"

struct api_sessiont;

Expand Down
82 changes: 82 additions & 0 deletions src/libcprover-rust/include/c_errors.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Author: Fotis Koutoulakis for Diffblue Ltd, 2023.

#pragma once

// The following type is cloning two types from the `util/exception_utils.h` and
// `util/invariant.h` files.
//
// The reason we need to do this is as follows: We have a fundamental constraint
// in that we don't want to export internal headers to the clients, and our
// current build system architecture on the C++ end doesn't allow us to do so.
//
// At the same time, we want to allow the Rust API to be able to catch at the
// shimlevel the errors generated within CBMC, which are C++ types (and
Copy link
Contributor

Choose a reason for hiding this comment

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

shimlevel -> shim level

// subtypes of those), and so because of the mechanism that cxx.rs uses, we
// need to have thetypes present at compilation time (an incomplete type won't
// do - I've tried).
//
// This is the best way that we have currently to be have the type definitions
// around so that the exception handling code knows what our exceptions look
// like (especially given that they don't inherit from `std::exception`), so
// that our system compiles and is functional, without needing include chains
// outside of the API implementation (which we can't expose as well).

// This should mirror the definition in `util/invariant.h`.
class invariant_failedt
{
private:
const std::string file;
const std::string function;
const int line;
const std::string backtrace;
const std::string condition;
const std::string reason;

public:
virtual ~invariant_failedt() = default;

virtual std::string what() const noexcept;

invariant_failedt(
const std::string &_file,
const std::string &_function,
int _line,
const std::string &_backtrace,
const std::string &_condition,
const std::string &_reason)
: file(_file),
function(_function),
line(_line),
backtrace(_backtrace),
condition(_condition),
reason(_reason)
{
}
};

// This is needed here because the original definition is in the file
// <util/exception_utils.h> which is including <util/source_location.h>, which
// being an `irep` is a no-go for our needs as we will need to expose internal
// headers as well.
class cprover_exception_baset
{
public:
/// A human readable description of what went wrong.
/// For readability, implementors should not add a leading
/// or trailing newline to this description.
virtual std::string what() const;
virtual ~cprover_exception_baset() = default;

protected:
/// This constructor is marked protected to ensure this class isn't used
/// directly. Deriving classes should be used to more precisely describe the
/// problem that occurred.
explicit cprover_exception_baset(std::string reason)
: reason(std::move(reason))
{
}

/// The reason this exception was generated. This is the string returned by
/// `what()` unless that method is overridden
std::string reason;
};
123 changes: 118 additions & 5 deletions src/libcprover-rust/readme.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# CProver (CBMC) Rust API
# Libcprover-rust

This folder contains the implementation of the Rust API of the CProver (CBMC) project.
A Rust interface for convenient interaction with the CProver tools.

## Building instructions

Expand All @@ -15,7 +15,9 @@ project:

* `CBMC_LIB_DIR`, for selecting where the `libcprover-x.y.z.a` is located
(say, if you have downloaded a pre-packaged release which contains
the static library), and
the static library),
* `CBMC_INCLUDE_DIR`, for selecting where the `cprover/api.h` is located,
and
* `CBMC_VERSION`, for selecting the version of the library to link against
(this is useful if you have multiple versions of the library in the same
location and you want to control which version you compile against).
Expand All @@ -27,7 +29,7 @@ directory of the CBMC project.)
```sh
$ cd src/libcprover-rust
$ cargo clean
$ CBMC_LIB_DIR=../../build/lib CBMC_VERSION=5.78.0 cargo build
$ CBMC_INCLUDE_DIR=../../build/include CBMC_LIB_DIR=../../build/lib CBMC_VERSION=5.78.0 cargo build
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe don't fix a version of CBMC here, use something like CBMC_VERSION=X.XX.X?

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 in the documentation, and I meant it as a concrete example.

Should I still change this? Perhaps a good middle ground might be to add a reference to the version schema in the description of the environment variable, starting in line 19.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm agnostic on it. On the one hand I can see someone simply copying and pasting, on the other hand a concrete example is nicer than one with further things to modify.

```

To build the project and run its associated tests, the command sequence would
Expand All @@ -36,7 +38,118 @@ look like this:
```sh
$ cd src/libcprover-rust
$ cargo clean
$ CBMC_LIB_DIR=../../build/lib CBMC_VERSION=5.78.0 cargo test -- --test-threads=1 --nocapture
$ CBMC_INCLUDE_DIR=../../build/include CBMC_LIB_DIR=../../build/lib CBMC_VERSION=5.78.0 cargo test -- --test-threads=1 --nocapture
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

```

## Basic Usage

This file will guide through a sample interaction with the API, under a basic
scenario: *loading a file and verifying the model contained within*.

To begin, we will assume that you have a file under `/tmp/api_example.c`,
with the following contents:

```c
int main(int argc, char *argv[])
{
int arr[] = {0, 1, 2, 3};
__CPROVER_assert(arr[3] != 3, "expected failure: arr[3] == 3");
}
```

The first thing we need to do to initiate any interaction with the API
itself is to create a new `api_sessiont` handle by using the function
`new_api_session`:

```rust
let client = cprover_api::new_api_session();
```

Then, we need to add the file to a vector with filenames that indicate
which files we want the verification engine to load the models of:

```rust
let vec: Vec<String> = vec!["/tmp/api_example.c".to_owned()];

let vect = ffi_util::translate_rust_vector_to_cpp(vec);
```

In the above code example, we created a Rust language Vector of Strings
(`vec`). In the next line, we called a utility function from the module
`ffi_util` to translate the Rust `Vec<String>` into the C++ equivalent
`std::vector<std::string>` - this step is essential, as we need to translate
the type into something that the C++ end understands.

These operations are *explicit*: we have opted to force users to translate
between types at the FFI level in order to reduce the "magic" and instill
mental models more compatible with the nature of the language-border (FFI)
work. If we didn't, and we assumed the labour of translating these types
transparently at the API level, we risked mistakes from our end or from the
user end frustrating debugging efforts.

At this point, we have a handle of a C++ vector containing the filenames
of the files we want the CProver verification engine to load. To do so,
we're going to use the following piece of code:

```rust
// Invoke load_model_from_files and see if the model has been loaded.
if let Err(_) = client.load_model_from_files(vect) {
eprintln!("Failed to load model from files: {:?}", vect);
process::exit(1);
}
```

The above is an example of a Rust idiom known as a `if let` - it's effectively
a pattern match with just one pattern - we don't match any other case.

What we we do above is two-fold:

* We call the function `load_model_from_files` with the C++ vector (`vect`)
we prepared before. It's worth noting that this function is being called
with `client.` - what this does is that it passes the `api_session` handle
we initialised at the beginning as the first argument to the `load_model_from_files`
on the C++ API's end.
* We handled the case where the model loading failed for whatever reason from
the C++ end by catching the error on the Rust side and printing a suitable error
message and exiting the process gracefully.

---

*Interlude*: **Error Handling**

`cxx.rs` (the FFI bridge we're using to build the Rust API) allows for a mechanism
wherein exceptions from the C++ program can be translated into Rust `Result<>` types
provided suitable infrastructure has been built.

Our Rust API contains a C++ shim which (among other things) intercepts CProver
exceptions (from `cbmc`, etc.) and translates them into a form that the bridge
can then translate to appropriate `Result` types that the Rust clients can use.

This means that, as above, we can use the same Rust idioms and types as we would
use on a purely Rust based codebase to interact with the API.

*All of the API calls* are returning `Result` types such as above.

---

After we have loaded the model, we can proceed to then engage the verification
engine for an analysis run:

```rust
if let Err(_) = client.verify_model() {
eprintln!("Failed to verify model from files: {:?}", vect);
process::exit(1);
}
```

While all this is happening, we are collecting the output of the various
phases into a message buffer. We can go forward and print any messages from
that buffer into `stdout`:

```rust
let msgs_cpp = cprover_api::get_messages();
let msgs_rust = ffi_util::translate_cpp_vector_to_rust(msgs_cpp);
ffi_util::print_response(msgs_rust);
```

## Notes
Expand Down
10 changes: 5 additions & 5 deletions src/libcprover-rust/src/c_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
// NOLINTNEXTLINE(build/include)
#include "include/c_api.h"

#include <util/invariant.h>
#include <util/make_unique.h>

#include <libcprover-cpp/api.h>
// clang-format off
#include <api.h>
// clang-format on

#include <algorithm>
#include <cassert>
Expand All @@ -29,7 +28,8 @@ _translate_vector_of_string(rust::Vec<rust::String> elements)
std::back_inserter(*stdv),
[](rust::String elem) { return std::string(elem); });

POSTCONDITION(elements.size() == stdv->size());
// NOLINTNEXTLINE(build/deprecated)
assert(elements.size() == stdv->size());
return *stdv;
}

Expand Down
20 changes: 18 additions & 2 deletions src/libcprover-rust/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#![doc = include_str!("../tutorial.md")]
#![doc = include_str!("../readme.md")]
#![warn(missing_docs)]

/// The main API module for interfacing with CProver tools (`cbmc`, `goto-analyzer`, etc).
#[cxx::bridge]
pub mod cprover_api {

unsafe extern "C++" {
include!("libcprover-cpp/api.h");
include!("api.h");
include!("include/c_api.h");

/// Central organisational handle of the API. This directly corresponds to the
Expand Down Expand Up @@ -101,6 +101,22 @@ mod tests {
assert_eq!(vect.len(), 2);
}

// This test will capture a `system_exceptiont` from CBMC's end at the C++ shim that this
// library depends on, and it will be correctly translated into the Result type for Rust.
// This also validates that our type definition include of the base class for the exceptions
// works as we expect it to.
#[test]
fn it_translates_exceptions_to_errors() {
let client = cprover_api::new_api_session();

// The vector of string is supposed to contain a string denoting
// a filepath that is erroneous.
let vec: Vec<String> = vec!["/fkjsdlkjfisudifoj2309".to_owned()];
Copy link
Contributor

Choose a reason for hiding this comment

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

I have not tried to break this, but what happens if I have a valid input file at /fkjsdlkjfisudifoj2309? I'm also curious if the / is OS specific to any degree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, that was my best attempt at making a random string + file path combo, without resorting to a more elaborate mechanism based on random strings or whatnot.

I'm personally happy with it failing if a user does have a randomly (?) named file locally present. If that happens, the user/developer can just have a look at the test, and see that we are looking for this file, inspect his filesystem and locate the file, and potentially alter the test/delete the file.

let vect = ffi_util::translate_rust_vector_to_cpp(vec);

assert!(client.load_model_from_files(vect).is_err());
}

#[test]
fn it_can_load_model_from_file() {
let binding = cprover_api::new_api_session();
Expand Down
Loading