Make it easier to find aml_tester

`uacpi_test_adapter` will now try three different ways to find
`aml_tester`:

1. From the `AML_TESTER_PATH` environment variable,
2. From the system path,
3. As a sibling executable to the running one.

Also, in `aml_test_tools`, remove the memory leak.

Author: martin-hughes

tests/README.md

@@ -41,9 +41,10 @@ suite as well as our own. This can be done by:
 2. Navigating to the root of that repo.
 3. Running something like: 
    ```shell
-   AML_TESTER_PATH=../acpi/target/debug/aml_tester python3 tests/run_tests.py --test-runner ../acpi/target/debug/uacpi_test_adapter`
+   python3 tests/run_tests.py --test-runner ../acpi/target/debug/uacpi_test_adapter
    ```
-   Adjusting the paths as necessary. (And editing as needed for PowerShell)
+   Adjusting the paths as necessary. See the `uacpi_test_adapter` documentation for details on how it finds
+   `aml_tester`.
 
 Note that, at present, several of the uACPI tests run indefinitely. You may want to skip these! The easiest way to do so
 is probably just to delete them... The relevant tests are:

tools/aml_test_tools/src/lib.rs

@@ -1,3 +1,4 @@
+#![feature(sync_unsafe_cell)]
 //! A collection of helper utilities for testing AML using the [`acpi`] crate.
 //!
 //! These utilities are very heavily based on the way the [`acpi`] crate has used them historically.
@@ -20,6 +21,7 @@ use acpi::{
 };
 use log::{error, trace};
 use std::{
+    cell::SyncUnsafeCell,
     ffi::OsStr,
     fmt::Debug,
     fs::File,
@@ -198,6 +200,11 @@ pub fn resolve_and_compile(path: &PathBuf, can_compile: bool) -> CompilationOutc
 ///
 /// * `handler`: The Handler to be called by the interpreter when needed. This crate includes some
 ///   example [handlers].
+///
+/// Thread safety:
+///
+/// This function uses a single, static, FACS for all tests. If tests are run in parallel, this
+/// means they will share a single global lock.
 pub fn new_interpreter<T>(handler: T) -> Interpreter<T>
 where
     T: Handler + Clone,
@@ -238,7 +245,7 @@ where
         },
     });
 
-    let fake_facs = Box::new(Facs {
+    static FAKE_FACS: SyncUnsafeCell<Facs> = SyncUnsafeCell::new(Facs {
         signature: Signature::FACS,
         length: size_of::<Facs>() as u32,
         hardware_signature: 0,
@@ -258,7 +265,7 @@ where
     // It'd be possible to tidy this up creating a newtype containing both the Interpeter and FACS
     // which manually drops the FACS when appropriate, but since that makes the test interfaces
     // less ergonomic, this is left until the leak is an actual problem.
-    let fake_facs_ptr = Box::leak(fake_facs) as *mut Facs;
+    let fake_facs_ptr = FAKE_FACS.get();
 
     // This PhysicalMapping is dropped when the interpreter is dropped, and if you use logging in
     // the handler object you'll see a call to Handler::unmap_physical_region without any

tools/uacpi_test_adapter/Cargo.toml

@@ -6,6 +6,9 @@ authors = ["Martin Hughes"]
 repository = "https://github.com/rust-osdev/acpi"
 description = "uACPI to aml_tester adapter"
 categories = ["hardware-support"]
-readme = "../README.md"
+readme = "../../README.md"
 license = "MIT/Apache-2.0"
 edition = "2024"
+
+[dependencies]
+which = "8.0.2"

tools/uacpi_test_adapter/src/main.rs

@@ -9,7 +9,12 @@
 //!   complicate `aml_tester`.
 //!
 //! Usage:
-//! * Set the environment variable `AML_TESTER_PATH` to the path of the `aml_tester` binary.
+//! * Make sure `aml_tester` can be found. The following methods are tried, in this order of
+//!   precedence:
+//!   * Set the environment variable `AML_TESTER_PATH` to the path and filename of the `aml_tester`
+//!     binary, or
+//!   * Make sure `aml_tester` is in the system PATH, or
+//!   * Ensure `aml_tester` is in the same folder as `uacpi_test_adapter`.
 //! * Run the uACPI test suite but setting `uacpi_test_adapter` to the test runner.
 //!
 //! e.g.: from the uACPI root directory:
@@ -25,8 +30,11 @@
 //! results.
 use std::{
     env,
+    ffi::OsString,
+    path::PathBuf,
     process::{Command, ExitCode},
 };
+use which::which;
 
 fn main() -> ExitCode {
     // We don't support the resource tests, so just claim success to allow the main test suite to
@@ -35,15 +43,15 @@ fn main() -> ExitCode {
         return ExitCode::SUCCESS;
     }
 
-    let Some(tester_path) = env::var_os("AML_TESTER_PATH") else {
-        eprintln!("AML_TESTER_PATH is not set");
+    let Some(tester_path) = get_aml_tester_path() else {
+        eprintln!("aml_tester not found. Try setting AML_TESTER_PATH to the path of the aml_tester binary.");
         return ExitCode::FAILURE;
     };
 
     let status = match Command::new(tester_path).args(env::args_os().skip(1)).status() {
         Ok(status) => status,
         Err(err) => {
-            eprintln!("Failed to execute AML_TESTER_PATH: {err}");
+            eprintln!("Failed to execute aml_tester: {err}");
             return ExitCode::FAILURE;
         }
     };
@@ -53,3 +61,55 @@ fn main() -> ExitCode {
         None => ExitCode::FAILURE,
     }
 }
+
+/// Find the path to the `aml_tester` binary.
+///
+/// Uses the search order given in this executable's main documentation.
+fn get_aml_tester_path() -> Option<OsString> {
+    [get_aml_tester_from_env, get_aml_tester_from_path_env, get_aml_tester_from_binary_path]
+        .iter()
+        .find_map(|f| f())
+}
+
+/// If the environment variable `AML_TESTER_PATH` is set, use that. Assume it is correct without
+/// checking.
+fn get_aml_tester_from_env() -> Option<OsString> {
+    env::var_os("AML_TESTER_PATH")
+}
+
+/// If `aml_tester` is in the system PATH, use that.
+fn get_aml_tester_from_path_env() -> Option<OsString> {
+    which("aml_tester").ok().map(|path| path.into())
+}
+
+/// If `aml_tester` exists alongside this executable, use that.
+fn get_aml_tester_from_binary_path() -> Option<OsString> {
+    // This says: "change the name of the current executable to `aml_tester` and see if that exists."
+    env::current_exe().ok().map(change_exec_name).and_then(|path| which(path).ok()).map(|path| path.into())
+}
+
+/// Replace the filename of the given path with `aml_tester`. Preserve the extension so that
+/// Windows won't have problems.
+fn change_exec_name(mut path: PathBuf) -> PathBuf {
+    // The final `unwrap` on the following line is reasonable because if the UTF-8 conversion fails,
+    // the filename is probably invalid, so we don't really want to keep trying to use it!
+    let extension = String::from(path.extension().unwrap_or_default().to_str().unwrap());
+    path.set_file_name("aml_tester");
+    path.set_extension(extension);
+    path
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_filename_replacement() {
+        const PATH: &str = "/a/b/c/uacpi_test_adapter.ext";
+        let original_path = PathBuf::from(PATH);
+        let expected_path = PathBuf::from("/a/b/c/aml_tester.ext");
+        let result_path = change_exec_name(original_path);
+
+        assert_eq!(result_path, expected_path);
+    }
+}