AML: Implement OpReg-relative PkgLength parser

aml/src/lib.rs

@@ -706,6 +706,11 @@ pub enum AmlError {
     MalformedStream,
     InvalidNameSeg,
     InvalidPkgLength,
+    /// Invalid PkgLength relative to an OperationRegion
+    InvalidRegionPkgLength {
+        region_bit_length: u64,
+        raw_length: u32,
+    },
     InvalidFieldFlags,
     UnterminatedStringConstant,
     InvalidStringConstant,

aml/src/pkg_length.rs

@@ -2,9 +2,23 @@ use crate::{
     parser::{take, take_n, Parser, Propagate},
     AmlContext,
     AmlError,
+    AmlHandle,
+    AmlValue,
 };
 use bit_field::BitField;
 
+/*
+ * There are two types of PkgLength implemented: PkgLength and RegionPkgLength. The reason for this
+ * is that while both are parsed as PkgLength op, they might have different meanings in different
+ * contexts:
+ *
+ * - PkgLength refers to an offset within the AML input slice
+ * - RegionPkgLength refers to an offset within an operation region (and is used this way in parsers
+ *      like def_field())
+ *
+ * They both have identical fields, but the fields themselves have an entirely different meaning.
+ */
+
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub struct PkgLength {
     pub raw_length: u32,
@@ -14,6 +28,12 @@ pub struct PkgLength {
     pub end_offset: u32,
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub struct RegionPkgLength {
+    pub raw_length: u32,
+    pub end_offset: u32,
+}
+
 impl PkgLength {
     pub fn from_raw_length(stream: &[u8], raw_length: u32) -> Result<PkgLength, AmlError> {
         Ok(PkgLength {
@@ -29,6 +49,48 @@ impl PkgLength {
     }
 }
 
+impl RegionPkgLength {
+    pub fn from_raw_length(region_bit_length: u64, raw_length: u32) -> Result<RegionPkgLength, AmlError> {
+        Ok(RegionPkgLength {
+            raw_length,
+            end_offset: (region_bit_length as u32)
+                .checked_sub(raw_length)
+                .ok_or(AmlError::InvalidRegionPkgLength { region_bit_length, raw_length })?,
+        })
+    }
+}
+
+pub fn region_pkg_length<'a, 'c>(region_handle: AmlHandle) -> impl Parser<'a, 'c, RegionPkgLength>
+where
+    'c: 'a,
+{
+    move |input: &'a [u8], context: &'c mut AmlContext| -> crate::parser::ParseResult<'a, 'c, RegionPkgLength> {
+        let region_value = match context.namespace.get(region_handle) {
+            Ok(value) => value,
+            Err(err) => return Err((input, context, Propagate::Err(err))),
+        };
+
+        /*
+         * OperationRegion length is in bytes, PkgLength is in bits, so conversion is needed
+         */
+        let region_bit_length = match region_value {
+            AmlValue::OpRegion { length, .. } => *length * 8,
+            _ => return Err((input, context, Propagate::Err(AmlError::FieldRegionIsNotOpRegion))),
+        };
+
+        let (new_input, context, raw_length) = raw_pkg_length().parse(input, context)?;
+
+        /*
+         * NOTE: we use the original input here, because `raw_length` includes the length of the
+         * `PkgLength`.
+         */
+        match RegionPkgLength::from_raw_length(region_bit_length, raw_length) {
+            Ok(pkg_length) => Ok((new_input, context, pkg_length)),
+            Err(err) => Err((input, context, Propagate::Err(err))),
+        }
+    }
+}
+
 pub fn pkg_length<'a, 'c>() -> impl Parser<'a, 'c, PkgLength>
 where
     'c: 'a,

aml/src/term_object.rs

@@ -17,7 +17,7 @@ use crate::{
         Parser,
         Propagate,
     },
-    pkg_length::{pkg_length, PkgLength},
+    pkg_length::{pkg_length, region_pkg_length, PkgLength},
     statement::statement_opcode,
     value::{AmlValue, FieldFlags, MethodCode, MethodFlags, RegionSpace},
     AmlContext,
@@ -561,32 +561,39 @@ where
      * Reserved fields shouldn't actually be added to the namespace; they seem to show gaps in
      * the operation region that aren't used for anything.
      */
-    let reserved_field =
-        opcode(opcode::RESERVED_FIELD).then(pkg_length()).map(|((), length)| Ok(length.raw_length as u64));
+    let reserved_field = opcode(opcode::RESERVED_FIELD)
+        .then(region_pkg_length(region_handle))
+        .map(|((), length)| Ok(length.raw_length as u64));
 
     // TODO: work out what to do with an access field
     // let access_field = opcode(opcode::ACCESS_FIELD)
     //     .then(take())
     //     .then(take())
     //     .map_with_context(|(((), access_type), access_attrib), context| (Ok(    , context));
 
-    let named_field = name_seg().then(pkg_length()).map_with_context(move |(name_seg, length), context| {
-        try_with_context!(
-            context,
-            context.namespace.add_value_at_resolved_path(
-                AmlName::from_name_seg(name_seg),
-                &context.current_scope,
-                AmlValue::Field {
-                    region: region_handle,
-                    flags,
-                    offset: current_offset,
-                    length: length.raw_length as u64,
-                },
-            )
-        );
-
-        (Ok(length.raw_length as u64), context)
-    });
+    // TODO: fields' start and end offsets need to be checked against their enclosing
+    //       OperationRegions to make sure they don't sit outside or cross the boundary.
+    //       This might not be a problem if a sane ASL compiler is used (which should check this
+    //       at compile-time), but it's better to be safe and validate that as well.
+
+    let named_field =
+        name_seg().then(region_pkg_length(region_handle)).map_with_context(move |(name_seg, length), context| {
+            try_with_context!(
+                context,
+                context.namespace.add_value_at_resolved_path(
+                    AmlName::from_name_seg(name_seg),
+                    &context.current_scope,
+                    AmlValue::Field {
+                        region: region_handle,
+                        flags,
+                        offset: current_offset,
+                        length: length.raw_length as u64,
+                    },
+                )
+            );
+
+            (Ok(length.raw_length as u64), context)
+        });
 
     choice!(reserved_field, named_field)
 }
@@ -868,7 +875,7 @@ where
                 if let Ok((_name, _handle)) = handle {
                     match target {
                         Target::Null => { /* just return the result of the check */ }
-                        _ => {return (Err(Propagate::Err(AmlError::Unimplemented)), context) },
+                        _ => return (Err(Propagate::Err(AmlError::Unimplemented)), context),
                     }
                 }
                 (Ok(result), context)