Merge tock#339

339: Implement Exit in `libtock_unittest`, and add functionality for testing Exit calls. r=hudson-ayers a=jrvanwhy

Calling Exit on a fake kernel will cause the test process to exit.

I wanted to make Exit unwind, but that would make it impossible to test error handling in Allow implementations. In particular, the critical `Drop` implementation in tock#338 cannot be tested if Exit unwinds, as unwinding past the destructor would immediately cause undefined behavior.

In order to make code that calls Exit testable, I added an `exit_test` function (in the `exit_test` module). `exit_test` spawns a new process to run test code that should exit, and verifies the test code does call Exit.

`exit_test` is not usable under Miri, so I added a non-Miri test invocation to `make test`. This caused two issues:

1. It started executing doc tests, and some of our doc tests are broken. I ignored the doc tests for now.
2. Some sort of interaction occurred between `cargo test` and `cargo miri` that caused errors related to incorrect or outdated JSON. These occurred in procedural macro crates, which Miri doesn't support, so I disabled Miri on those crates.

Co-authored-by: Johnathan Van Why <[email protected]>

Makefile

@@ -89,6 +89,12 @@ examples:
 # libtock_runtime only supports running on Tock.
 EXCLUDE_RUNTIME := --exclude libtock2 --exclude libtock_runtime
 
+# Arguments to pass to cargo to exclude crates that cannot be tested by Miri. In
+# addition to excluding libtock_runtime, Miri also cannot test proc macro crates
+# (and in fact will generate broken data that causes cargo test to fail).
+EXCLUDE_MIRI := $(EXCLUDE_RUNTIME) --exclude libtock_codegen \
+                --exclude ufmt-macros
+
 # Arguments to pass to cargo to exclude `std` and crates that depend on it. Used
 # when we build a crate for an embedded target, as those targets lack `std`.
 EXCLUDE_STD := --exclude libtock_unittest --exclude print_sizes \
@@ -105,6 +111,7 @@ test-stable:
 
 .PHONY: test
 test: examples test-qemu-hifive test-stable
+	PLATFORM=nrf52 cargo test $(EXCLUDE_RUNTIME) --workspace
 	# TODO: When we have a working embedded test harness, change the libtock2
 	# builds to --all-targets rather than --examples.
 	# Build libtock2 on both a RISC-V target and an ARM target. We pick
@@ -120,9 +127,9 @@ test: examples test-qemu-hifive test-stable
 	# currently pass clippy on ARM.
 	LIBTOCK_PLATFORM=hifive1 PLATFORM=hifive1 cargo clippy $(EXCLUDE_STD) \
 		--target=riscv32imac-unknown-none-elf --workspace
-	PLATFORM=nrf52 cargo miri test $(EXCLUDE_RUNTIME) --workspace
+	PLATFORM=nrf52 cargo miri test $(EXCLUDE_MIRI) --workspace
 	MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-track-raw-pointers" \
-		PLATFORM=nrf52 cargo miri test $(EXCLUDE_RUNTIME) --workspace
+		PLATFORM=nrf52 cargo miri test $(EXCLUDE_MIRI) --workspace
 	echo '[ SUCCESS ] libtock-rs tests pass'
 
 .PHONY: analyse-stack-sizes

core/src/stack_size.rs

@@ -1,6 +1,6 @@
 //! Executables must specify their stack size by using the `stack_size!` macro.
 //! It takes a single argument, the desired stack size in bytes. Example:
-//! ```
+//! ```ignore
 //! stack_size!{0x400}
 //! ```
 

ufmt/src/lib.rs

@@ -40,7 +40,7 @@
 //!
 //! - `uwrite!` / `uwriteln!`
 //!
-//! ```
+//! ```ignore
 //! use ufmt::{derive::uDebug, uwrite};
 //!
 //! #[derive(uDebug)]

unittest/src/exit_test/mod.rs

