From b5b2dda0304c6d10daaad4c5622b589a02eddf52 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Mon, 16 Mar 2026 12:20:59 +0000 Subject: [PATCH 1/4] Allow `aml_tester` to continue after a panic Fix accidentally broken tests Exit with a failure code if needed This makes it easier for any user to see if a test has failed. Which may be useful for example in CI environments. Remove AML filename replacement `resolve_and_compile` relies on the current `iasl` way of naming its output, there's no need to duplicate that in `create_script_file` --- tests/normal_fields.rs | 6 +- tests/test_infra/mod.rs | 14 ++- tools/aml_test_tools/src/lib.rs | 216 ++++++++++++++++++++++---------- tools/aml_tester/src/main.rs | 119 ++++++++++++------ 4 files changed, 243 insertions(+), 112 deletions(-) diff --git a/tests/normal_fields.rs b/tests/normal_fields.rs index e1a88754..f486731b 100644 --- a/tests/normal_fields.rs +++ b/tests/normal_fields.rs @@ -16,7 +16,7 @@ mod test_infra; #[test] fn test_basic_store_and_load() { - const AML: &str = r#"DefinitionBlock("%FN%", "DSDT", 1, "RSACPI", "BUFFLD", 1) { + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "BUFFLD", 1) { OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) Field(MEM, WordAcc, NoLock, Preserve) { A, 16, @@ -46,7 +46,7 @@ fn test_basic_store_and_load() { #[test] fn test_narrow_access_store_and_load() { - const AML: &str = r#"DefinitionBlock("%FN%", "DSDT", 1, "RSACPI", "BUFFLD", 1) { + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "BUFFLD", 1) { OperationRegion(MEM, SystemIO, 0x40, 0x10) Field(MEM, ByteAcc, NoLock, Preserve) { A, 16, @@ -79,7 +79,7 @@ fn test_narrow_access_store_and_load() { #[test] fn test_unaligned_field_store() { - const AML: &str = r#"DefinitionBlock("%FN%", "DSDT", 1, "RSACPI", "BUFFLD", 1) { + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "BUFFLD", 1) { OperationRegion(MEM, SystemIO, 0x40, 0x10) Field(MEM, WordAcc, NoLock, Preserve) { A, 7, diff --git a/tests/test_infra/mod.rs b/tests/test_infra/mod.rs index 31c5ff88..baf45585 100644 --- a/tests/test_infra/mod.rs +++ b/tests/test_infra/mod.rs @@ -1,13 +1,17 @@ use acpi::Handler; -use aml_test_tools::{new_interpreter, run_test_for_string, TestResult}; -use aml_test_tools::handlers::logging_handler::LoggingHandler; +use aml_test_tools::{ + RunTestResult, + handlers::logging_handler::LoggingHandler, + new_interpreter, + run_test_for_string, +}; pub fn run_aml_test(asl: &'static str, handler: impl Handler) { // Tests calling `run_aml_test` don't do much else, and we usually want logging, so initialize it here. let _ = pretty_env_logger::try_init(); - + let logged_handler = LoggingHandler::new(handler); - let mut interpreter = new_interpreter(logged_handler); + let interpreter = new_interpreter(logged_handler); - assert_eq!(run_test_for_string(asl, &mut interpreter), TestResult::Pass); + assert!(matches!(run_test_for_string(asl, interpreter), RunTestResult::Pass(_))); } diff --git a/tools/aml_test_tools/src/lib.rs b/tools/aml_test_tools/src/lib.rs index 33d2b0a9..130630e8 100644 --- a/tools/aml_test_tools/src/lib.rs +++ b/tools/aml_test_tools/src/lib.rs @@ -14,8 +14,10 @@ use acpi::{ use log::{error, trace}; use std::{ ffi::OsStr, + fmt::Debug, fs::File, io::{Read, Write}, + panic::{AssertUnwindSafe, catch_unwind}, path::PathBuf, process::Command, ptr::NonNull, @@ -24,10 +26,59 @@ use std::{ }; use tempfile::{NamedTempFile, TempDir, tempdir}; +/// The result of a call to [`run_test`]. This is a bit of a combination - it contains the actual +/// result of the test, but also the interpreter - if it is still valid. +pub enum RunTestResult +where + T: Handler, +{ + /// The test passed, and the interpreter is still valid. + Pass(Interpreter), + + /// The test failed, but the interpreter is still valid. A failure reason is also provided. + Failed(Interpreter, TestFailureReason), + + /// The test failed, and the interpreter is no longer valid. + Panicked, +} + +/// The result of a test +#[derive(Debug, PartialEq)] +pub enum TestResult { + /// The test passed. + Pass, + + /// The test failed. + Failed(TestFailureReason), + + /// The test caused a panic, probably in the interpreter. This is separated out from the failed + /// case for two reasons: + /// + /// 1. We want to highlight panics as they are in some ways worse than a failure - some AML + /// might cause the problems for the interpreter, but the system as a whole shouldn't crash + /// 2. The interpreter that was being used for testing is no longer valid - a new one is needed. + Panicked, +} + +impl From<&RunTestResult> for TestResult +where + T: Handler, +{ + fn from(result: &RunTestResult) -> Self { + match result { + RunTestResult::Pass(_) => TestResult::Pass, + RunTestResult::Failed(_, reason) => TestResult::Failed(reason.clone()), + RunTestResult::Panicked => TestResult::Panicked, + } + } +} + /// Possible results of [`resolve_and_compile`]. pub enum CompilationOutcome { /// Not a valid ASL or AML file. Ignored, + /// A filesystem error occurred. + FilesystemErr(PathBuf), /// This file is already an AML file, does not need recompiling. IsAml(PathBuf), /// Both .asl and .aml files exist, and the .aml file is newer - it does not need recompiling. @@ -41,17 +92,15 @@ pub enum CompilationOutcome { Succeeded(PathBuf), } -/// Possible results of [`run_test_for_file`]. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -pub enum TestResult { - /// The test passed. - Pass, +/// Possible causes of a test failure. +#[derive(Clone, Debug, /*Eq, Hash,*/ PartialEq)] +pub enum TestFailureReason { /// The test ASL failed compilation by `iasl`. CompileFail, - /// Our interpreter failed to parse the resulting AML. - ParseFail, - // TODO: should we do this?? - NotCompiled, + /// Some error occurred attempting to read or write the test file. + FilesystemErr, + /// Our interpreter failed to parse or execute the resulting AML. + ParseFail(AmlError), } /// An internal-only struct that helps keep track of temporary files generated by @@ -62,7 +111,6 @@ struct TempScriptFile { #[allow(unused)] dir: TempDir, asl_file: NamedTempFile, - aml_file: PathBuf, } /// Determine what to do with this file - ignore, compile and parse, or just parse. @@ -73,47 +121,62 @@ struct TempScriptFile { /// - `path`: The filename of an ASL file to (possibly) compile. /// - `can_compile`: Whether compilation is allowed for this file. If not, any existing AML file /// with the same name (but different extension) will be used. -pub fn resolve_and_compile(path: &PathBuf, can_compile: bool) -> std::io::Result { +pub fn resolve_and_compile(path: &PathBuf, can_compile: bool) -> CompilationOutcome { + let Ok(meta) = path.metadata() else { + return CompilationOutcome::FilesystemErr(path.clone()); + }; + // If this file is aml and it exists, it's ready for parsing // metadata() will error if the file does not exist - if path.extension() == Some(OsStr::new("aml")) && path.metadata()?.is_file() { - return Ok(CompilationOutcome::IsAml(path.clone())); + if path.extension() == Some(OsStr::new("aml")) && meta.is_file() { + return CompilationOutcome::IsAml(path.clone()); } // If this file is not asl, it's not interesting. Error if the file does not exist. - if path.extension() != Some(OsStr::new("asl")) || !path.metadata()?.is_file() { - return Ok(CompilationOutcome::Ignored); + if path.extension() != Some(OsStr::new("asl")) || !meta.is_file() { + return CompilationOutcome::Ignored; } let aml_path = path.with_extension("aml"); - if aml_path.is_file() { - let asl_last_modified = path.metadata()?.modified()?; - let aml_last_modified = aml_path.metadata()?.modified()?; + let Ok(asl_last_modified) = meta.modified() else { + return CompilationOutcome::FilesystemErr(path.clone()); + }; + let Ok(aml_last_modified) = aml_path.metadata().and_then(|m| m.modified()) else { + return CompilationOutcome::FilesystemErr(path.clone()); + }; // If the aml is more recent than the asl, use the existing aml // Otherwise continue to compilation if asl_last_modified <= aml_last_modified { - return Ok(CompilationOutcome::Newer(aml_path)); + return CompilationOutcome::Newer(aml_path); } } if !can_compile { - return Ok(CompilationOutcome::NotCompiled(path.clone())); + return CompilationOutcome::NotCompiled(path.clone()); } // Compile the ASL file using `iasl` println!("Compiling file: {}", path.display()); - let output = Command::new("iasl").arg(path).output()?; + let output = Command::new("iasl").arg(path).output(); - if !output.status.success() { - println!( - "Failed to compile ASL file: {}. Output from iasl:\n {}", - path.display(), - String::from_utf8_lossy(&output.stderr) - ); - Ok(CompilationOutcome::Failed(path.clone())) - } else { - Ok(CompilationOutcome::Succeeded(aml_path)) + match output { + Ok(output) => { + if !output.status.success() { + println!( + "Failed to compile ASL file: {}. Output from iasl:\n {}", + path.display(), + String::from_utf8_lossy(&output.stderr) + ); + CompilationOutcome::Failed(path.clone()) + } else { + CompilationOutcome::Succeeded(aml_path) + } + } + Err(e) => { + println!("Failed to compile ASL file: {}. Error: {}", path.display(), e); + CompilationOutcome::Failed(path.clone()) + } } } @@ -185,11 +248,17 @@ where /// * `asl`: A string slice containing an ASL script. This will be compiled to AML using `iasl` and /// then tested using [`run_test`] /// * `interpreter`: The interpreter to use for testing. -pub fn run_test_for_string(asl: &'static str, interpreter: &mut Interpreter) -> TestResult { +pub fn run_test_for_string(asl: &'static str, interpreter: Interpreter) -> RunTestResult +where + T: Handler, +{ let script = create_script_file(asl); - resolve_and_compile(&script.asl_file.path().to_path_buf(), true).unwrap(); - - run_test_for_file(&script.aml_file, interpreter) + match resolve_and_compile(&script.asl_file.path().to_path_buf(), true) { + CompilationOutcome::Succeeded(aml_path) | CompilationOutcome::IsAml(aml_path) => { + run_test_for_file(&aml_path, interpreter) + } + _ => RunTestResult::Failed(interpreter, TestFailureReason::CompileFail), + } } /// Test an AML file using [`run_test`] @@ -199,9 +268,12 @@ pub fn run_test_for_string(asl: &'static str, interpreter: &mut Interpreter) -> TestResult { - let Ok(mut file) = File::open(&file) else { - return TestResult::CompileFail; +pub fn run_test_for_file(file: &PathBuf, interpreter: Interpreter) -> RunTestResult +where + T: Handler, +{ + let Ok(mut file) = File::open(file) else { + return RunTestResult::Failed(interpreter, TestFailureReason::FilesystemErr); }; let mut contents = Vec::new(); file.read_to_end(&mut contents).unwrap(); @@ -209,13 +281,7 @@ pub fn run_test_for_file(file: &PathBuf, interpreter: &mut Interpreter TestResult::Pass, - Err(e) => { - error!("Error running test: {:?}", e); - TestResult::ParseFail - } - } + run_test(stream, interpreter) } /// Internal function to create a temporary script file from an ASL string, plus to calculate the @@ -226,16 +292,10 @@ fn create_script_file(asl: &'static str) -> TempScriptFile { let mut script_file = NamedTempFile::with_suffix_in(".asl", &dir).unwrap(); trace!("Created temp file: {:?}", script_file.path()); - let output_stem = script_file.path().file_stem().unwrap().to_str().unwrap(); - let output_name = format!("{}.aml", output_stem); - let output_full_name = dir.path().join(output_name.clone()); - - let new_asl = asl.replace("%FN%", output_name.as_str()); - - script_file.write_all(new_asl.as_bytes()).unwrap(); + script_file.write_all(asl.as_bytes()).unwrap(); script_file.flush().unwrap(); - TempScriptFile { dir, asl_file: script_file, aml_file: output_full_name } + TempScriptFile { dir, asl_file: script_file } } /// Run a test on an AML stream. @@ -248,25 +308,43 @@ fn create_script_file(asl: &'static str) -> TempScriptFile { /// Arguments: /// /// * `stream`: A slice containing the AML bytecode to test. -/// * `interpreter`: The interpreter to test with. -pub fn run_test(stream: &[u8], interpreter: &mut Interpreter) -> Result<(), AmlError> { - interpreter.load_table(stream)?; +/// * `interpreter`: The interpreter to test with. The interpreter is consumed to maintain unwind +/// safety - if the interpreter panics, the caller should not be able to see the interpreter in +/// an inconsistent state. +pub fn run_test(stream: &[u8], interpreter: Interpreter) -> RunTestResult +where + T: Handler, +{ + // Without `AssertUnwindSafe`, the following code will not build as the Interpreter is not + // unwind safe. To avoid the caller being able to see an inconsistent Interpreter, if a panic + // occurs we drop the Interpreter, forcing the caller to create a new one. + let result = catch_unwind(AssertUnwindSafe(|| -> Result<(), AmlError> { + interpreter.load_table(stream)?; - if let Some(result) = interpreter.evaluate_if_present(AmlName::from_str("\\MAIN").unwrap(), vec![])? { - match *result { - Object::Integer(0) => Ok(()), - Object::Integer(other) => { - error!("Test _MAIN returned non-zero exit code: {}", other); - // TODO: wrong error - this should probs return a more complex err type - Err(AmlError::NoCurrentOp) - } - _ => { - error!("Test _MAIN returned unexpected object type: {}", *result); - // TODO: wrong error - Err(AmlError::NoCurrentOp) + if let Some(result) = interpreter.evaluate_if_present(AmlName::from_str("\\MAIN").unwrap(), vec![])? { + match *result { + Object::Integer(0) => Ok(()), + Object::Integer(other) => { + error!("Test _MAIN returned non-zero exit code: {}", other); + // TODO: wrong error - this should probs return a more complex err type + Err(AmlError::NoCurrentOp) + } + _ => { + error!("Test _MAIN returned unexpected object type: {}", *result); + // TODO: wrong error + Err(AmlError::NoCurrentOp) + } } + } else { + Ok(()) } - } else { - Ok(()) + })); + + match result { + Ok(inner) => match inner { + Ok(()) => RunTestResult::Pass(interpreter), + Err(e) => RunTestResult::Failed(interpreter, TestFailureReason::ParseFail(e)), + }, + Err(_e) => RunTestResult::Panicked, } } diff --git a/tools/aml_tester/src/main.rs b/tools/aml_tester/src/main.rs index 82e6e3d2..b48ef4bc 100644 --- a/tools/aml_tester/src/main.rs +++ b/tools/aml_tester/src/main.rs @@ -15,6 +15,8 @@ use aml_test_tools::{ new_interpreter, resolve_and_compile, CompilationOutcome, + RunTestResult, + TestFailureReason, TestResult, }; use clap::{Arg, ArgAction, ArgGroup}; @@ -26,8 +28,34 @@ use std::{ path::{Path, PathBuf}, process::Command, }; +use std::process::ExitCode; -fn main() -> std::io::Result<()> { +/// The result of a test, with all other information (error codes etc.) stripped away. This value +/// can then be stored in a Set - the test results with more info cannot. +#[derive(Eq, Hash, PartialEq)] +enum FinalTestResult { + Passed, + Failed, + NotCompiled, + CompileFailed, +} + +impl From for FinalTestResult { + fn from(outcome: TestResult) -> Self { + match outcome { + TestResult::Pass => FinalTestResult::Passed, + TestResult::Failed(e) => match e { + TestFailureReason::CompileFail | TestFailureReason::FilesystemErr => { + FinalTestResult::CompileFailed + } + _ => FinalTestResult::Failed, + }, + TestResult::Panicked => FinalTestResult::Failed, + } + } +} + +fn main() -> ExitCode { pretty_env_logger::init(); let mut cmd = clap::Command::new("aml_tester") @@ -48,8 +76,8 @@ If the ASL contains a MAIN method, it will be executed.", .arg(Arg::new("files").action(ArgAction::Append).value_name("FILE.{asl,aml}")) .group(ArgGroup::new("files_list").args(["path", "files"]).required(true)); if std::env::args().count() <= 1 { - cmd.print_help()?; - return Ok(()); + cmd.print_help().unwrap(); + return ExitCode::SUCCESS; } log::set_max_level(log::LevelFilter::Info); @@ -67,9 +95,9 @@ If the ASL contains a MAIN method, it will be executed.", Err(_) => false, }; - let tests = find_tests(&matches)?; + let tests = find_tests(&matches).unwrap(); let compiled_files: Vec = - tests.iter().map(|name| resolve_and_compile(name, can_compile).unwrap()).collect(); + tests.iter().map(|name| resolve_and_compile(name, can_compile)).collect(); // Check if compilation should have happened but did not if user_wants_compile @@ -97,22 +125,24 @@ If the ASL contains a MAIN method, it will be executed.", } } + let mut passed = 0u32; + let mut failed = 0u32; + // Make a list of the files we have processed, and skip them if we see them again let mut dedup_list: HashSet = HashSet::new(); - let mut summaries: HashSet<(PathBuf, TestResult)> = HashSet::new(); + let mut summaries: HashSet<(PathBuf, FinalTestResult)> = HashSet::new(); // Filter down to the final list of AML files let aml_files = compiled_files .iter() .filter_map(|outcome| match outcome { - CompilationOutcome::IsAml(path) => Some(path.clone()), - CompilationOutcome::Newer(path) => Some(path.clone()), - CompilationOutcome::Succeeded(path) => Some(path.clone()), - CompilationOutcome::Failed(path) => { - summaries.insert((path.clone(), TestResult::CompileFail)); - None - } - CompilationOutcome::NotCompiled(path) => { - summaries.insert((path.clone(), TestResult::NotCompiled)); + CompilationOutcome::IsAml(path) + | CompilationOutcome::Newer(path) + | CompilationOutcome::Succeeded(path) => Some(path.clone()), + CompilationOutcome::Failed(path) + | CompilationOutcome::NotCompiled(path) + | CompilationOutcome::FilesystemErr(path) => { + summaries.insert((path.clone(), FinalTestResult::NotCompiled)); + failed += 1; None } CompilationOutcome::Ignored => None, @@ -130,53 +160,72 @@ If the ASL contains a MAIN method, it will be executed.", let combined_test = matches.get_flag("combined"); let mut interpreter = new_interpreter(new_handler()); - let (passed, failed) = aml_files.into_iter().fold((0, 0), |(passed, failed), file_entry| { + for file_entry in aml_files { print!("Testing AML file: {:?}... ", file_entry); std::io::stdout().flush().unwrap(); - if !combined_test { - interpreter = new_interpreter(new_handler()); - } + let result = aml_test_tools::run_test_for_file(&file_entry, interpreter); + let simple_result: FinalTestResult = TestResult::from(&result).into(); - let result = aml_test_tools::run_test_for_file(&file_entry, &mut interpreter); - let updates = match result { - TestResult::Pass => { + let interpreter_returned = match result { + RunTestResult::Pass(i) => { println!("{}", "OK".green()); - (passed + 1, failed) + passed += 1; + Some(i) + } + RunTestResult::Failed(i, e) => { + println!("{}", format!("Failed ({:?})", e).red()); + failed += 1; + Some(i) } - TestResult::CompileFail | TestResult::ParseFail | TestResult::NotCompiled => { - println!("{}", format!("Failed ({:?})", result).red()); - (passed, failed + 1) + RunTestResult::Panicked => { + println!("{}", "Panicked".red()); + failed += 1; + if combined_test { + // We can't continue to use the old Interpreter, and the user specifically wants + // to. Creating a new one most likely invalidates the test they were trying to + // perform. + panic!("Interpreter panicked during combined test, unable to continue."); + } + None } }; - println!("Namespace: {}", interpreter.namespace.lock()); - summaries.insert((file_entry, result)); + interpreter = match interpreter_returned { + _ if !combined_test => new_interpreter(new_handler()), + None => new_interpreter(new_handler()), + Some(i) => i, + }; - updates - }); + println!("Namespace: {}", interpreter.namespace.lock()); + summaries.insert((file_entry, simple_result)); + } // Print summaries println!("Summary:"); for (file, status) in summaries.iter() { let status = match status { - TestResult::Pass => { + FinalTestResult::Passed => { format!("{}", "OK".green()) } - TestResult::CompileFail => { + FinalTestResult::CompileFailed => { format!("{}", "COMPILE FAIL".red()) } - TestResult::ParseFail => { + FinalTestResult::Failed => { format!("{}", "PARSE FAIL".red()) } - TestResult::NotCompiled => { + FinalTestResult::NotCompiled => { format!("{}", "NOT COMPILED".red()) } }; println!("\t{:<50}: {}", file.to_str().unwrap(), status); } println!("\nTest results: {}, {}", format!("{} passed", passed).green(), format!("{} failed", failed).red()); - Ok(()) + if failed == 0 { + ExitCode::SUCCESS + } else { + ExitCode::FAILURE + } } fn find_tests(matches: &clap::ArgMatches) -> std::io::Result> { From b880f70d2a3c85a1f0779a46584fc75c12e5309a Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 17 Mar 2026 20:08:38 +0000 Subject: [PATCH 2/4] Disable iasl optimisations for tests This was masking some bugs in `ToHexString` and `ToInteger` because those calls were being folded out by the iasl optimizer. --- src/aml/mod.rs | 2 +- tests/to_integer.asl | 12 ++++++++++++ tools/aml_test_tools/src/lib.rs | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index a54a2321..3043f45a 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -2031,7 +2031,7 @@ where Object::String(ref value) => Object::String(value.clone()), Object::Integer(value) => match op.op { Opcode::ToDecimalString => Object::String(value.to_string()), - Opcode::ToHexString => Object::String(alloc::format!("{value:#x}")), + Opcode::ToHexString => Object::String(alloc::format!("{value:#X}")), _ => panic!(), }, Object::Buffer(ref bytes) => { diff --git a/tests/to_integer.asl b/tests/to_integer.asl index 8de15d7c..e6712ff0 100644 --- a/tests/to_integer.asl +++ b/tests/to_integer.asl @@ -20,29 +20,39 @@ DefinitionBlock ("", "SSDT", 2, "uTEST", "TESTTABL", 0xF0F0F0F0) Local1 = 123 CHEK(Local0, Local1, __LINE__) + /* TODO: Restore commented out tests Local0 = ToInteger(" \t\t\t\v 123") Local1 = 123 CHEK(Local0, Local1, __LINE__) + */ + /* Local0 = ToInteger("123abcd") Local1 = 123 CHEK(Local0, Local1, __LINE__) + */ Local0 = ToInteger("0x123abcd") Local1 = 0x123abcd CHEK(Local0, Local1, __LINE__) + /* Local0 = ToInteger("") Local1 = 0 CHEK(Local0, Local1, __LINE__) + */ + /* Local0 = ToInteger("0X") Local1 = 0 CHEK(Local0, Local1, __LINE__) + */ + /* Local0 = ToInteger("0x") Local1 = 0 CHEK(Local0, Local1, __LINE__) + */ Local0 = ToInteger("0") Local1 = 0 @@ -52,9 +62,11 @@ DefinitionBlock ("", "SSDT", 2, "uTEST", "TESTTABL", 0xF0F0F0F0) Local1 = 0xDEADBEEF CHEK(Local0, Local1, __LINE__) + /* Local0 = ToInteger("0XDeAdBeeFCafeBabeHelloWorld") Local1 = 0xDEADBEEFCAFEBABE CHEK(Local0, Local1, __LINE__) + */ Local0 = ToInteger(Buffer { 0xDE, 0xAD, 0xBE, 0xEF }) Local1 = 0xEFBEADDE diff --git a/tools/aml_test_tools/src/lib.rs b/tools/aml_test_tools/src/lib.rs index 130630e8..57c3e603 100644 --- a/tools/aml_test_tools/src/lib.rs +++ b/tools/aml_test_tools/src/lib.rs @@ -158,7 +158,7 @@ pub fn resolve_and_compile(path: &PathBuf, can_compile: bool) -> CompilationOutc // Compile the ASL file using `iasl` println!("Compiling file: {}", path.display()); - let output = Command::new("iasl").arg(path).output(); + let output = Command::new("iasl").arg("-oa").arg(path).output(); match output { Ok(output) => { From 93d46d29833592aa31e8f55da12f89fd9d81889d Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 17 Mar 2026 21:07:37 +0000 Subject: [PATCH 3/4] Fix `ToInteger` handling --- src/aml/mod.rs | 22 ++++++++++++---------- tests/to_integer.asl | 12 ------------ 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 3043f45a..3d7b47a2 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -1972,17 +1972,19 @@ where * that won't fit in a `u64` etc. We probably need to write a more robust parser * 'real' parser to handle those cases. */ - if let Some(value) = value.strip_prefix("0x") { - let parsed = u64::from_str_radix(value, 16).map_err(|_| { + let value = value.trim(); + let value = value.to_ascii_lowercase(); + let (value, radix): (&str, u32) = match value.strip_prefix("0x") { + Some(value) => { + (value.split(|c: char| !c.is_ascii_hexdigit()).next().unwrap_or(""), 16) + } + None => (value.split(|c: char| !c.is_ascii_digit()).next().unwrap_or(""), 10), + }; + match value.len() { + 0 => Object::Integer(0), + _ => Object::Integer(u64::from_str_radix(value, radix).map_err(|_| { AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: ObjectType::String } - })?; - Object::Integer(parsed) - } else { - let parsed = str::parse::(value).map_err(|_| AmlError::InvalidOperationOnObject { - op: Operation::ToInteger, - typ: ObjectType::String, - })?; - Object::Integer(parsed) + })?), } } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToBuffer, typ: operand.typ() })?, diff --git a/tests/to_integer.asl b/tests/to_integer.asl index e6712ff0..8de15d7c 100644 --- a/tests/to_integer.asl +++ b/tests/to_integer.asl @@ -20,39 +20,29 @@ DefinitionBlock ("", "SSDT", 2, "uTEST", "TESTTABL", 0xF0F0F0F0) Local1 = 123 CHEK(Local0, Local1, __LINE__) - /* TODO: Restore commented out tests Local0 = ToInteger(" \t\t\t\v 123") Local1 = 123 CHEK(Local0, Local1, __LINE__) - */ - /* Local0 = ToInteger("123abcd") Local1 = 123 CHEK(Local0, Local1, __LINE__) - */ Local0 = ToInteger("0x123abcd") Local1 = 0x123abcd CHEK(Local0, Local1, __LINE__) - /* Local0 = ToInteger("") Local1 = 0 CHEK(Local0, Local1, __LINE__) - */ - /* Local0 = ToInteger("0X") Local1 = 0 CHEK(Local0, Local1, __LINE__) - */ - /* Local0 = ToInteger("0x") Local1 = 0 CHEK(Local0, Local1, __LINE__) - */ Local0 = ToInteger("0") Local1 = 0 @@ -62,11 +52,9 @@ DefinitionBlock ("", "SSDT", 2, "uTEST", "TESTTABL", 0xF0F0F0F0) Local1 = 0xDEADBEEF CHEK(Local0, Local1, __LINE__) - /* Local0 = ToInteger("0XDeAdBeeFCafeBabeHelloWorld") Local1 = 0xDEADBEEFCAFEBABE CHEK(Local0, Local1, __LINE__) - */ Local0 = ToInteger(Buffer { 0xDE, 0xAD, 0xBE, 0xEF }) Local1 = 0xEFBEADDE From 92db67cf6e52b22f9b8134df141ae957adb1e349 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Sat, 21 Mar 2026 20:43:37 +0000 Subject: [PATCH 4/4] A more appropriate test failure error --- src/aml/mod.rs | 8 ++++++++ tools/aml_test_tools/src/lib.rs | 12 ++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 3d7b47a2..c55267bb 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -3335,4 +3335,12 @@ pub enum AmlError { /// This is emitted to signal that the library does not support the requested behaviour. This /// should eventually never be emitted. LibUnimplemented, + + /// The library has given a response the host does not understand, or the host is otherwise + /// unable to continue operating the library correctly. The specific reason is given in the + /// contained String. + /// + /// This variant is set by the host, not by the library, and can be used when it is convenient + /// not to construct a more complex error type around [`AmlError`]. + HostError(String), } diff --git a/tools/aml_test_tools/src/lib.rs b/tools/aml_test_tools/src/lib.rs index 57c3e603..3b7a3c32 100644 --- a/tools/aml_test_tools/src/lib.rs +++ b/tools/aml_test_tools/src/lib.rs @@ -325,14 +325,14 @@ where match *result { Object::Integer(0) => Ok(()), Object::Integer(other) => { - error!("Test _MAIN returned non-zero exit code: {}", other); - // TODO: wrong error - this should probs return a more complex err type - Err(AmlError::NoCurrentOp) + let e = format!("Test _MAIN returned non-zero exit code: {}", other); + error!("{}", e); + Err(AmlError::HostError(e)) } _ => { - error!("Test _MAIN returned unexpected object type: {}", *result); - // TODO: wrong error - Err(AmlError::NoCurrentOp) + let e = format!("Test _MAIN returned unexpected object type: {}", *result); + error!("{}", e); + Err(AmlError::HostError(e)) } } } else {