From 67af636bf768a0a088243c0714b29698df5cde25 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 31 Mar 2026 22:01:23 +0100 Subject: [PATCH 1/7] Allow BufferField -> Integer conversion Both explicit and some implicit conversions are supported. --- src/aml/mod.rs | 66 ++++++++++++--------------------------------- src/aml/object.rs | 53 ++++++++++++++++++++++++++++++------ src/aml/resource.rs | 1 - tests/de_ref_of.rs | 27 +++++++++++++++++++ 4 files changed, 89 insertions(+), 58 deletions(-) create mode 100644 tests/de_ref_of.rs diff --git a/src/aml/mod.rs b/src/aml/mod.rs index f347e0fb..1a3a763e 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -838,17 +838,19 @@ where } Opcode::DerefOf => { extract_args!(op => [Argument::Object(object)]); - 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); @@ -1866,8 +1868,9 @@ where extract_args!(op[0..3] => [Argument::Object(left), Argument::Object(right), Argument::Object(target)]); 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), @@ -2051,42 +2054,7 @@ where extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); 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) => { - /* - * TODO: - * 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.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); diff --git a/src/aml/object.rs b/src/aml/object.rs index d79af4b1..13d89ecd 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -179,6 +179,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) @@ -203,18 +206,52 @@ 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]; + let o = self.read_buffer_field(allowed_bytes)?; + match o { + Object::Integer(value) => Ok(value), + Object::Buffer(bytes) => Ok(u64::from_le_bytes(bytes.try_into().unwrap())), + _ => unreachable!(), + } } - // 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 87b306f2c241d0226d118f997b424da342fe20cf Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 14 Apr 2026 21:53:26 +0100 Subject: [PATCH 2/7] Buffer[Field] -> Integer tests & fixes --- src/aml/object.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/aml/object.rs b/src/aml/object.rs index 13d89ecd..befe844c 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -219,7 +219,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) => { @@ -592,4 +592,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 7bd119717630cf2fa051437951355fc9805d7ee6 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Tue, 14 Apr 2026 22:02:50 +0100 Subject: [PATCH 3/7] Remove possible overflow during conversion --- src/aml/mod.rs | 3 +++ src/aml/object.rs | 34 +++++----------------------------- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 1a3a763e..1cbcbfa8 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -3500,4 +3500,7 @@ pub enum AmlError { /// An internal interpreter error has occured, and the interpreter has been left in an unknown /// state. More information may be given in the contained value. InternalError(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 befe844c..928726a1 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -210,6 +210,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,6 @@ impl Object { } } Object::BufferField { .. } => { - let mut buffer = [0u8; 8]; let o = self.read_buffer_field(allowed_bytes)?; match o { Object::Integer(value) => Ok(value), @@ -598,32 +602,4 @@ mod tests { 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 7d8590a32dd1b87d1461cf4124f5129b54c2bfb5 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Mon, 27 Apr 2026 15:28:50 +0100 Subject: [PATCH 4/7] Reinstate failing tests --- src/aml/object.rs | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/aml/object.rs b/src/aml/object.rs index 928726a1..4f20fe1d 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -249,11 +249,7 @@ impl Object { } Object::BufferField { .. } => { let o = self.read_buffer_field(allowed_bytes)?; - match o { - Object::Integer(value) => Ok(value), - Object::Buffer(bytes) => Ok(u64::from_le_bytes(bytes.try_into().unwrap())), - _ => unreachable!(), - } + o.to_integer(allowed_bytes) } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: self.typ() })?, } @@ -602,4 +598,35 @@ mod tests { 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); + } + + #[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); + } } From 554b719a018f46f9d15b47309e5b97ef39a90cf6 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Mon, 27 Apr 2026 20:30:22 +0100 Subject: [PATCH 5/7] Add integer size type This removes some repetitive calculations and unreachable branches. --- src/aml/dsdt_info.rs | 21 +++++++++++++++++++++ src/aml/mod.rs | 38 ++++++++++++++++---------------------- src/aml/object.rs | 35 ++++++++++++++--------------------- 3 files changed, 51 insertions(+), 43 deletions(-) create mode 100644 src/aml/dsdt_info.rs diff --git a/src/aml/dsdt_info.rs b/src/aml/dsdt_info.rs new file mode 100644 index 00000000..6518dc86 --- /dev/null +++ b/src/aml/dsdt_info.rs @@ -0,0 +1,21 @@ +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum IntegerSize { + FourBytes = 4, + EightBytes = 8, +} + +#[derive(Debug, Clone)] +pub struct DsdtInfo { + #[allow(dead_code)] + pub revision: u8, + pub integer_size: IntegerSize, +} + +impl DsdtInfo { + pub fn from_revision(revision: u8) -> DsdtInfo { + DsdtInfo { + integer_size: if revision >= 2 { IntegerSize::EightBytes } else { IntegerSize::FourBytes }, + revision, + } + } +} diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 1cbcbfa8..e52606d0 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -16,6 +16,7 @@ * - Fuzzing and guarantee panic-free interpretation */ +pub mod dsdt_info; pub mod namespace; pub mod object; pub mod op_region; @@ -28,6 +29,7 @@ use crate::{ Handle, Handler, PhysicalMapping, + aml::dsdt_info::{DsdtInfo, IntegerSize}, platform::AcpiPlatform, registers::{FixedRegisters, Pm1ControlBit}, sdt::{SdtHeader, facs::Facs, fadt::Fadt}, @@ -105,7 +107,7 @@ where handler: H, pub namespace: Spinlock, pub object_token: Spinlock, - dsdt_revision: u8, + dsdt_info: DsdtInfo, region_handlers: Spinlock>>, global_lock_mutex: Handle, @@ -140,7 +142,7 @@ where handler, namespace: Spinlock::new(Namespace::new(global_lock_mutex)), object_token: Spinlock::new(unsafe { ObjectToken::create_interpreter_token() }), - dsdt_revision, + dsdt_info: DsdtInfo::from_revision(dsdt_revision), region_handlers: Spinlock::new(BTreeMap::new()), global_lock_mutex, registers, @@ -403,12 +405,6 @@ where } } - /// Returns the size of an integer (in bytes) for the set of tables parsed so far. This depends - /// on the revision of the initial DSDT. - pub fn integer_size(&self) -> usize { - if self.dsdt_revision >= 2 { 8 } else { 4 } - } - fn do_execute_method(&self, mut context: MethodContext) -> Result { /* * This is the main loop that executes operations. Every op is handled at the top-level of @@ -1621,7 +1617,7 @@ where let value = self.do_field_read(field)?; context.contribute_arg(Argument::Object(value)); } else if let Object::BufferField { .. } = *object { - let value = object.read_buffer_field(self.integer_size())?; + let value = object.read_buffer_field(self.dsdt_info.integer_size)?; context.contribute_arg(Argument::Object(value.wrap())); } else { context.contribute_arg(Argument::Object(object)); @@ -1868,9 +1864,8 @@ where extract_args!(op[0..3] => [Argument::Object(left), Argument::Object(right), Argument::Object(target)]); let target2 = if op.op == Opcode::Divide { Some(&op.arguments[3]) } else { None }; - 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 left = left.clone().unwrap_transparent_reference().to_integer(self.dsdt_info.integer_size)?; + let right = right.clone().unwrap_transparent_reference().to_integer(self.dsdt_info.integer_size)?; let result = match op.op { Opcode::Add => left.wrapping_add(right), @@ -1913,10 +1908,9 @@ where * This is a particularly important place to respect the integer width as set * by the DSDT revision. */ - match self.integer_size() { - 4 => ((operand as u32).leading_zeros() + 1) as u64, - 8 => (operand.leading_zeros() + 1) as u64, - _ => unreachable!(), + match self.dsdt_info.integer_size { + IntegerSize::FourBytes => ((operand as u32).leading_zeros() + 1) as u64, + IntegerSize::EightBytes => (operand.leading_zeros() + 1) as u64, } } } @@ -2024,7 +2018,7 @@ where let result = match *operand { Object::Buffer(ref bytes) => Object::Buffer(bytes.clone()), Object::Integer(value) => { - if self.integer_size() == 8 { + if self.dsdt_info.integer_size == IntegerSize::EightBytes { Object::Buffer(value.to_le_bytes().to_vec()) } else { Object::Buffer((value as u32).to_le_bytes().to_vec()) @@ -2054,7 +2048,7 @@ where extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); - let result = Object::Integer(operand.to_integer(if self.dsdt_revision >= 2 { 8 } else { 4 })?).wrap(); + let result = Object::Integer(operand.to_integer(self.dsdt_info.integer_size)?).wrap(); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); @@ -2195,9 +2189,9 @@ where let result = match source1.typ() { ObjectType::Integer => { let source1 = source1.as_integer()?; - let source2 = source2.to_integer(self.integer_size())?; + let source2 = source2.to_integer(self.dsdt_info.integer_size)?; let mut buffer = Vec::new(); - if self.integer_size() == 8 { + if self.dsdt_info.integer_size == IntegerSize::EightBytes { buffer.extend_from_slice(&source1.to_le_bytes()); buffer.extend_from_slice(&source2.to_le_bytes()); } else { @@ -2208,7 +2202,7 @@ where } ObjectType::Buffer => { let mut buffer = source1.as_buffer()?.to_vec(); - buffer.extend(source2.to_buffer(self.integer_size())?); + buffer.extend(source2.to_buffer(self.dsdt_info.integer_size)?); Object::Buffer(buffer).wrap() } _ => { @@ -2446,7 +2440,7 @@ where /// return either an `Integer` or `Buffer` as appropriate, guided by the size of the field /// and expected integer size (as per the DSDT revision). fn do_field_read(&self, field: &FieldUnit) -> Result { - let needs_buffer = field.bit_length > (self.integer_size() * 8); + let needs_buffer = field.bit_length > (self.dsdt_info.integer_size as usize * 8); let access_width_bits = field.flags.access_type_bytes()? * 8; trace!("AML field read. Field = {:?}", field); diff --git a/src/aml/object.rs b/src/aml/object.rs index 4f20fe1d..ecc81f59 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -1,4 +1,4 @@ -use crate::aml::{AmlError, Handle, Operation, op_region::OpRegion}; +use crate::aml::{AmlError, Handle, Operation, dsdt_info::IntegerSize, op_region::OpRegion}; use alloc::{ borrow::Cow, string::{String, ToString}, @@ -209,12 +209,7 @@ 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 { - // 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)); - } - + pub fn to_integer(&self, integer_size: IntegerSize) -> Result { match self { Object::Integer(value) => Ok(*value), Object::Buffer(bytes) => { @@ -222,7 +217,7 @@ impl Object { * 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 length = usize::min(bytes.len(), integer_size as usize); let mut to_interpret = [0u8; 8]; to_interpret[0..length].copy_from_slice(&bytes[0..length]); Ok(u64::from_le_bytes(to_interpret)) @@ -248,34 +243,32 @@ impl Object { } } Object::BufferField { .. } => { - let o = self.read_buffer_field(allowed_bytes)?; - o.to_integer(allowed_bytes) + self.read_buffer_field(integer_size)?.to_integer(integer_size) } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: self.typ() })?, } } - pub fn to_buffer(&self, allowed_bytes: usize) -> Result, AmlError> { + pub fn to_buffer(&self, integer_size: IntegerSize) -> Result, AmlError> { match self { Object::Buffer(bytes) => Ok(bytes.clone()), - Object::Integer(value) => match allowed_bytes { - 4 => Ok((*value as u32).to_le_bytes().to_vec()), - 8 => Ok(value.to_le_bytes().to_vec()), - _ => panic!(), + Object::Integer(value) => match integer_size { + IntegerSize::FourBytes => Ok((*value as u32).to_le_bytes().to_vec()), + IntegerSize::EightBytes => Ok(value.to_le_bytes().to_vec()), }, Object::String(value) => Ok(value.as_bytes().to_vec()), _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ConvertToBuffer, typ: self.typ() }), } } - pub fn read_buffer_field(&self, integer_size: usize) -> Result { + pub fn read_buffer_field(&self, integer_size: IntegerSize) -> Result { if let Self::BufferField { buffer, offset, length } = self { let buffer = match **buffer { Object::Buffer(ref buffer) => buffer.as_slice(), Object::String(ref string) => string.as_bytes(), _ => panic!(), }; - if *length <= integer_size { + if *length <= integer_size as usize { let mut dst = [0u8; 8]; copy_bits(buffer, *offset, &mut dst, 0, *length); Ok(Object::Integer(u64::from_le_bytes(dst))) @@ -596,7 +589,7 @@ mod tests { #[test] fn buffer_to_integer() { let buffer = Object::Buffer(Vec::from([0xab, 0xcd, 0xef, 0x01, 0xff])); - assert_eq!(buffer.to_integer(4).unwrap(), 0x01efcdab); + assert_eq!(buffer.to_integer(IntegerSize::FourBytes).unwrap(), 0x01efcdab); } #[test] fn buffer_field_to_integer() { @@ -604,7 +597,7 @@ mod tests { 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); + assert_eq!(buffer_field.to_integer(IntegerSize::FourBytes).unwrap(), 0x1ff); } #[test] @@ -618,7 +611,7 @@ mod tests { length: 36, // This should be truncated to 32 bits in the conversion }; - assert_eq!(buffer_field.to_integer(4).unwrap(), 0); + assert_eq!(buffer_field.to_integer(IntegerSize::FourBytes).unwrap(), 0); } #[test] @@ -627,6 +620,6 @@ mod tests { 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); + assert_eq!(buffer_field.to_integer(IntegerSize::EightBytes).unwrap(), 0x0000000f_00000000); } } From ffd52f34afc3cb4a2ff300f4fa1c60b71f0e4017 Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Mon, 27 Apr 2026 20:35:01 +0100 Subject: [PATCH 6/7] Remove unused error code --- src/aml/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index e52606d0..e78920ad 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -3494,7 +3494,4 @@ pub enum AmlError { /// An internal interpreter error has occured, and the interpreter has been left in an unknown /// state. More information may be given in the contained value. InternalError(String), - - /// An integer operation was attempted for an integer greater than 8 bytes long. - InvalidIntegerSize(usize), } From 4b81eba2604f05799f6d40ee85c18bfdc9d3002d Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Fri, 15 May 2026 06:44:11 +0100 Subject: [PATCH 7/7] Move IntegerSize back to mod.rs --- src/aml/dsdt_info.rs | 21 --------------------- src/aml/mod.rs | 38 ++++++++++++++++++++++++-------------- src/aml/object.rs | 6 ++---- tests/de_ref_of.rs | 1 + 4 files changed, 27 insertions(+), 39 deletions(-) delete mode 100644 src/aml/dsdt_info.rs diff --git a/src/aml/dsdt_info.rs b/src/aml/dsdt_info.rs deleted file mode 100644 index 6518dc86..00000000 --- a/src/aml/dsdt_info.rs +++ /dev/null @@ -1,21 +0,0 @@ -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum IntegerSize { - FourBytes = 4, - EightBytes = 8, -} - -#[derive(Debug, Clone)] -pub struct DsdtInfo { - #[allow(dead_code)] - pub revision: u8, - pub integer_size: IntegerSize, -} - -impl DsdtInfo { - pub fn from_revision(revision: u8) -> DsdtInfo { - DsdtInfo { - integer_size: if revision >= 2 { IntegerSize::EightBytes } else { IntegerSize::FourBytes }, - revision, - } - } -} diff --git a/src/aml/mod.rs b/src/aml/mod.rs index e78920ad..3fc9251f 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -16,7 +16,6 @@ * - Fuzzing and guarantee panic-free interpretation */ -pub mod dsdt_info; pub mod namespace; pub mod object; pub mod op_region; @@ -29,7 +28,6 @@ use crate::{ Handle, Handler, PhysicalMapping, - aml::dsdt_info::{DsdtInfo, IntegerSize}, platform::AcpiPlatform, registers::{FixedRegisters, Pm1ControlBit}, sdt::{SdtHeader, facs::Facs, fadt::Fadt}, @@ -107,7 +105,7 @@ where handler: H, pub namespace: Spinlock, pub object_token: Spinlock, - dsdt_info: DsdtInfo, + integer_size: IntegerSize, region_handlers: Spinlock>>, global_lock_mutex: Handle, @@ -142,7 +140,7 @@ where handler, namespace: Spinlock::new(Namespace::new(global_lock_mutex)), object_token: Spinlock::new(unsafe { ObjectToken::create_interpreter_token() }), - dsdt_info: DsdtInfo::from_revision(dsdt_revision), + integer_size: IntegerSize::from_revision(dsdt_revision), region_handlers: Spinlock::new(BTreeMap::new()), global_lock_mutex, registers, @@ -1617,7 +1615,7 @@ where let value = self.do_field_read(field)?; context.contribute_arg(Argument::Object(value)); } else if let Object::BufferField { .. } = *object { - let value = object.read_buffer_field(self.dsdt_info.integer_size)?; + let value = object.read_buffer_field(self.integer_size)?; context.contribute_arg(Argument::Object(value.wrap())); } else { context.contribute_arg(Argument::Object(object)); @@ -1864,8 +1862,8 @@ where extract_args!(op[0..3] => [Argument::Object(left), Argument::Object(right), Argument::Object(target)]); let target2 = if op.op == Opcode::Divide { Some(&op.arguments[3]) } else { None }; - let left = left.clone().unwrap_transparent_reference().to_integer(self.dsdt_info.integer_size)?; - let right = right.clone().unwrap_transparent_reference().to_integer(self.dsdt_info.integer_size)?; + let left = left.clone().unwrap_transparent_reference().to_integer(self.integer_size)?; + let right = right.clone().unwrap_transparent_reference().to_integer(self.integer_size)?; let result = match op.op { Opcode::Add => left.wrapping_add(right), @@ -1908,7 +1906,7 @@ where * This is a particularly important place to respect the integer width as set * by the DSDT revision. */ - match self.dsdt_info.integer_size { + match self.integer_size { IntegerSize::FourBytes => ((operand as u32).leading_zeros() + 1) as u64, IntegerSize::EightBytes => (operand.leading_zeros() + 1) as u64, } @@ -2018,7 +2016,7 @@ where let result = match *operand { Object::Buffer(ref bytes) => Object::Buffer(bytes.clone()), Object::Integer(value) => { - if self.dsdt_info.integer_size == IntegerSize::EightBytes { + if self.integer_size == IntegerSize::EightBytes { Object::Buffer(value.to_le_bytes().to_vec()) } else { Object::Buffer((value as u32).to_le_bytes().to_vec()) @@ -2048,7 +2046,7 @@ where extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); - let result = Object::Integer(operand.to_integer(self.dsdt_info.integer_size)?).wrap(); + let result = Object::Integer(operand.to_integer(self.integer_size)?).wrap(); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); @@ -2189,9 +2187,9 @@ where let result = match source1.typ() { ObjectType::Integer => { let source1 = source1.as_integer()?; - let source2 = source2.to_integer(self.dsdt_info.integer_size)?; + let source2 = source2.to_integer(self.integer_size)?; let mut buffer = Vec::new(); - if self.dsdt_info.integer_size == IntegerSize::EightBytes { + if self.integer_size == IntegerSize::EightBytes { buffer.extend_from_slice(&source1.to_le_bytes()); buffer.extend_from_slice(&source2.to_le_bytes()); } else { @@ -2202,7 +2200,7 @@ where } ObjectType::Buffer => { let mut buffer = source1.as_buffer()?.to_vec(); - buffer.extend(source2.to_buffer(self.dsdt_info.integer_size)?); + buffer.extend(source2.to_buffer(self.integer_size)?); Object::Buffer(buffer).wrap() } _ => { @@ -2440,7 +2438,7 @@ where /// return either an `Integer` or `Buffer` as appropriate, guided by the size of the field /// and expected integer size (as per the DSDT revision). fn do_field_read(&self, field: &FieldUnit) -> Result { - let needs_buffer = field.bit_length > (self.dsdt_info.integer_size as usize * 8); + let needs_buffer = field.bit_length > (self.integer_size as usize * 8); let access_width_bits = field.flags.access_type_bytes()? * 8; trace!("AML field read. Field = {:?}", field); @@ -3495,3 +3493,15 @@ pub enum AmlError { /// state. More information may be given in the contained value. InternalError(String), } + +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum IntegerSize { + FourBytes = 4, + EightBytes = 8, +} + +impl IntegerSize { + pub fn from_revision(revision: u8) -> IntegerSize { + if revision >= 2 { IntegerSize::EightBytes } else { IntegerSize::FourBytes } + } +} diff --git a/src/aml/object.rs b/src/aml/object.rs index ecc81f59..ec0a34a8 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -1,4 +1,4 @@ -use crate::aml::{AmlError, Handle, Operation, dsdt_info::IntegerSize, op_region::OpRegion}; +use crate::aml::{AmlError, Handle, IntegerSize, Operation, op_region::OpRegion}; use alloc::{ borrow::Cow, string::{String, ToString}, @@ -242,9 +242,7 @@ impl Object { })?), } } - Object::BufferField { .. } => { - self.read_buffer_field(integer_size)?.to_integer(integer_size) - } + Object::BufferField { .. } => self.read_buffer_field(integer_size)?.to_integer(integer_size), _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToInteger, typ: self.typ() })?, } } diff --git a/tests/de_ref_of.rs b/tests/de_ref_of.rs index 82d09490..0bd0a576 100644 --- a/tests/de_ref_of.rs +++ b/tests/de_ref_of.rs @@ -17,6 +17,7 @@ DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) { Local0 = (DerefOf(\_SB.ADAT[0x09])) // This relies on subtraction rather than equality as logical ops on BufferFields don't work // yet. + // TODO: Use logical ops for clarity, when available. return (Local0 - 0xaa) } }