@@ -0,0 +1,211 @@
+//! Tools for testing code that calls the Exit system call.
+//!
+//! This module is not compatible with Miri because it requires the ability to
+//! spawn external processes, which Miri does not support by default. Therefore
+//! it is only available for non-Miri tests.
+
+#[cfg(test)]
+mod tests;
+
+use std::panic::{catch_unwind, Location, UnwindSafe};
+
+/// Utility for testing code that is expected to call the Exit system call. It
+/// is used as follows (inside a unit test case):
+///
+/// ```
+/// use libtock_platform::Syscalls;
+/// let _kernel = libtock_unittest::fake::Kernel::new();
+/// let exit = libtock_unittest::exit_test("tests::foo", || {
+///     libtock_unittest::fake::Syscalls::exit_terminate(0);
+/// });
+/// assert_eq!(exit, libtock_unittest::ExitCall::Terminate(0));
+/// ```
+///
+/// `exit_test` will panic (to fail the test case) if the code does not call
+/// Exit, or if the parameters to exit do not match `expected_exit`.
+///
+/// `test_name` must match the name of the test case, as is used in Rust's test
+/// framework's filter syntax.
+///
+/// `exit_test` is a hack, and the user should understand how it works to
+/// understand its limitations. When the above test case is executed, the
+/// following happens:
+///
+/// 1. The first test process (the one started by the user, e.g. through
+///    `cargo test`) executes the `foo()` test case, which calls `exit_test`.
+///    We'll call this process A, as it was the first test process to start.
+/// 2. `exit_test` spawns a second process, B, by invoking the same test binary
+///    as process A. When it does, it passes a filter to process B telling it
+///    to only invoke `foo()` (this is the purpose of the `test_name` argument).
+///    It also sets an environment variable telling process B that `exit_test`
+///    launched it.
+/// 3. Process B runs the `foo()` test case, which invokes `exit_test` a second
+///    time.
+/// 4. `exit_test` in process B uses the environment variable to detect that it
+///    is the subprocess version, and it runs closure `fcn`. If `fcn` does not
+///    call Exit, it panics. `exit_test` will not return from process B.
+/// 5. `exit_test` in process A waits until process B terminates.
+/// 6. `exit_test` in process A reads the output of process B to determine
+///    whether Exit was called, and if so what arguments were called.
+/// 7. `exit_test` in process A returns a value indicating what happened in
+///    process B, which `foo()` can make assertions on.
+#[track_caller]
+pub fn exit_test<F: FnOnce() + UnwindSafe>(test_name: &str, fcn: F) -> ExitCall {
+    if let Some(signal_var) = std::env::var_os(SIGNAL_VAR) {
+        // We are process B, run the test function.
+        run_test(signal_var, fcn)
+    } else {
+        // We are process A, spawn process B.
+        spawn_test(test_name)
+    }
+}
+
+/// Indicates what type of Exit call was performed, and what completion code was
+/// provided.
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum ExitCall {
+    Terminate(u32),
+    Restart(u32),
+}
+
+// -----------------------------------------------------------------------------
+// Public API above, implementation details below.
+// -----------------------------------------------------------------------------
+
+// Prints a message telling exit_test the Exit system call was called.
+pub(crate) fn signal_exit(exit_call: ExitCall) {
+    signal_message(ExitMessage::ExitCall(exit_call));
+}
+
+#[doc(hidden)]
+impl std::fmt::Display for ExitCall {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
+        match self {
+            ExitCall::Terminate(code) => write!(f, "exit-terminate({})", code),
+            ExitCall::Restart(code) => write!(f, "exit-restart({})", code),
+        }
+    }
+}
+
+#[doc(hidden)]
+impl std::str::FromStr for ExitCall {
+    type Err = ParseExitError;
+
+    fn from_str(s: &str) -> Result<ExitCall, ParseExitError> {
+        // Strip off the trailing ), leaving the name and (
+        let s = s.strip_suffix(')').ok_or(ParseExitError)?;
+
+        if let Some(s) = s.strip_prefix("exit-terminate(") {
+            Ok(ExitCall::Terminate(s.parse().or(Err(ParseExitError))?))
+        } else if let Some(s) = s.strip_prefix("exit-restart(") {
+            Ok(ExitCall::Restart(s.parse().or(Err(ParseExitError))?))
+        } else {
+            Err(ParseExitError)
+        }
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+#[doc(hidden)]
+pub struct ParseExitError;
+
+// The name of the environment variable used by process A to tell process B that
+// it is process B. The value of the environment variable is the location where
+// exit_test was called (this location is used to help verify that test_name is
+// correct).
+const SIGNAL_VAR: &str = "LIBTOCK_UNITTEST_EXIT_TEST";
+
+// This string is printed by process B to tell process A how it exited. It is
+// followed by the Display string for a ExitMessage.
+const EXIT_STRING: &str = "LIBTOCK_UNITTEST_EXIT_TEST_RESULT: ";
+
+#[derive(Debug, PartialEq)]
+enum ExitMessage {
+    ExitCall(ExitCall),
+    WrongCase,
+    DidNotExit,
+}
+
+impl std::fmt::Display for ExitMessage {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
+        match self {
+            ExitMessage::ExitCall(exit_call) => write!(f, "ExitCall({})", exit_call),
+            ExitMessage::WrongCase => write!(f, "WrongCase"),
+            ExitMessage::DidNotExit => write!(f, "DidNotExit"),
+        }
+    }
+}
+
+impl std::str::FromStr for ExitMessage {
+    type Err = ParseExitError;
+
+    fn from_str(s: &str) -> Result<ExitMessage, ParseExitError> {
+        if let Some(s) = s.strip_prefix("ExitCall(") {
+            let s = s.strip_suffix(')').ok_or(ParseExitError)?;
+            Ok(ExitMessage::ExitCall(s.parse()?))
+        } else if s == "WrongCase" {
+            Ok(ExitMessage::WrongCase)
+        } else if s == "DidNotExit" {
+            Ok(ExitMessage::DidNotExit)
+        } else {
+            Err(ParseExitError)
+        }
+    }
+}
+
+// Implements process A's behavior for exit_test: spawns this test again as a
+// subprocess, only executing the test specified by test_name.
+#[track_caller]
+fn spawn_test(test_name: &str) -> ExitCall {
+    let current_exe = std::env::current_exe().expect("Unable to find test executable");
+    let output = std::process::Command::new(current_exe)
+        .args(std::env::args_os())
+        .arg("--nocapture")
+        .arg("--exact")
+        .arg(test_name)
+        .envs(std::env::vars_os())
+        .env(SIGNAL_VAR, format!("{}", Location::caller()))
+        .output()
+        .expect("Subprocess exec failed");
+    let stdout = String::from_utf8(output.stdout).expect("Subprocess produced invalid UTF-8");
+    println!("{} subprocess stdout:\n{}", test_name, stdout);
+    let stderr = String::from_utf8(output.stderr).expect("Subprocess produced invalid UTF-8");
+    println!("{} subprocess stderr:\n{}", test_name, stderr);
+
+    // Search for the exit message in stdout.
+    for line in stdout.lines() {
+        if let Some(message) = line.strip_prefix(EXIT_STRING) {
+            match message
+                .parse::<ExitMessage>()
+                .expect("Failed to parse exit message")
+            {
+                ExitMessage::ExitCall(exit_call) => return exit_call,
+                ExitMessage::WrongCase => panic!(
+                    "Subprocess executed the wrong test case. Perhaps test_name is incorrect?"
+                ),
+                ExitMessage::DidNotExit => panic!("Subprocess did not call Exit."),
+            }
+        }
+    }
+    panic!("Subprocess did not indicate why it exited. Perhaps test_name is incorrect?");
+}
+
+// Used by process B to send a message to process A.
+fn signal_message(message: ExitMessage) {
+    println!("{}{}", EXIT_STRING, message);
+}
+
+// Implements process B's behavior for exit_test. Verifies the test case was
+// specified correctly, runs the test function, and prints an error if the test
+// function did not call Exit.
+#[track_caller]
+fn run_test<F: FnOnce() + UnwindSafe>(signal_var: std::ffi::OsString, fcn: F) -> ! {
+    let signal_var = signal_var.to_str().expect("Invalid signal variable value");
+    if format!("{}", Location::caller()) != signal_var {
+        signal_message(ExitMessage::WrongCase);
+        std::process::exit(1);
+    }
+    println!("exit_test: closure return value {:?}", catch_unwind(fcn));
+    signal_message(ExitMessage::DidNotExit);
+    std::process::exit(1);
+}

unittest/src/exit_test/tests.rs

@@ -0,0 +1,94 @@
+use super::*;
+
+#[test]
+fn exitcall_display() {
+    assert_eq!(format!("{}", ExitCall::Terminate(3)), "exit-terminate(3)");
+    assert_eq!(format!("{}", ExitCall::Restart(14)), "exit-restart(14)");
+}
+
+#[test]
+fn exitcall_parse() {
+    assert_eq!("exit-terminate(3)".parse(), Ok(ExitCall::Terminate(3)));
+    assert_eq!("exit-restart(14)".parse(), Ok(ExitCall::Restart(14)));
+    assert_eq!("exit-unknown(3)".parse::<ExitCall>(), Err(ParseExitError));
+    assert_eq!(
+        "exit-restart(not-an-int)".parse::<ExitCall>(),
+        Err(ParseExitError)
+    );
+    assert_eq!("no-parens".parse::<ExitCall>(), Err(ParseExitError));
+    assert_eq!("".parse::<ExitCall>(), Err(ParseExitError));
+}
+
+#[test]
+fn exitmessage_display() {
+    assert_eq!(
+        format!("{}", ExitMessage::ExitCall(ExitCall::Restart(1))),
+        "ExitCall(exit-restart(1))"
+    );
+    assert_eq!(format!("{}", ExitMessage::WrongCase), "WrongCase");
+    assert_eq!(format!("{}", ExitMessage::DidNotExit), "DidNotExit");
+}
+
+#[test]
+fn exitmessage_parse() {
+    assert_eq!("".parse::<ExitMessage>(), Err(ParseExitError));
+    assert_eq!("ExitCall()".parse::<ExitMessage>(), Err(ParseExitError));
+    assert_eq!(
+        "ExitCall(error)".parse::<ExitMessage>(),
+        Err(ParseExitError)
+    );
+    assert_eq!(
+        "ExitCall(exit-restart(5))".parse::<ExitMessage>(),
+        Ok(ExitMessage::ExitCall(ExitCall::Restart(5)))
+    );
+    assert_eq!(
+        "WrongCase".parse::<ExitMessage>(),
+        Ok(ExitMessage::WrongCase)
+    );
+    assert_eq!(
+        "DidNotExit".parse::<ExitMessage>(),
+        Ok(ExitMessage::DidNotExit)
+    );
+}
+
+#[should_panic(expected = "did not call Exit")]
+#[test]
+fn exit_test_did_not_exit() {
+    exit_test("exit_test::tests::exit_test_did_not_exit", || {});
+}
+
+#[should_panic(expected = "did not indicate why it exited")]
+#[test]
+fn exit_test_did_not_signal() {
+    exit_test("exit_test::tests::exit_test_did_not_signal", || {
+        std::process::exit(1)
+    });
+}
+
+#[test]
+fn exit_test_signal_terminate() {
+    let result = exit_test("exit_test::tests::exit_test_signal_terminate", || {
+        signal_exit(ExitCall::Terminate(159));
+        std::process::exit(1);
+    });
+    assert_eq!(result, ExitCall::Terminate(159));
+}
+
+#[test]
+fn exit_test_signal_restart() {
+    let result = exit_test("exit_test::tests::exit_test_signal_restart", || {
+        signal_exit(ExitCall::Restart(0));
+        std::process::exit(1);
+    });
+    assert_eq!(result, ExitCall::Restart(0));
+}
+
+#[should_panic(expected = "executed the wrong test case")]
+#[test]
+fn exit_test_wrong_case() {
+    // Intentionally-incorrect test case name.
+    exit_test("exit_test::tests::exit_test_signal_restart", || {
+        signal_exit(ExitCall::Restart(0));
+        std::process::exit(1);
+    });
+}

unittest/src/fake/syscalls/exit_impl.rs

@@ -0,0 +1,25 @@
+use core::convert::TryInto;
+
+pub(super) fn exit(r0: libtock_platform::Register, r1: libtock_platform::Register) -> ! {
+    let exit_number: u32 = r0.try_into().expect("Too large exit number");
+    let completion_code: u32 = r1.try_into().expect("Too large completion code");
+    match exit_number {
+        libtock_platform::exit_id::TERMINATE => {
+            println!("exit-terminate called with code {}", completion_code);
+
+            #[cfg(not(miri))]
+            crate::exit_test::signal_exit(crate::ExitCall::Terminate(completion_code));
+
+            std::process::exit(1);
+        }
+        libtock_platform::exit_id::RESTART => {
+            println!("exit-restart called with code {}", completion_code);
+
+            #[cfg(not(miri))]
+            crate::exit_test::signal_exit(crate::ExitCall::Restart(completion_code));
+
+            std::process::exit(1);
+        }
+        _ => panic!("Unknown exit number {} invoked.", exit_number),
+    }
+}