Skip to content

Commit

Permalink
Auto merge of #2363 - emarteca:int-function-args-returns, r=oli-obk
Browse files Browse the repository at this point in the history
Adding support for external C functions that have integer (or empty) args and/or returns

Starts addressing `@https://github.com/rust-lang/miri/issues/11`

### Implementation
Adding support for calling external C functions that have any number of integer arguments (types of integers: `i8`, `i16`, `i32`, `i64`, `u8`, `u16`, `u32`, `u64`) and an integer return type (or `void`).
As suggested in `@https://github.com/rust-lang/miri/issues/11,` the [`libffi` crate](https://docs.rs/libffi/latest/libffi/index.html) is used to dispatch the calls to external C functions.

#### Modifications
Main modifications are to:
* [helper.rs](https://github.com/emarteca/miri/blob/int-function-args-returns/src/helpers.rs) : adding a function `call_and_add_external_c_fct_to_context` to read the code pointer to the external C function, dispatch the call, and save the return in MIRI internal memory. Handles all conversions between MIRI and C values (using some macros, also defined in this file).
* [foreign_items.rs](https://github.com/emarteca/miri/blob/int-function-args-returns/src/shims/foreign_items.rs) : handles the calling of `call_and_add_external_c_fct_to_context` in [helper.rs](https://github.com/emarteca/miri/blob/int-function-args-returns/src/helpers.rs) when a foreign item is encountered. Also adds some structs to model C representations of arguments, and the signature of the external C call.

### Testing
Adds tests for the following external functions which are now supported:
* [int tests](https://github.com/emarteca/miri/blob/int-function-args-returns/tests/pass/external_C/int_c_tests.rs):
     - adds 2 to a provided int (no type of int specified, so autocasts)
     - takes the sum of its 12 arguments (tests stack spill)
     - adds 3 to a 16 bit int
     - adds an `i16` to an `i64`
     - returns -10 as an unsigned int
* [void tests](https://github.com/emarteca/miri/blob/int-function-args-returns/tests/pass/external_C/print_from_c.rs)
     - void function that prints from C

### Code review
The code in this PR was reviewed by `@maurer` on [another fork](maurer#1) -- thanks!
  • Loading branch information
bors committed Aug 26, 2022
2 parents d5853bc + 88a7882 commit 6418501
Show file tree
Hide file tree
Showing 19 changed files with 565 additions and 4 deletions.
8 changes: 8 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ target
/doc
tex/*/out
*.dot
*.out
*.rs.bk
.vscode
*.mm_profdata
perf.data
perf.data.old
flamegraph.svg
<<<<<<< HEAD
=======
<<<<<<< HEAD
tests/extern-so/libtestlib.so
=======
>>>>>>> master
>>>>>>> 58ba05a0 (C FFI support for functions with int args and returns)
.auto-*
38 changes: 38 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ doctest = false # and no doc tests
[dependencies]
getrandom = { version = "0.2", features = ["std"] }
env_logger = "0.9"
libffi = "3.0.0"
libloading = "0.7"
log = "0.4"
shell-escape = "0.1.4"
rand = "0.8"
Expand Down
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,17 @@ to Miri failing to detect cases of undefined behavior in a program.
this flag is **unsound**.
* `-Zmiri-disable-weak-memory-emulation` disables the emulation of some C++11 weak
memory effects.
* `-Zmiri-extern-so-file=<path to a shared object file>` is an experimental flag for providing support
for FFI calls. Functions not provided by that file are still executed via the usual Miri shims.
**WARNING**: If an invalid/incorrect `.so` file is specified, this can cause undefined behaviour in Miri itself!
And of course, Miri cannot do any checks on the actions taken by the external code.
Note that Miri has its own handling of file descriptors, so if you want to replace *some* functions
working on file descriptors, you will have to replace *all* of them, or the two kinds of
file descriptors will be mixed up.
This is **work in progress**; currently, only integer arguments and return values are
supported (and no, pointer/integer casts to work around this limitation will not work;
they will fail horribly).
Follow [the discussion on supporting other types](https://github.com/rust-lang/miri/issues/2365).
* `-Zmiri-measureme=<name>` enables `measureme` profiling for the interpreted program.
This can be used to find which parts of your program are executing slowly under Miri.
The profile is written out to a file with the prefix `<name>`, and can be processed
Expand Down
6 changes: 6 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn main() {
// Re-export the TARGET environment variable so it can
// be accessed by miri.
let target = std::env::var("TARGET").unwrap();
println!("cargo:rustc-env=TARGET={:?}", target);
}
1 change: 1 addition & 0 deletions miri
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ esac

## Prepare the environment
# Determine some toolchain properties
# export the target so its available in miri
TARGET=$(rustc +$TOOLCHAIN --version --verbose | grep "^host:" | cut -d ' ' -f 2)
SYSROOT=$(rustc +$TOOLCHAIN --print sysroot)
LIBDIR=$SYSROOT/lib/rustlib/$TARGET/lib
Expand Down
13 changes: 13 additions & 0 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,19 @@ fn main() {
"full" => BacktraceStyle::Full,
_ => show_error!("-Zmiri-backtrace may only be 0, 1, or full"),
};
} else if let Some(param) = arg.strip_prefix("-Zmiri-extern-so-file=") {
let filename = param.to_string();
if std::path::Path::new(&filename).exists() {
if let Some(other_filename) = miri_config.external_so_file {
panic!(
"-Zmiri-extern-so-file external SO file is already set to {}",
other_filename.display()
);
}
miri_config.external_so_file = Some(filename.into());
} else {
panic!("-Zmiri-extern-so-file path {} does not exist", filename);
}
} else {
// Forward to rustc.
rustc_args.push(arg);
Expand Down
5 changes: 5 additions & 0 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::ffi::{OsStr, OsString};
use std::iter;
use std::panic::{self, AssertUnwindSafe};
use std::path::PathBuf;
use std::thread;

use log::info;
Expand Down Expand Up @@ -128,6 +129,9 @@ pub struct MiriConfig {
pub report_progress: Option<u32>,
/// Whether Stacked Borrows retagging should recurse into fields of datatypes.
pub retag_fields: bool,
/// The location of a shared object file to load when calling external functions
/// FIXME! consider allowing users to specify paths to multiple SO files, or to a directory
pub external_so_file: Option<PathBuf>,
}

impl Default for MiriConfig {
Expand Down Expand Up @@ -159,6 +163,7 @@ impl Default for MiriConfig {
preemption_rate: 0.01, // 1%
report_progress: None,
retag_fields: false,
external_so_file: None,
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,14 @@ pub struct Evaluator<'mir, 'tcx> {
pub(crate) report_progress: Option<u32>,
/// The number of blocks that passed since the last progress report.
pub(crate) since_progress_report: u32,

/// Handle of the optional shared object file for external functions.
pub external_so_lib: Option<(libloading::Library, std::path::PathBuf)>,
}

impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Self {
let target_triple = &layout_cx.tcx.sess.opts.target_triple.to_string();
let local_crates = helpers::get_local_crates(layout_cx.tcx);
let layouts =
PrimitiveLayouts::new(layout_cx).expect("Couldn't get layouts of primitive types");
Expand Down Expand Up @@ -412,6 +416,24 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
preemption_rate: config.preemption_rate,
report_progress: config.report_progress,
since_progress_report: 0,
external_so_lib: config.external_so_file.as_ref().map(|lib_file_path| {
// Check if host target == the session target.
if option_env!("TARGET") == Some(target_triple) {
panic!(
"calling external C functions in linked .so file requires target and host to be the same"
);
}
// Note: it is the user's responsibility to provide a correct SO file.
// WATCH OUT: If an invalid/incorrect SO file is specified, this can cause
// undefined behaviour in Miri itself!
(
unsafe {
libloading::Library::new(lib_file_path)
.expect("Failed to read specified shared object file")
},
lib_file_path.clone(),
)
}),
}
}

Expand Down
Loading

0 comments on commit 6418501

Please sign in to comment.