From 14ea326644e1236fe8a76ec56ceff3970e717a3f Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 31 Mar 2026 22:01:23 +0100 Subject: [PATCH 1/3] Allow BufferField -> Integer conversion Both explicit and some implicit conversions are supported. --- src/aml/mod.rs | 67 ++++++++++++--------------------------------- src/aml/object.rs | 49 +++++++++++++++++++++++++++------ src/aml/resource.rs | 1 - tests/de_ref_of.rs | 27 ++++++++++++++++++ 4 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 tests/de_ref_of.rs diff --git a/src/aml/mod.rs b/src/aml/mod.rs index dd92b876..6b0f32ec 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -807,17 +807,19 @@ where } Opcode::DerefOf => { let [Argument::Object(object)] = &op.arguments[..] else { panic!() }; - let result = if object.typ() == ObjectType::Reference { - object.clone().unwrap_reference() - } else if object.typ() == ObjectType::String { - let path = AmlName::from_str(&object.as_string().unwrap())?; - let (_, object) = self.namespace.lock().search(&path, &context.current_scope)?; - object.clone() - } else { - return Err(AmlError::ObjectNotOfExpectedType { - expected: ObjectType::Reference, - got: object.typ(), - }); + let result = match **object { + Object::Reference { kind: _, inner: _ } => object.clone().unwrap_reference(), + Object::String(_) => { + let path = AmlName::from_str(&object.as_string().unwrap())?; + let (_, object) = self.namespace.lock().search(&path, &context.current_scope)?; + object.clone() + } + _ => { + return Err(AmlError::ObjectNotOfExpectedType { + expected: ObjectType::Reference, + got: object.typ(), + }); + } }; context.contribute_arg(Argument::Object(result)); context.retire_op(op); @@ -1796,8 +1798,9 @@ where let [Argument::Object(left), Argument::Object(right), target] = &op.arguments[0..3] else { panic!() }; let target2 = if op.op == Opcode::Divide { Some(&op.arguments[3]) } else { None }; - let left = left.clone().unwrap_transparent_reference().as_integer()?; - let right = right.clone().unwrap_transparent_reference().as_integer()?; + let allowed_length = if self.dsdt_revision >= 2 { 8 } else { 4 }; + let left = left.clone().unwrap_transparent_reference().to_integer(allowed_length)?; + let right = right.clone().unwrap_transparent_reference().to_integer(allowed_length)?; let result = match op.op { Opcode::Add => left.wrapping_add(right), @@ -1981,43 +1984,7 @@ where let [Argument::Object(operand), target] = &op.arguments[..] else { panic!() }; let operand = operand.clone().unwrap_transparent_reference(); - let result = match *operand { - Object::Integer(value) => Object::Integer(value), - Object::Buffer(ref bytes) => { - /* - * The spec says this should respect the revision of the current definition block. - * Apparently, the NT interpreter always uses the first 8 bytes of the buffer. - */ - let mut to_interpret = [0u8; 8]; - (to_interpret[0..usize::min(bytes.len(), 8)]).copy_from_slice(bytes); - Object::Integer(u64::from_le_bytes(to_interpret)) - } - Object::String(ref value) => { - /* - * This is about the same level of effort as ACPICA puts in. The uACPI test suite - * has tests that this fails - namely because of support for octal, signs, strings - * that won't fit in a `u64` etc. We probably need to write a more robust parser - * 'real' parser to handle those cases. - */ - 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 } - })?), - } - } - _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToBuffer, typ: operand.typ() })?, - } - .wrap(); - + let result = Object::Integer(operand.to_integer(if self.dsdt_revision >= 2 { 8 } else { 4 })?).wrap(); let result = self.do_store(target, result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); diff --git a/src/aml/object.rs b/src/aml/object.rs index 769be18f..938c8210 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -177,6 +177,9 @@ impl Object { WrappedObject::new(self) } + /// Unwraps an integer object. Errors if not already an integer. + /// + /// For casting to integer, use [`Object::to_integer`] instead. pub fn as_integer(&self) -> Result { if let Object::Integer(value) = self { Ok(*value) @@ -201,18 +204,48 @@ impl Object { } } + /// Converts the object to an integer. Used for both implicit and explicit conversions. + /// + /// To avoid the cast, use [`Object::as_integer`] instead. pub fn to_integer(&self, allowed_bytes: usize) -> Result { match self { Object::Integer(value) => Ok(*value), - Object::Buffer(value) => { - let length = usize::min(value.len(), allowed_bytes); - let mut bytes = [0u8; 8]; - bytes[0..length].copy_from_slice(&value[0..length]); - Ok(u64::from_le_bytes(bytes)) + Object::Buffer(bytes) => { + /* + * The spec says this should respect the revision of the current definition block. + * Apparently, the NT interpreter always uses the first 8 bytes of the buffer. + */ + let length = usize::min(bytes.len(), allowed_bytes); + let mut to_interpret = [0u8; 8]; + to_interpret[0..length].copy_from_slice(bytes); + Ok(u64::from_le_bytes(to_interpret)) + } + Object::String(value) => { + /* + * This is about the same level of effort as ACPICA puts in. The uACPI test suite + * has tests that this fails - namely because of support for octal, signs, strings + * that won't fit in a `u64` etc. We probably need to write a more robust parser + * 'real' parser to handle those cases. + */ + 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 => Ok(0), + _ => Ok(u64::from_str_radix(value, radix).map_err(|_| { + AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: ObjectType::String } + })?), + } + } + Object::BufferField { .. } => { + let mut buffer = [0u8; 8]; + self.read_buffer_field(&mut buffer)?; + Ok(u64::from_le_bytes(buffer)) } - // TODO: how should we handle invalid inputs? What does NT do here? - Object::String(value) => Ok(value.parse::().unwrap_or(0)), - _ => Ok(0), + _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: self.typ() })?, } } 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/de_ref_of.rs b/tests/de_ref_of.rs new file mode 100644 index 00000000..82d09490 --- /dev/null +++ b/tests/de_ref_of.rs @@ -0,0 +1,27 @@ +use aml_test_tools::handlers::null_handler::NullHandler; + +mod test_infra; + +#[test] +fn test_deref_of_buffer_field() { + const AML: &str = r#" +DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) { + Scope (\_SB) { + Name (ADAT, Buffer (0x0010) { + /* 0000 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* 0008 */ 0x00, 0xaa, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + }) + } + + Method(MAIN, 0, NotSerialized) { + Local0 = (DerefOf(\_SB.ADAT[0x09])) + // This relies on subtraction rather than equality as logical ops on BufferFields don't work + // yet. + return (Local0 - 0xaa) + } +} +"#; + + let handler = NullHandler {}; + test_infra::run_aml_test(AML, handler); +} From 5a1dc27fcf24dc93bf9138e507973c8c2606f401 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 14 Apr 2026 21:53:26 +0100 Subject: [PATCH 2/3] Buffer[Field] -> Integer tests & fixes --- src/aml/object.rs | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/aml/object.rs b/src/aml/object.rs index 938c8210..189c3798 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -217,7 +217,7 @@ impl Object { */ let length = usize::min(bytes.len(), allowed_bytes); let mut to_interpret = [0u8; 8]; - to_interpret[0..length].copy_from_slice(bytes); + to_interpret[0..length].copy_from_slice(&bytes[0..length]); Ok(u64::from_le_bytes(to_interpret)) } Object::String(value) => { @@ -243,7 +243,7 @@ impl Object { Object::BufferField { .. } => { let mut buffer = [0u8; 8]; self.read_buffer_field(&mut buffer)?; - Ok(u64::from_le_bytes(buffer)) + Ok(u64::from_le_bytes(buffer) % (1 << (allowed_bytes as u64 * 8))) } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: self.typ() })?, } @@ -544,4 +544,38 @@ mod tests { copy_bits(&src, 0, &mut dst, 2, 15); assert_eq!(dst, [0b1111_1101, 0b1101_1110, 0b0000_0001, 0b0000_0000, 0b0000_0000]); } + + #[test] + fn buffer_to_integer() { + let buffer = Object::Buffer(Vec::from([0xab, 0xcd, 0xef, 0x01, 0xff])); + assert_eq!(buffer.to_integer(4).unwrap(), 0x01efcdab); + } + + + #[test] + fn buffer_field_to_integer() { + const BUFFER: [u8; 5] = [0xffu8; 5]; + let buffer = Object::Buffer(Vec::from(BUFFER)).wrap(); + let buffer_field = Object::BufferField { + buffer, + offset: 5, + length: 9, + }; + + assert_eq!(buffer_field.to_integer(4).unwrap(), 0x1ff); + } + + #[test] + fn buffer_field_to_4_byte_integer() { + // The ones in this buffer are strategically chosen to not make it to the final integer. + const BUFFER: [u8; 5] = [0x0f, 0x00, 0x00, 0x00, 0xf0]; + let buffer = Object::Buffer(Vec::from(BUFFER)).wrap(); + let buffer_field = Object::BufferField { + buffer, + offset: 4, + length: 36, // This should be truncated to 32 bits in the conversion + }; + + assert_eq!(buffer_field.to_integer(4).unwrap(), 0); + } } From a6de1171adbece2f150b78d390fb46d395cbcfe4 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 14 Apr 2026 22:02:50 +0100 Subject: [PATCH 3/3] Remove possible overflow during conversion --- src/aml/mod.rs | 3 +++ src/aml/object.rs | 24 +++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 6b0f32ec..8bf1c420 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -3386,4 +3386,7 @@ pub enum AmlError { /// 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), + + /// An integer operation was attempted for an integer greater than 8 bytes long. + InvalidIntegerSize(usize), } diff --git a/src/aml/object.rs b/src/aml/object.rs index 189c3798..77d4b488 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -208,6 +208,11 @@ impl Object { /// /// To avoid the cast, use [`Object::as_integer`] instead. pub fn to_integer(&self, allowed_bytes: usize) -> Result { + // This check shouldn't hit, but it protects the `to_interpret` buffer below from panicking. + if allowed_bytes > size_of::() { + return Err(AmlError::InvalidIntegerSize(allowed_bytes)); + } + match self { Object::Integer(value) => Ok(*value), Object::Buffer(bytes) => { @@ -243,7 +248,11 @@ impl Object { Object::BufferField { .. } => { let mut buffer = [0u8; 8]; self.read_buffer_field(&mut buffer)?; - Ok(u64::from_le_bytes(buffer) % (1 << (allowed_bytes as u64 * 8))) + let mut result = u64::from_le_bytes(buffer); + if allowed_bytes < 8 { + result &= (1 << (allowed_bytes as u64 * 8)) - 1; + } + Ok(result) } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: self.typ() })?, } @@ -578,4 +587,17 @@ mod tests { assert_eq!(buffer_field.to_integer(4).unwrap(), 0); } + + #[test] + fn buffer_field_to_8_byte_integer() { + const BUFFER: [u8; 6] = [0x0f, 0x00, 0x00, 0x00, 0xf0, 0xff]; + let buffer = Object::Buffer(Vec::from(BUFFER)).wrap(); + let buffer_field = Object::BufferField { + buffer, + offset: 4, + length: 36, + }; + + assert_eq!(buffer_field.to_integer(8).unwrap(), 0x0000000f_00000000); + } }