diff --git a/src/aml/mod.rs b/src/aml/mod.rs index f347e0fb..fc773b52 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -3018,13 +3018,6 @@ impl MethodContext { } } - fn last_op(&mut self) -> Result<&mut OpInFlight, AmlError> { - match self.in_flight.last_mut() { - Some(op) => Ok(op), - None => Err(AmlError::NoCurrentOp), - } - } - fn contribute_arg(&mut self, arg: Argument) { if let Some(in_flight) = self.in_flight.last_mut() && in_flight.arguments.len() < in_flight.expected_arguments diff --git a/src/aml/resource.rs b/src/aml/resource.rs index ffdd3a65..5a85651e 100644 --- a/src/aml/resource.rs +++ b/src/aml/resource.rs @@ -551,7 +551,6 @@ fn extended_interrupt_descriptor(bytes: &[u8]) -> Result { #[cfg(test)] mod tests { use super::*; - use alloc::sync::Arc; #[test] fn test_parses_keyboard_crs() { diff --git a/tests/stray_opcodes_test.rs b/tests/stray_opcodes_test.rs new file mode 100644 index 00000000..ed239354 --- /dev/null +++ b/tests/stray_opcodes_test.rs @@ -0,0 +1,14 @@ +mod test_infra; + +use crate::test_infra::run_opcodes_test; +use aml_test_tools::handlers::null_handler::NullHandler; + +#[test] +fn test_stray_opcode() { + // Make sure that a stray `One` opcode is ignored (see issue #289) + // The first opcode is `One`, the remainder are `Return (0)`. + let opcodes: Vec = vec![0x01, 0xA4, 0x0A, 0x00]; + + let handler = NullHandler; + run_opcodes_test(&opcodes, handler); +} diff --git a/tests/test_infra/mod.rs b/tests/test_infra/mod.rs index 3e8751bf..03bfd60e 100644 --- a/tests/test_infra/mod.rs +++ b/tests/test_infra/mod.rs @@ -1,12 +1,14 @@ use acpi::Handler; -use aml_test_tools::{ - RunTestResult, - TestResult, - handlers::logging_handler::LoggingHandler, - new_interpreter, - run_test_for_string, -}; +use aml_test_tools::{RunTestResult, TestResult, handlers::logging_handler::LoggingHandler, new_interpreter, run_test_for_string, run_test_for_opcodes}; +// The following two functions are very similar in structure, but whilst there are only two of them +// it's not worth adding complexity to make them DRY. + +/// Run a test against an ASL string. +/// +/// The string `asl` represents a compile-able ASL string, so needs to include the `DefinitionBlock` +/// statement. +#[allow(dead_code)] 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(); @@ -17,3 +19,19 @@ pub fn run_aml_test(asl: &'static str, handler: impl Handler) { let result = run_test_for_string(asl, interpreter, &None); assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result)); } + +/// Run a test against a sequence of AML opcodes. +/// +/// The provided opcodes are wrapped in a `MAIN` method and also have a table header prepended. The +/// `MAIN` function is executed as part of the test, so `opcodes` only needs to include the actual +/// opcodes to execute. +#[allow(dead_code)] +pub fn run_opcodes_test(opcodes: &[u8], handler: impl Handler) { + let _ = pretty_env_logger::try_init(); + + let logged_handler = LoggingHandler::new(handler); + let interpreter = new_interpreter(logged_handler); + + let result = run_test_for_opcodes(opcodes, interpreter, &None); + assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result)); +} diff --git a/tools/aml_test_tools/src/lib.rs b/tools/aml_test_tools/src/lib.rs index d1692767..0407a9cd 100644 --- a/tools/aml_test_tools/src/lib.rs +++ b/tools/aml_test_tools/src/lib.rs @@ -5,6 +5,7 @@ //! As always, feel free to offer PRs for improvements. pub mod handlers; +pub mod pkglength; pub mod result; pub mod tables; @@ -17,7 +18,7 @@ use acpi::{ PhysicalMapping, address::MappedGas, aml::{AmlError, Interpreter, namespace::AmlName}, - sdt::{Signature, facs::Facs}, + sdt::{SdtHeader, Signature, facs::Facs}, }; use log::{error, trace}; use std::{ @@ -330,6 +331,61 @@ where run_test(tables, interpreter, expected_result) } +/// Test a sequence of opcodes. +/// +/// This may be useful for testing a sequence of opcodes that are seen in the wild, but which +/// can't be generated by the AML compiler. (For example, see issue #289 where stray `One` opcodes +/// were seen in AML - the `iasl` compiler will not generate this sequence.) +/// +/// The provided opcodes are wrapped in a `MAIN` method and also have a table header prepended. +/// +/// As with all other tests: if `MAIN` returns either 0 or undefined, then the test is considered +/// successful. +pub fn run_test_for_opcodes( + opcodes: &[u8], + interpreter: Interpreter, + expected_result: &Option, +) -> RunTestResult +where + T: Handler, +{ + // DefMethod := MethodOp PkgLength NameString MethodFlags TermList + let mut method_bytes: Vec = vec![0x14]; + // PkgLength - add 5 to cover the length of the method name and flags. + method_bytes.extend(pkglength::encode(opcodes.len() as u32 + 5).iter()); + method_bytes.extend(r"MAIN".as_bytes()); // NameString + method_bytes.extend([0]); // MethodFlags + // Nothing for termlist + + let table_header = SdtHeader { + signature: Signature::DSDT, + length: (size_of::() + opcodes.len() + method_bytes.len()) as u32, + revision: 1, + checksum: 0, // The crate doesn't check checksums, so no need to set it. + oem_id: [0; 6], + oem_table_id: [0; 8], + oem_revision: 1, + creator_id: [0; 4], + creator_revision: 1, + }; + + // Safe as we retain ownership of table_header and then drop both it and the slice at the end + // of this function. + let table_hdr_slice = + unsafe { std::slice::from_raw_parts(&table_header as *const _ as *const u8, size_of::()) }; + + let mut table_data = vec![]; + table_data.extend(table_hdr_slice); + table_data.extend(method_bytes); + table_data.extend(opcodes); + + let Ok(tables) = bytes_to_tables(&table_data) else { + return RunTestResult::Failed(interpreter, TestFailureReason::TablesErr); + }; + + run_test(tables, interpreter, expected_result) +} + /// Internal function to create a temporary script file from an ASL string, plus to calculate the /// name of the post-compilation AML file. fn create_script_file(asl: &'static str) -> TempScriptFile { diff --git a/tools/aml_test_tools/src/pkglength.rs b/tools/aml_test_tools/src/pkglength.rs new file mode 100644 index 00000000..7855bcc6 --- /dev/null +++ b/tools/aml_test_tools/src/pkglength.rs @@ -0,0 +1,54 @@ +//! pkglength encoding. + +use std::{vec, vec::Vec}; + +/// Encode a pkglength field. +/// +/// This is a variable length field used to define the length of other variable-length items in +/// ACPI - see [the spec](https://uefi.org/specs/ACPI/6.6/20_AML_Specification.html#package-length-encoding) +/// for details. +/// +/// This is less straightforward than it could be, because pkglength needs to include the length of +/// the pkglength field that gets output. This function adds that extra length. +/// +/// Panics if length >= 2^28. Otherwise, returns the encoded pkglength in a vec, LSB-first. +pub fn encode(data_length: u32) -> Vec { + let extra_length = match data_length { + 0..63 => 1, + 63..0xFFE => 2, + 0xFFE..0xFFFFD => 3, + _ => 4, + }; + let result = encode_raw(data_length + extra_length); + + // Check the table above is correct. + assert_eq!(result.len(), extra_length as usize); + + result +} + +/// Encode a pkglength field, without taking into account the extra length of the pkglength field. +/// +/// Most callers should use [`encode`] instead. +/// +/// Panics if length >= 2^28. Otherwise, returns the encoded pkglength in a vec, LSB-first. +pub fn encode_raw(mut length: u32) -> Vec { + assert_eq!(length & 0xF0000000, 0); + + if length < 64 { + vec![length as u8] + } else { + let mut result = vec![(length & 0xF) as u8]; + length >>= 4; + + while length != 0 { + result.push((length & 0xff) as u8); + length >>= 8; + } + + let num_bytes = result.len() as u8 - 1; + result[0] |= num_bytes << 6; + + result + } +}