Repository: Gankra/abi-cafe (language: rust)

Issue:
sandbox executions?
Right now we build DLLs and load them into the test harness. If a test is mega-busted this can result in corrupting the harness or just locking it up. In an ideal world we would maybe instead build binaries that do ipc or write out a file to disk, so that the test harness could survive crashes or timeout a hung test.

Of course it would also be nice if we could *literally* sandbox the tests and prevent them from doing anything interesting, since all they ever need to be able to do is call a set of specific functions. They don't need i/o of any kind except to send the harness a report.


Discussion / hints:
The prophecies are fulfilled: in #65 cranelift<->rustc with OptionU128 fully SIGILL's during execution, making the whole harness suddenly hard-abort, as the SIGILL is in our process.

Fix patch:
diff --git a/include/harness/harness.rs b/include/harness/harness_lib.rs
similarity index 100%
rename from include/harness/harness.rs
rename to include/harness/harness_lib.rs
diff --git a/include/harness/harness_main.rs b/include/harness/harness_main.rs
new file mode 100644
index 0000000..a470fe1
--- /dev/null
+++ b/include/harness/harness_main.rs
@@ -0,0 +1,75 @@
+//! This is the primary file for the abi-cafe harness main that all tests are compiled into.
+//!
+//! This will be statically linked into a cdylib with two other static libraries:
+//! the caller and callee. The caller is expected to define the function `do_test`,
+//! and call a bunch of functions defined by the callee. The cdylib
+//! is run by the harness `dlopen`ing it and running `test_start`, passing in various
+//! buffers and callbacks for instrumenting the result of the execution.
+//!
+//! This instrumentation is only used in the default mode of `WriteImpl::HarnessCallback`.
+//! Otherwise the caller/callee may use things like asserts/prints.
+
+/// Tests write back the raw bytes of their values to a WriteBuffer.
+pub struct WriteBuffer {
+    pub identity: &'static str,
+}
+
+impl WriteBuffer {
+    fn new(identity: &'static str) -> Self {
+        // Preload the hierarchy for the first test.
+        WriteBuffer {
+            identity,
+        }
+    }
+}
+
+// The signatures of the interface from our perspective.
+// From the test's perspective the WriteBuffers are totally opaque.
+pub type SetFuncCallback = unsafe extern "C" fn(&mut WriteBuffer, u32) -> ();
+pub type WriteValCallback = unsafe extern "C" fn(&mut WriteBuffer, u32, *const u8, u32) -> ();
+pub type TestInit =
+    unsafe extern "C" fn(SetFuncCallback, WriteValCallback, &mut WriteBuffer, &mut WriteBuffer) -> ();
+
+pub unsafe extern "C" fn set_func(test: &mut WriteBuffer, func: u32) {
+    let ident = &test.identity;
+    println!(r#"{{ "info": "func", "id": "{ident}", "func": {func} }}"#);
+}
+
+pub unsafe extern "C" fn write_val(
+    test: &mut WriteBuffer,
+    val_idx: u32,
+    input: *const u8,
+    size: u32,
+) {
+    let data = std::slice::from_raw_parts(input, size as usize);
+    let ident = &test.identity;
+    println!(r#"{{ "info": "val", "id": "{ident}", "val": {val_idx}, "bytes": {data:?} }}"#);
+}
+
+
+#[no_mangle]
+pub static mut CALLER_VALS: *mut () = core::ptr::null_mut();
+#[no_mangle]
+pub static mut CALLEE_VALS: *mut () = core::ptr::null_mut();
+#[no_mangle]
+pub static mut SET_FUNC: Option<SetFuncCallback> = None;
+#[no_mangle]
+pub static mut WRITE_VAL: Option<WriteValCallback> = None;
+
+extern {
+    fn do_test();
+}
+
+pub fn main() {
+    unsafe {
+        let mut caller_vals = WriteBuffer::new("caller");
+        let mut callee_vals = WriteBuffer::new("callee");
+        CALLER_VALS = &mut caller_vals as *mut _ as *mut _;
+        CALLEE_VALS = &mut callee_vals as *mut _ as *mut _;
+        SET_FUNC = Some(set_func);
+        WRITE_VAL = Some(write_val);
+
+        do_test();
+        println!(r#"{{ "info": "done" }}"#);
+    }
+}
\ No newline at end of file
diff --git a/include/harness/main.rs b/include/harness/main.rs
index 9bf9047..8a8cea8 100644
--- a/include/harness/main.rs
+++ b/include/harness/main.rs
@@ -8,6 +8,7 @@
 //!
 //! In theory this could be replaced with just making `caller::do_test` into `main`
 //! but this might be a bit easier..?
+
 extern {
     fn do_test();
 }
diff --git a/src/error.rs b/src/error.rs
index 28ac108..723da86 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,6 +1,6 @@
 use miette::Diagnostic;
 
-use crate::harness::test::TestId;
+use crate::{harness::test::TestId, TestBuffer};
 
 #[derive(Debug, thiserror::Error, Diagnostic)]
 pub enum CliParseError {
@@ -132,12 +132,37 @@ pub enum LinkError {
 
 #[derive(Debug, thiserror::Error, Diagnostic)]
 pub enum RunError {
-    #[error("test loading error (dynamic linking failed)\n{0}")]
+    #[error("test loading error (dynamic linking failed)\n  {0}")]
     LoadError(#[from] libloading::Error),
+    #[error("failed to spawn test bin: {bin}\n  {e}")]
+    ExecError {
+        bin: camino::Utf8PathBuf,
+        e: std::io::Error,
+    },
     #[error("test impl didn't call set_func before calling write_val")]
     MissingSetFunc,
     #[error("test impl called write_val on func {func} val {val} twice")]
     DoubleWrite { func: usize, val: usize },
+    #[error(
+        "test impl exited with bad status (crashed?): {status}
+    caller last reported: fn {caller_func} value {caller_val_idx}
+    callee last reported: fn {callee_func} value {callee_val_idx}
+    "
+    )]
+    BadExit {
+        status: std::process::ExitStatus,
+        caller_func_idx: usize,
+        caller_val_idx: usize,
+        caller_func: String,
+        callee_func_idx: usize,
+        callee_val_idx: usize,
+        callee_func: String,
+    },
+    #[error("test impl sent invalid messages to harness (executed some kind of UB?)")]
+    InvalidMessages {
+        caller_funcs: TestBuffer,
+        callee_funcs: TestBuffer,
+    },
 }
 
 fn fmt_bytes(bytes: &[u8]) -> String {
diff --git a/src/files.rs b/src/files.rs
index dee8cb7..f5108b1 100644
--- a/src/files.rs
+++ b/src/files.rs
@@ -15,8 +15,14 @@ pub struct Paths {
     pub runtime_test_input_dir: Option<Utf8PathBuf>,
 }
 impl Paths {
-    pub fn harness_main_file(&self) -> Utf8PathBuf {
-        self.out_dir.join("harness.rs")
+    pub fn harness_dylib_main_file(&self) -> Utf8PathBuf {
+        self.out_dir.join("harness_lib.rs")
+    }
+    pub fn harness_bin_main_file(&self) -> Utf8PathBuf {
+        self.out_dir.join("harness_main.rs")
+    }
+    pub fn freestanding_bin_main_file(&self) -> Utf8PathBuf {
+        self.out_dir.join("main.rs")
     }
 
     /// Delete and recreate the build dir
@@ -27,12 +33,28 @@ impl Paths {
 
         // Initialize harness.rs
         {
-            let harness_file_contents = get_file("harness/harness.rs");
-            let harness_file_path = self.harness_main_file();
+            let harness_file_contents = get_file("harness/harness_lib.rs");
+            let harness_file_path = self.harness_dylib_main_file();
+            let mut file = std::fs::File::create_new(harness_file_path)
+                .expect("failed to create harness_lib.rs");
+            file.write_all(harness_file_contents.as_bytes())
+                .expect("failed to initialize harness_lib.rs");
+        }
+        {
+            let harness_file_contents = get_file("harness/harness_main.rs");
+            let harness_file_path = self.harness_bin_main_file();
+            let mut file = std::fs::File::create_new(harness_file_path)
+                .expect("failed to create harness_main.rs");
+            file.write_all(harness_file_contents.as_bytes())
+                .expect("failed to initialize harness_main.rs");
+        }
+        {
+            let harness_file_contents = get_file("harness/main.rs");
+            let harness_file_path = self.freestanding_bin_main_file();
             let mut file =
-                std::fs::File::create_new(harness_file_path).expect("failed to create harness.rs");
+                std::fs::File::create_new(harness_file_path).expect("failed to create main.rs");
             file.write_all(harness_file_contents.as_bytes())
-                .expect("failed to initialize harness.rs");
+                .expect("failed to initialize main.rs");
         }
 
         // Set up env vars for CC
diff --git a/src/harness/build.rs b/src/harness/build.rs
index 8055bb6..ed70068 100644
--- a/src/harness/build.rs
+++ b/src/harness/build.rs
@@ -14,14 +14,13 @@ impl TestHarness {
         &self,
         key: &TestKey,
         src: &GenerateOutput,
-        out_dir: &Utf8Path,
     ) -> Result<BuildOutput, BuildError> {
         // FIXME: these two could be done concurrently
         let caller_lib = self
-            .build_static_lib(key, CallSide::Caller, &src.caller_src, out_dir)
+            .build_static_lib(key, CallSide::Caller, &src.caller_src)
             .await?;
         let callee_lib = self
-            .build_static_lib(key, CallSide::Callee, &src.callee_src, out_dir)
+            .build_static_lib(key, CallSide::Callee, &src.callee_src)
             .await?;
         Ok(BuildOutput {
             caller_lib,
@@ -34,7 +33,6 @@ impl TestHarness {
         key: &TestKey,
         call_side: CallSide,
         src_path: &Utf8Path,
-        out_dir: &Utf8Path,
     ) -> Result<String, BuildError> {
         let toolchain = self.toolchain_by_test_key(key, call_side);
         let lib_name = self.static_lib_name(key, call_side);
@@ -55,18 +53,18 @@ impl TestHarness {
                     .await
                     .expect("failed to acquire concurrency limit semaphore");
                 info!("compiling   {lib_name}");
-                build_static_lib(src_path, toolchain, call_side, out_dir, &lib_name).await
+                build_static_lib(&self.paths, src_path, toolchain, call_side, &lib_name).await
             })
             .await?
             .clone();
         Ok(real_lib_name)
     }
 
-    pub async fn link_dynamic_lib(
+    #[allow(dead_code)]
+    pub async fn link_dylib(
         &self,
         key: &TestKey,
         build: &BuildOutput,
-        out_dir: &Utf8Path,
     ) -> Result<LinkOutput, LinkError> {
         let _token = self
             .concurrency_limiter
@@ -75,7 +73,27 @@ impl TestHarness {
             .expect("failed to acquire concurrency limit semaphore");
         let dynamic_lib_name = self.dynamic_lib_name(key);
         info!("linking     {dynamic_lib_name}");
-        link_dynamic_lib(build, out_dir, &dynamic_lib_name)
+        build_harness_dylib(&self.paths, build, &dynamic_lib_name)
+    }
+
+    pub async fn link_bin(
+        &self,
+        key: &TestKey,
+        build: &BuildOutput,
+    ) -> Result<LinkOutput, LinkError> {
+        let _token = self
+            .concurrency_limiter
+            .acquire()
+            .await
+            .expect("failed to acquire concurrency limit semaphore");
+        let bin_name = self.bin_name(key);
+        info!("linking     {bin_name}");
+        let bin_main = if let WriteImpl::HarnessCallback = key.options.val_writer {
+            self.paths.harness_bin_main_file()
+        } else {
+            self.paths.freestanding_bin_main_file()
+        };
+        build_harness_main(&self.paths, build, &bin_name, &bin_main)
     }
 
     fn static_lib_name(&self, key: &TestKey, call_side: CallSide) -> String {
@@ -87,35 +105,43 @@ impl TestHarness {
         output.push_str(".dll");
         output
     }
+
+    fn bin_name(&self, key: &TestKey) -> String {
+        let mut output = self.base_id(key, None, "_");
+        if cfg!(target_os = "windows") {
+            output.push_str(".exe");
+        }
+        output
+    }
 }
 
 async fn build_static_lib(
+    paths: &Paths,
     src_path: &Utf8Path,
     toolchain: Arc<dyn Toolchain + Send + Sync>,
     call_side: CallSide,
-    out_dir: &Utf8Path,
     static_lib_name: &str,
 ) -> Result<String, BuildError> {
     let lib_name = match call_side {
-        CallSide::Callee => toolchain.compile_callee(src_path, out_dir, static_lib_name)?,
-        CallSide::Caller => toolchain.compile_caller(src_path, out_dir, static_lib_name)?,
+        CallSide::Callee => toolchain.compile_callee(src_path, &paths.out_dir, static_lib_name)?,
+        CallSide::Caller => toolchain.compile_caller(src_path, &paths.out_dir, static_lib_name)?,
     };
 
     Ok(lib_name)
 }
 
 /// Compile and link the test harness with the two sides of the FFI boundary.
-fn link_dynamic_lib(
+fn build_harness_dylib(
+    paths: &Paths,
     build: &BuildOutput,
-    out_dir: &Utf8Path,
     dynamic_lib_name: &str,
 ) -> Result<LinkOutput, LinkError> {
-    let src = out_dir.join("harness.rs");
-    let output = out_dir.join(dynamic_lib_name);
+    let src = paths.harness_dylib_main_file();
+    let output = paths.out_dir.join(dynamic_lib_name);
     let mut cmd = Command::new("rustc");
     cmd.arg("-v")
         .arg("-L")
-        .arg(out_dir)
+        .arg(&paths.out_dir)
         .arg("-l")
         .arg(&build.caller_lib)
         .arg("-l")
@@ -140,3 +166,40 @@ fn link_dynamic_lib(
         Ok(LinkOutput { test_bin: output })
     }
 }
+
+/// Compile and link the test harness with the two sides of the FFI boundary.
+fn build_harness_main(
+    paths: &Paths,
+    build: &BuildOutput,
+    bin_name: &str,
+    bin_main: &Utf8Path,
+) -> Result<LinkOutput, LinkError> {
+    let output = paths.out_dir.join(bin_name);
+    let mut cmd = Command::new("rustc");
+    cmd.arg("-v")
+        .arg("-L")
+        .arg(&paths.out_dir)
+        .arg("-l")
+        .arg(&build.caller_lib)
+        .arg("-l")
+        .arg(&build.callee_lib)
+        .arg("--crate-type")
+        .arg("bin")
+        .arg("--target")
+        .arg(built_info::TARGET)
+        // .arg("-Csave-temps=y")
+        // .arg("--out-dir")
+        // .arg("target/temp/")
+        .arg("-o")
+        .arg(&output)
+        .arg(bin_main);
+
+    debug!("running: {:?}", cmd);
+    let out = cmd.output()?;
+
+    if !out.status.success() {
+        Err(LinkError::RustLink(out))
+    } else {
+        Ok(LinkOutput { test_bin: output })
+    }
+}
diff --git a/src/harness/mod.rs b/src/harness/mod.rs
index b444f3f..ae8379a 100644
--- a/src/harness/mod.rs
+++ b/src/harness/mod.rs
@@ -131,7 +131,6 @@ impl TestHarness {
     pub async fn do_test(&self, test_key: TestKey, test_rules: TestRules) -> TestRunResults {
         use TestRunMode::*;
 
-        let out_dir = self.paths.out_dir.clone();
         let mut res = TestRunResults::new(test_key, test_rules);
         if res.rules.run <= Skip {
             return res;
@@ -159,7 +158,7 @@ impl TestHarness {
         }
 
         res.ran_to = Build;
-        res.build = Some(self.build_test(&res.key, source, &out_dir).await);
+        res.build = Some(self.build_test(&res.key, source).await);
         let build = match res.build.as_ref().unwrap() {
             Ok(v) => v,
             Err(e) => {
@@ -172,7 +171,7 @@ impl TestHarness {
         }
 
         res.ran_to = Link;
-        res.link = Some(self.link_dynamic_lib(&res.key, build, &out_dir).await);
+        res.link = Some(self.link_bin(&res.key, build).await);
         let link = match res.link.as_ref().unwrap() {
             Ok(v) => v,
             Err(e) => {
@@ -185,7 +184,7 @@ impl TestHarness {
         }
 
         res.ran_to = Run;
-        res.run = Some(self.run_dynamic_test(&res.key, link).await);
+        res.run = Some(self.run_bin_test(&res.key, link).await);
         let run = match res.run.as_ref().unwrap() {
             Ok(v) => v,
             Err(e) => {
diff --git a/src/harness/run.rs b/src/harness/run.rs
index f0e8cc8..cf7df25 100644
--- a/src/harness/run.rs
+++ b/src/harness/run.rs
@@ -1,6 +1,7 @@
 //! The runtime actual types and functions that are injected into
 //! compiled tests.
 
+use serde::Deserialize;
 use serde::Serialize;
 use tracing::info;
 
@@ -9,13 +10,23 @@ use crate::report::*;
 use crate::*;
 
 impl TestHarness {
-    pub async fn run_dynamic_test(
+    #[allow(dead_code)]
+    pub async fn run_dylib_test(
+        &self,
+        _key: &TestKey,
+        linked_test: &LinkOutput,
+    ) -> Result<RunOutput, RunError> {
+        let output = run_dylib_test(linked_test)?;
+        Ok(output)
+    }
+
+    pub async fn run_bin_test(
         &self,
         key: &TestKey,
-        test_dylib: &LinkOutput,
+        linked_test: &LinkOutput,
     ) -> Result<RunOutput, RunError> {
-        let full_test_name = self.full_test_name(key);
-        let output = run_dynamic_test(test_dylib, &full_test_name)?;
+        let test = self.test(&key.test);
+        let output = run_bin_test(test, linked_test)?;
         Ok(output)
     }
 }
@@ -83,6 +94,14 @@ pub unsafe extern "C" fn write_val(
     input: *const u8,
     size: u32,
 ) {
+    write_val_inner(
+        test,
+        val_idx,
+        std::slice::from_raw_parts(input, size as usize),
+    )
+}
+
+fn write_val_inner(test: &mut TestBuffer, val_idx: u32, data: &[u8]) {
     // Get the current function
     let Some(func_idx) = test.cur_func else {
         test.had_missing_set_func = true;
@@ -105,14 +124,13 @@ pub unsafe extern "C" fn write_val(
         test.had_double_writes.push((func_idx, val_idx));
         return;
     }
-    let data = std::slice::from_raw_parts(input, size as usize);
     val.bytes = data.to_vec();
 }
 
 /// Run the test!
 ///
 /// See the README for a high-level description of this design.
-fn run_dynamic_test(test_dylib: &LinkOutput, _full_test_name: &str) -> Result<RunOutput, RunError> {
+fn run_dylib_test(test_dylib: &LinkOutput) -> Result<RunOutput, RunError> {
     // Initialize all the buffers the tests will write to
     let mut caller_vals = TestBuffer::new();
     let mut callee_vals = TestBuffer::new();
@@ -140,3 +158,115 @@ fn run_dynamic_test(test_dylib: &LinkOutput, _full_test_name: &str) -> Result<Ru
         callee_funcs: callee_vals,
     })
 }
+
+/// Run the test!
+///
+/// See the README for a high-level description of this design.
+fn run_bin_test(test: Arc<Test>, test_bin: &LinkOutput) -> Result<RunOutput, RunError> {
+    #[derive(Deserialize, Serialize)]
+    #[serde(rename_all = "kebab-case")]
+    #[serde(tag = "info")]
+    enum HarnessJsonMessage {
+        Func {
+            id: HarnessSide,
+            func: u32,
+        },
+        Val {
+            id: HarnessSide,
+            val: u32,
+            bytes: Vec<u8>,
+        },
+        Done,
+    }
+
+    #[derive(Deserialize, Serialize)]
+    #[serde(rename_all = "kebab-case")]
+    enum HarnessSide {
+        Caller,
+        Callee,
+    }
+
+    // Initialize all the buffers the tests will write to
+    let mut caller_vals = TestBuffer::new();
+    let mut callee_vals = TestBuffer::new();
+    let mut finished_clean = false;
+
+    unsafe {
+        info!(
+            "running     {}",
+            test_bin.test_bin.file_name().unwrap_or_default()
+        );
+        // Load the dylib of the test, and get its test_start symbol
+
+        debug!("loading     {}", &test_bin.test_bin);
+        let mut cmd = Command::new(&test_bin.test_bin);
+        let output = cmd.output().map_err(|e| RunError::ExecError {
+            bin: test_bin.test_bin.clone(),
+            e,
+        })?;
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        for line in stdout.lines() {
+            let Ok(message): Result<HarnessJsonMessage, _> = serde_json::from_str(line) else {
+                finished_clean = false;
+                continue;
+            };
+            match message {
+                HarnessJsonMessage::Func { id, func } => {
+                    let buf = match id {
+                        HarnessSide::Caller => &mut caller_vals,
+                        HarnessSide::Callee => &mut callee_vals,
+                    };
+                    set_func(buf, func)
+                }
+                HarnessJsonMessage::Val { id, val, bytes } => {
+                    let buf = match id {
+                        HarnessSide::Caller => &mut caller_vals,
+                        HarnessSide::Callee => &mut callee_vals,
+                    };
+                    write_val_inner(buf, val, &bytes)
+                }
+                HarnessJsonMessage::Done => {
+                    finished_clean = true;
+                }
+            }
+        }
+        if !output.status.success() {
+            let (caller_func_idx, caller_val_idx, caller_func) = best_vals(&test, &caller_vals);
+            let (callee_func_idx, callee_val_idx, callee_func) = best_vals(&test, &callee_vals);
+            return Err(RunError::BadExit {
+                status: output.status,
+                caller_func_idx,
+                caller_val_idx,
+                caller_func,
+                callee_func_idx,
+                callee_val_idx,
+                callee_func,
+            });
+        }
+    }
+
+    if !finished_clean {
+        return Err(RunError::InvalidMessages {
+            caller_funcs: caller_vals,
+            callee_funcs: callee_vals,
+        });
+    }
+
+    caller_vals.finish_tests()?;
+    callee_vals.finish_tests()?;
+
+    Ok(RunOutput {
+        caller_funcs: caller_vals,
+        callee_funcs: callee_vals,
+    })
+}
+
+fn best_vals(test: &Test, vals: &TestBuffer) -> (usize, usize, String) {
+    let default_funcs = FuncBuffer::default();
+    let func_idx = vals.cur_func.unwrap_or(0);
+    let funcs = vals.funcs.get(func_idx).unwrap_or(&default_funcs);
+    let val_idx = funcs.vals.len().saturating_sub(1);
+    let func_name = test.types.realize_func(func_idx).name.to_string();
+
+    (func_idx, val_idx, func_name)
+}


Test patch: