Skip to content

Commit 9928f16

Browse files
committed
Directly test a stream of opcodes
Use this to add a regression test for #289
1 parent 04baa6d commit 9928f16

5 files changed

Lines changed: 246 additions & 22 deletions

File tree

src/aml/mod.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub mod namespace;
2020
pub mod object;
2121
pub mod op_region;
2222
pub mod pci_routing;
23+
pub mod pkglength;
2324
pub mod resource;
2425

2526
use crate::{
@@ -28,6 +29,7 @@ use crate::{
2829
Handle,
2930
Handler,
3031
PhysicalMapping,
32+
aml::pkglength::decode_stream as decode_pkglength,
3133
platform::AcpiPlatform,
3234
registers::{FixedRegisters, Pm1ControlBit},
3335
sdt::{SdtHeader, facs::Facs, fadt::Fadt},
@@ -3201,19 +3203,7 @@ impl MethodContext {
32013203
}
32023204

32033205
fn pkglength(&mut self) -> Result<usize, AmlError> {
3204-
let lead_byte = self.next()?;
3205-
let byte_count = lead_byte.get_bits(6..8);
3206-
assert!(byte_count < 4);
3207-
3208-
if byte_count == 0 {
3209-
Ok(lead_byte.get_bits(0..6) as usize)
3210-
} else {
3211-
let mut length = lead_byte.get_bits(0..4) as usize;
3212-
for i in 0..byte_count {
3213-
length |= (self.next()? as usize) << (4 + i * 8);
3214-
}
3215-
Ok(length)
3216-
}
3206+
decode_pkglength(|| self.next())
32173207
}
32183208

32193209
fn namestring(&mut self) -> Result<AmlName, AmlError> {

src/aml/pkglength.rs

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
//! pkglength encoding and decoding.
2+
3+
use alloc::{vec, vec::Vec};
4+
use bit_field::BitField;
5+
use core::fmt::{Display, Formatter};
6+
7+
/// Indicates an attempt to encode a pkglength that is too long (>= 2 ^ 28).
8+
#[derive(Clone, Debug)]
9+
pub struct PkglengthTooLongError {}
10+
11+
impl Display for PkglengthTooLongError {
12+
fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
13+
write!(f, "pkglength too long")
14+
}
15+
}
16+
17+
impl core::error::Error for PkglengthTooLongError {}
18+
19+
/// Encode a pkglength field.
20+
///
21+
/// This is a variable length field used to define the length of other variable-length items in
22+
/// ACPI - see [the spec](https://uefi.org/specs/ACPI/6.6/20_AML_Specification.html#package-length-encoding)
23+
/// for details.
24+
///
25+
/// This is less straightforward than it could be, because pkglength needs to include the length of
26+
/// the pkglength field that gets output. This function adds that extra length.
27+
///
28+
/// Returns an error if length >= 2^28. Otherwise, returns the encoded pkglength in a vec, LSB-first.
29+
pub fn encode(data_length: u32) -> Result<Vec<u8>, PkglengthTooLongError> {
30+
let extra_length = match data_length {
31+
0..63 => 1,
32+
63..0xFFE => 2,
33+
0xFFE..0xFFFFD => 3,
34+
_ => 4,
35+
};
36+
let result = encode_raw(data_length + extra_length);
37+
result.inspect(|result| {
38+
assert_eq!(result.len(), extra_length as usize);
39+
})
40+
}
41+
42+
/// Encode a pkglength field, without taking into account the extra length of the pkglength field.
43+
///
44+
/// Most callers should use [`encode`] instead.
45+
///
46+
/// Returns an error if length >= 2^28. Otherwise, returns the encoded pkglength in a vec, LSB-first.
47+
pub fn encode_raw(mut length: u32) -> Result<Vec<u8>, PkglengthTooLongError> {
48+
if length & 0xF0000000 != 0 {
49+
// Must be less than 2 ^ 28
50+
return Err(PkglengthTooLongError {});
51+
}
52+
53+
if length < 64 {
54+
Ok(vec![length as u8])
55+
} else {
56+
let mut result = vec![(length & 0xF) as u8];
57+
length >>= 4;
58+
59+
while length != 0 {
60+
result.push((length & 0xff) as u8);
61+
length >>= 8;
62+
}
63+
64+
let num_bytes = result.len() as u8 - 1;
65+
result[0] |= num_bytes << 6;
66+
67+
Ok(result)
68+
}
69+
}
70+
71+
/// Decode a pkglength field from a stream
72+
///
73+
/// If the stream returns an error, that error is returned. Otherwise, the decoded length is
74+
/// returned.
75+
///
76+
/// `stream_next` must return the next byte in the stream when called (or an error, which is passed
77+
/// through)
78+
pub fn decode_stream<T>(mut stream_next: impl FnMut() -> Result<u8, T>) -> Result<usize, T> {
79+
let lead_byte = stream_next()?;
80+
let byte_count = lead_byte.get_bits(6..8);
81+
assert!(byte_count < 4);
82+
83+
if byte_count == 0 {
84+
Ok(lead_byte.get_bits(0..6) as usize)
85+
} else {
86+
let mut length = lead_byte.get_bits(0..4) as usize;
87+
for i in 0..byte_count {
88+
length |= (stream_next()? as usize) << (4 + i * 8);
89+
}
90+
Ok(length)
91+
}
92+
}
93+
94+
#[cfg(test)]
95+
mod tests {
96+
use super::*;
97+
98+
#[test]
99+
fn basic_round_trip() {
100+
let length: u32 = 0x12345;
101+
let encoded = encode_raw(length).unwrap();
102+
let mut i = encoded.iter();
103+
let decoded = decode_stream(|| i.next().copied().ok_or(())).unwrap();
104+
assert_eq!(decoded, length as usize);
105+
}
106+
107+
#[test]
108+
fn less_than_64() {
109+
let length: u32 = 0x12;
110+
let encoded = encode_raw(length).unwrap();
111+
assert_eq!(encoded, vec![0x12]);
112+
113+
let mut i = encoded.iter();
114+
let decoded = decode_stream(|| i.next().copied().ok_or(())).unwrap();
115+
assert_eq!(decoded, 0x12);
116+
}
117+
118+
#[test]
119+
fn encodes_zero() {
120+
let encoded = encode_raw(0).unwrap();
121+
assert_eq!(encoded, vec![0]);
122+
}
123+
124+
fn round_trip_length(length: u32) -> usize {
125+
let encoded = encode(length).unwrap();
126+
let mut i = encoded.iter();
127+
decode_stream(|| i.next().copied().ok_or(())).unwrap()
128+
}
129+
130+
#[test]
131+
fn extra_length_correct() {
132+
assert_eq!(round_trip_length(62), 63); // An increase of a single byte
133+
// 63 bytes of payload requires two bytes of pkglength
134+
assert_eq!(round_trip_length(63), 65);
135+
136+
// A random test:
137+
assert_eq!(round_trip_length(0x12345), 0x12348);
138+
139+
// Simply check that there's no errors around the boundaries - this tells us the maths to
140+
// calculate `extra_length` is correct.
141+
for i in 0xFF9..0x1004 {
142+
encode(i).unwrap();
143+
}
144+
for i in 0xFFFF9..0x100004 {
145+
encode(i).unwrap();
146+
}
147+
}
148+
}

tests/stray_opcodes_test.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
mod test_infra;
2+
3+
use crate::test_infra::run_opcodes_test;
4+
use aml_test_tools::handlers::null_handler::NullHandler;
5+
6+
#[test]
7+
fn test_stray_opcode() {
8+
// Make sure that a stray `One` opcode is ignored (see issue #289)
9+
// The first opcode is `One`, the remainder are `Return (0)`.
10+
let opcodes: Vec<u8> = vec![0x01, 0xA4, 0x0A, 0x00];
11+
12+
let handler = NullHandler;
13+
run_opcodes_test(&opcodes, handler);
14+
}

tests/test_infra/mod.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
use acpi::Handler;
2-
use aml_test_tools::{
3-
RunTestResult,
4-
TestResult,
5-
handlers::logging_handler::LoggingHandler,
6-
new_interpreter,
7-
run_test_for_string,
8-
};
2+
use aml_test_tools::{RunTestResult, TestResult, handlers::logging_handler::LoggingHandler, new_interpreter, run_test_for_string, run_test_for_opcodes};
93

4+
/// Run a test against an ASL string.
5+
///
6+
/// The string `asl` represents a compile-able ASL string, so needs to include the `DefinitionBlock`
7+
/// statement.
8+
#[allow(dead_code)]
109
pub fn run_aml_test(asl: &'static str, handler: impl Handler) {
1110
// Tests calling `run_aml_test` don't do much else, and we usually want logging, so initialize it here.
1211
let _ = pretty_env_logger::try_init();
@@ -17,3 +16,21 @@ pub fn run_aml_test(asl: &'static str, handler: impl Handler) {
1716
let result = run_test_for_string(asl, interpreter, &None);
1817
assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result));
1918
}
19+
20+
/// Run a test against a sequence of AML opcodes.
21+
///
22+
/// The provided opcodes are wrapped in a `MAIN` method and also have a table header prepended. The
23+
/// `MAIN` function is executed as part of the test, so `opcodes` only needs to include the actual
24+
/// opcodes to execute.
25+
#[allow(dead_code)]
26+
pub fn run_opcodes_test(opcodes: &[u8], handler: impl Handler) {
27+
// This function is very similar in structure to `run_aml_test`, but whilst there are only two
28+
// similar functions it's not worth adding complexity to make them DRY.
29+
let _ = pretty_env_logger::try_init();
30+
31+
let logged_handler = LoggingHandler::new(handler);
32+
let interpreter = new_interpreter(logged_handler);
33+
34+
let result = run_test_for_opcodes(opcodes, interpreter, &None);
35+
assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result));
36+
}

tools/aml_test_tools/src/lib.rs

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use acpi::{
1616
Handler,
1717
PhysicalMapping,
1818
address::MappedGas,
19-
aml::{AmlError, Interpreter, namespace::AmlName},
20-
sdt::{Signature, facs::Facs},
19+
aml::{AmlError, Interpreter, namespace::AmlName, pkglength::encode},
20+
sdt::{SdtHeader, Signature, facs::Facs},
2121
};
2222
use log::{error, trace};
2323
use std::{
@@ -330,6 +330,61 @@ where
330330
run_test(tables, interpreter, expected_result)
331331
}
332332

333+
/// Test a sequence of opcodes.
334+
///
335+
/// This may be useful for testing a sequence of opcodes that are seen in the wild, but which
336+
/// can't be generated by the AML compiler. (For example, see issue #289 where stray `One` opcodes
337+
/// were seen in AML - the `iasl` compiler will not generate this sequence.)
338+
///
339+
/// The provided opcodes are wrapped in a `MAIN` method and also have a table header prepended.
340+
///
341+
/// As with all other tests: if `MAIN` returns either 0 or undefined, then the test is considered
342+
/// successful.
343+
pub fn run_test_for_opcodes<T>(
344+
opcodes: &[u8],
345+
interpreter: Interpreter<T>,
346+
expected_result: &Option<ExpectedResult>,
347+
) -> RunTestResult<T>
348+
where
349+
T: Handler,
350+
{
351+
// DefMethod := MethodOp PkgLength NameString MethodFlags TermList
352+
let mut method_bytes: Vec<u8> = vec![0x14];
353+
// PkgLength - add 5 to cover the length of the method name and flags.
354+
method_bytes.extend(encode(opcodes.len() as u32 + 5).unwrap().iter());
355+
method_bytes.extend(r"MAIN".as_bytes()); // NameString
356+
method_bytes.extend([0]); // MethodFlags
357+
// Nothing for termlist
358+
359+
let table_header = SdtHeader {
360+
signature: Signature::DSDT,
361+
length: (size_of::<SdtHeader>() + opcodes.len() + method_bytes.len()) as u32,
362+
revision: 1,
363+
checksum: 0, // The crate doesn't check checksums, so no need to set it.
364+
oem_id: [0; 6],
365+
oem_table_id: [0; 8],
366+
oem_revision: 1,
367+
creator_id: [0; 4],
368+
creator_revision: 1,
369+
};
370+
371+
// Safe as we retain ownership of table_header and then drop both it and the slice at the end
372+
// of this function.
373+
let table_hdr_slice =
374+
unsafe { std::slice::from_raw_parts(&table_header as *const _ as *const u8, size_of::<SdtHeader>()) };
375+
376+
let mut table_data = vec![];
377+
table_data.extend(table_hdr_slice);
378+
table_data.extend(method_bytes);
379+
table_data.extend(opcodes);
380+
381+
let Ok(tables) = bytes_to_tables(&table_data) else {
382+
return RunTestResult::Failed(interpreter, TestFailureReason::TablesErr);
383+
};
384+
385+
run_test(tables, interpreter, expected_result)
386+
}
387+
333388
/// Internal function to create a temporary script file from an ASL string, plus to calculate the
334389
/// name of the post-compilation AML file.
335390
fn create_script_file(asl: &'static str) -> TempScriptFile {

0 commit comments

Comments
 (0)