From 951370f77afd82cd45e81008de31d36ce43983cc Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Sat, 24 Sep 2022 20:32:49 -0700 Subject: [PATCH 1/6] Replace the `extern fn printf` with a non-variadic, Rust `fn printf` so that it works with `miri`. This works in `analysis/test` because all `printf`s are monomorphic. --- analysis/test/src/pointers.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/analysis/test/src/pointers.rs b/analysis/test/src/pointers.rs index d4f55cd56a..a618885f34 100644 --- a/analysis/test/src/pointers.rs +++ b/analysis/test/src/pointers.rs @@ -14,7 +14,14 @@ extern "C" { fn calloc(_: libc::c_ulong, _: libc::c_ulong) -> *mut libc::c_void; fn realloc(_: *mut libc::c_void, _: libc::c_ulong) -> *mut libc::c_void; fn free(__ptr: *mut libc::c_void); - fn printf(_: *const libc::c_char, _: ...) -> libc::c_int; +} + +fn printf(fmt: *const libc::c_char, i: i32) -> libc::c_int { + use std::ffi::CStr; + assert_eq!(unsafe { CStr::from_ptr(fmt) }, CStr::from_bytes_with_nul(b"%i\n\x00").unwrap()); + let s = format!("{i}\n"); + print!("{s}"); + s.len() as libc::c_int } /// Hidden from instrumentation so that we can polyfill [`reallocarray`] with it. From 43cd6b2dcb90afe7087b0e08c0e28f409a78c134 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Sun, 25 Sep 2022 04:01:35 -0700 Subject: [PATCH 2/6] Added a `miri` feature. When set, the miri-compatible, monomorphic `printf` shim is defined instead. --- analysis/test/Cargo.toml | 3 +++ analysis/test/src/pointers.rs | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/analysis/test/Cargo.toml b/analysis/test/Cargo.toml index 5055716415..eccd834856 100644 --- a/analysis/test/Cargo.toml +++ b/analysis/test/Cargo.toml @@ -8,3 +8,6 @@ edition = "2021" [dependencies] libc = "0.2" c2rust-analysis-rt = { path = "../runtime", optional = true, version = "0.1.0" } + +[features] +miri = [] diff --git a/analysis/test/src/pointers.rs b/analysis/test/src/pointers.rs index a618885f34..4d882dbe47 100644 --- a/analysis/test/src/pointers.rs +++ b/analysis/test/src/pointers.rs @@ -16,6 +16,12 @@ extern "C" { fn free(__ptr: *mut libc::c_void); } +#[cfg(not(feature = "miri"))] +extern "C" { + fn printf(_: *const libc::c_char, _: ...) -> libc::c_int; +} + +#[cfg(feature = "miri")] fn printf(fmt: *const libc::c_char, i: i32) -> libc::c_int { use std::ffi::CStr; assert_eq!(unsafe { CStr::from_ptr(fmt) }, CStr::from_bytes_with_nul(b"%i\n\x00").unwrap()); From 5b1194432be6ae406ce666862151660b2f0b5430 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Sun, 25 Sep 2022 04:08:53 -0700 Subject: [PATCH 3/6] Added a test in `c2rust-pdg` that runs `cd analysis/test && cargo miri run --features miri`. --- pdg/src/main.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pdg/src/main.rs b/pdg/src/main.rs index 927e0656c6..a6433bb3f3 100644 --- a/pdg/src/main.rs +++ b/pdg/src/main.rs @@ -365,4 +365,16 @@ mod tests { insta::assert_display_snapshot!(pdg); Ok(()) } + + #[test] + fn analysis_test_miri() -> eyre::Result<()> { + init(); + let mut cmd = Command::new("cargo"); + cmd.current_dir(repo_dir()?.join("analysis/test")) + .args(&["miri", "run", "--features", "miri"]) + .env("MIRIFLAGS", ""); + let status = cmd.status()?; + ensure!(status.success(), eyre!("{cmd:?} failed: {status}")); + Ok(()) + } } From 654b0c055fbf015da024da6be5c021ef33f736ba Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Sun, 25 Sep 2022 04:15:41 -0700 Subject: [PATCH 4/6] Added `miri` to the needed components in `rust-toolchain.toml` now that it's used to test `analysis/test`. --- rust-toolchain.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 7b98babb0e..f4d107d9d2 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] channel = "nightly-2022-08-08" -components = ["rustfmt-preview", "rustc-dev", "rust-src"] +components = ["rustfmt-preview", "rustc-dev", "rust-src", "miri"] From 33130b7e0e2da284caa936d99604519f4b74d007 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 27 Sep 2022 20:25:16 -0700 Subject: [PATCH 5/6] Added docs for the `printf` shim for `miri` compat. --- analysis/test/src/pointers.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/analysis/test/src/pointers.rs b/analysis/test/src/pointers.rs index 4d882dbe47..fe0b88374f 100644 --- a/analysis/test/src/pointers.rs +++ b/analysis/test/src/pointers.rs @@ -21,6 +21,18 @@ extern "C" { fn printf(_: *const libc::c_char, _: ...) -> libc::c_int; } +/// `miri` does not support calling variadic functions like [`printf`], +/// but we want to test for UB, leaks, etc. using `cargo miri run`. +/// +/// Luckily, all [`printf`] calls in this module are monomorphic, +/// in that they all have the same format string and same call signature, +/// so we can replace it with a [`printf`] shim that preserves the behavior +/// only for the exact monomorphic usages in this module. +/// +/// Note that there is no way to detect `miri` is running, +/// so we have to put this under a separate `miri` feature +/// that should be enabled when testing under `miri` with +/// `cargo miri run --features miri`. #[cfg(feature = "miri")] fn printf(fmt: *const libc::c_char, i: i32) -> libc::c_int { use std::ffi::CStr; From 4152d34d1f7140f28b346f4be52f0500aa7d7bc4 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Sun, 9 Oct 2022 17:37:12 -0700 Subject: [PATCH 6/6] `#[ignore]` the `analysis_test_miri` test for now as there are issues running `miri` (which installs `xargo`) in CI. The test can still be manually run with `cargo test -p c2rust-pdg -- --ignored analysis_test_miri`. --- pdg/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pdg/src/main.rs b/pdg/src/main.rs index a6433bb3f3..bc9ef13e42 100644 --- a/pdg/src/main.rs +++ b/pdg/src/main.rs @@ -367,6 +367,7 @@ mod tests { } #[test] + #[ignore] fn analysis_test_miri() -> eyre::Result<()> { init(); let mut cmd = Command::new("cargo");