Compare strings and buffers like uACPI

src/aml/mod.rs

@@ -1949,57 +1949,34 @@ where
         let left = left.clone().unwrap_transparent_reference();
         let right = right.clone().unwrap_transparent_reference();
 
-        /*
-         * Some of these operations allow strings and buffers to be used as operands. Apparently
-         * NT's interpreter just takes the first 4 bytes of the string/buffer and casts them as an
-         * integer...
-         */
-        let (left, right) = match *left {
-            Object::Integer(left) => (left, right.as_integer()?),
-            Object::String(ref left) => {
-                let left = {
-                    let mut bytes = [0u8; 4];
-                    let left_bytes = left.as_bytes();
-                    let bytes_to_use = usize::min(4, left_bytes.len());
-                    (bytes[0..bytes_to_use]).copy_from_slice(&left_bytes[0..bytes_to_use]);
-                    u32::from_le_bytes(bytes) as u64
-                };
-                let right = {
-                    let mut bytes = [0u8; 4];
-                    let right = right.as_string()?;
-                    let right_bytes = right.as_bytes();
-                    let bytes_to_use = usize::min(4, right_bytes.len());
-                    (bytes[0..bytes_to_use]).copy_from_slice(&right_bytes[0..bytes_to_use]);
-                    u32::from_le_bytes(bytes) as u64
-                };
-                (left, right)
-            }
-            Object::Buffer(ref left) => {
-                let Object::Buffer(ref right) = *right else { panic!() };
-                let left = {
-                    let mut bytes = [0u8; 4];
-                    (bytes[0..left.len()]).copy_from_slice(left);
-                    u32::from_le_bytes(bytes) as u64
-                };
-                let right = {
-                    let mut bytes = [0u8; 4];
-                    (bytes[0..right.len()]).copy_from_slice(right);
-                    u32::from_le_bytes(bytes) as u64
-                };
-                (left, right)
+        let mut int_size = self.integer_size;
+
+        // Make sure both sides are the same type.
+        let right = match *left {
+            Object::Integer(_) => &Object::Integer(right.as_integer()?),
+            Object::String(_) => &Object::String(right.as_string()?.parse().unwrap()),
+            Object::Buffer(_) => {
+                // When doing && or ||, uACPI and NT only compare the first 4 bytes of a buffer.
+                int_size = IntegerSize::FourBytes;
+                if right.typ() == ObjectType::Buffer {
+                    &*right
+                } else {
+                    &Object::Buffer(right.to_buffer(self.integer_size)?)
+                }
             }
             _ => Err(AmlError::InvalidOperationOnObject { op: Operation::LogicalOp, typ: left.typ() })?,
         };
 
+        let ordering = left.aml_cmp(right);
         let result = match op.op {
-            Opcode::LAnd => (left > 0) && (right > 0),
-            Opcode::LOr => (left > 0) || (right > 0),
-            Opcode::LNotEqual => left != right,
-            Opcode::LLessEqual => left <= right,
-            Opcode::LGreaterEqual => left >= right,
-            Opcode::LEqual => left == right,
-            Opcode::LGreater => left > right,
-            Opcode::LLess => left < right,
+            Opcode::LAnd => (left.to_integer(int_size)? > 0) && (right.to_integer(int_size)? > 0),
+            Opcode::LOr => (left.to_integer(int_size)? > 0) || (right.to_integer(int_size)? > 0),
+            Opcode::LNotEqual => ordering?.is_ne(),
+            Opcode::LLessEqual => ordering?.is_le(),
+            Opcode::LGreaterEqual => ordering?.is_ge(),
+            Opcode::LEqual => ordering?.is_eq(),
+            Opcode::LGreater => ordering?.is_gt(),
+            Opcode::LLess => ordering?.is_lt(),
             _ => panic!(),
         };
         let result = if result { Object::Integer(u64::MAX) } else { Object::Integer(0) };

src/aml/object.rs

@@ -6,7 +6,7 @@ use alloc::{
     vec::Vec,
 };
 use bit_field::BitField;
-use core::{cell::UnsafeCell, fmt, ops, sync::atomic::AtomicU64};
+use core::{cell::UnsafeCell, cmp::Ordering, fmt, ops, sync::atomic::AtomicU64};
 
 type NativeMethod = dyn Fn(&[WrappedObject]) -> Result<WrappedObject, AmlError>;
 
@@ -355,6 +355,25 @@ impl Object {
             Object::Debug => ObjectType::Debug,
         }
     }
+
+    /// Calculate the ordering of two objects using the AML rules
+    ///
+    /// This function is not intended to be used for `impl PartialOrd` because we don't want to tie
+    /// the meaning of `object_a.cmp(object_b)` to those AML rules - we may want more flexibility.
+    pub fn aml_cmp(&self, other: &Object) -> Result<Ordering, AmlError> {
+        match (self, &other) {
+            (Object::Integer(a), Object::Integer(b)) => Ok(a.cmp(b)),
+            (Object::String(a), Object::String(b)) => Ok(a.cmp(b)),
+            (Object::Buffer(a), Object::Buffer(b)) => {
+                let size_cmp = a.len().cmp(&b.len());
+                if size_cmp != Ordering::Equal {
+                    return Ok(size_cmp);
+                }
+                Ok(a.cmp(b))
+            }
+            _ => Err(AmlError::InvalidOperationOnObject { op: Operation::LogicalOp, typ: self.typ() }),
+        }
+    }
 }
 
 #[derive(Clone, Debug)]

tests/buffer_logic.asl

@@ -0,0 +1,80 @@
+// Check correct logic operations on Buffers - see issue #297
+// These tests have been checked against uACPI commit 71f981d
+DefinitionBlock("", "DSDT", 1, "RSACPI", "Buffers", 1) {
+    // Buffers longer than integer size to check all bytes are compared, not just the first integer-size bytes.
+    Name(BUF0, Buffer() { 0x01, 0x02, 0x03, 0x04, 0x05 })
+    Name(BUF1, Buffer() { 0x01, 0x02, 0x03, 0x04, 0x05 })
+    Name(BUF2, Buffer() { 0x01, 0x02, 0x03, 0x04, 0x06 })
+
+    // Buffers integer size and less
+    NAME(BUF3, Buffer() { 0x01, 0x02, 0x03, 0x04 })
+    NAME(BUF4, Buffer() { 0x01, 0x02, 0x03, 0x05 })
+    NAME(BUF5, Buffer() { 0x01, 0x02, 0x03 })
+
+    // Zero buffer for checking and/or operators
+    NAME(ZBUF, Buffer() { 0x00 })
+
+    // Very long zero buffer to check only 4 bytes are used for and/or operators
+    NAME(LZBF, Buffer() { 0x00, 0x00, 0x00, 0x00, 0x01 })
+
+    Name(FCNT, 0)
+
+    Method (CHEK, 2) {
+        If (!Arg0) {
+            FCNT++
+            Printf("Failure on line %o", ToDecimalString(Arg1))
+        }
+    }
+
+    Method(MAIN) {
+        CHEK(BUF0 == BUF1, __LINE__)
+        CHEK(!(BUF0 != BUF1), __LINE__)
+
+        CHEK(BUF3 != BUF1, __LINE__)
+        CHEK(!(BUF3 == BUF1), __LINE__)
+
+        CHEK(BUF3 != BUF4, __LINE__)
+        CHEK(!(BUF3 == BUF4), __LINE__)
+
+        CHEK(BUF5 != BUF3, __LINE__)
+        CHEK(!(BUF5 == BUF3), __LINE__)
+
+        CHEK(BUF1 > BUF3, __LINE__)
+        CHEK(!(BUF3 > BUF1), __LINE__)
+
+        CHEK(BUF4 > BUF3, __LINE__)
+        CHEK(!(BUF3 > BUF4), __LINE__)
+
+        CHEK(BUF3 < BUF4, __LINE__)
+        CHEK(!(BUF4 < BUF3), __LINE__)
+
+        CHEK(BUF3 < BUF1, __LINE__)
+        CHEK(!(BUF1 < BUF3), __LINE__)
+
+        CHEK(BUF3 >= BUF3, __LINE__)
+        CHEK(!(BUF3 >= BUF4), __LINE__)
+
+        CHEK(BUF4 >= BUF3, __LINE__)
+        CHEK(!(BUF3 >= BUF4), __LINE__)
+
+        CHEK(BUF3 <= BUF3, __LINE__)
+        CHEK(!(BUF4 <= BUF3), __LINE__)
+
+        CHEK(BUF3 <= BUF4, __LINE__)
+        CHEK(!(BUF4 <= BUF3), __LINE__)
+
+        CHEK(BUF3 && BUF3, __LINE__)
+        CHEK(!(ZBUF && BUF3), __LINE__)
+        CHEK(!(BUF3 && ZBUF), __LINE__)
+        CHEK(!(ZBUF && ZBUF), __LINE__)
+
+        CHEK(BUF3 || BUF3, __LINE__)
+        CHEK(ZBUF || BUF3, __LINE__)
+        CHEK(BUF3 || ZBUF, __LINE__)
+        CHEK(!(ZBUF || ZBUF), __LINE__)
+
+        CHEK(!(LZBF && LZBF), __LINE__)
+
+        Return(FCNT)
+    }
+}

tests/string_logic.asl

@@ -0,0 +1,54 @@
+// Check correct logic operations on strings - see issue #297
+// These tests have been checked against uACPI commit 71f981d
+DefinitionBlock("", "DSDT", 1, "RSACPI", "Strings", 1) {
+    Name(FCNT, 0)
+
+    Method (CHEK, 2) {
+        If (!Arg0) {
+            FCNT++
+            Printf("Failure on line %o", ToDecimalString(Arg1))
+        }
+    }
+
+    Method(MAIN) {
+        CHEK("Hello" != "Hell", __LINE__)
+        CHEK(!("Hello" == "Hell"), __LINE__)
+
+        CHEK("Hella" != "Hello", __LINE__)
+        CHEK(!("Hello" != "Hello"), __LINE__)
+
+        CHEK("Hello" == "Hello", __LINE__)
+        CHEK(!("Hello" == "Hella"), __LINE__)
+
+        CHEK("AAAAB" > "AAAAA", __LINE__)
+        CHEK(!("AAAAA" > "AAAAB"), __LINE__)
+
+        CHEK("AAAAA" < "AAAAB", __LINE__)
+        CHEK(!("AAAAB" < "AAAAA"), __LINE__)
+
+        CHEK("AAAAA" >= "AAAAA", __LINE__)
+        CHEK(!("AAAAA" >= "AAAAB"), __LINE__)
+
+        CHEK("AAAAB" >= "AAAAA", __LINE__)
+        CHEK(!("AAAAA" >= "AAAAB"), __LINE__)
+
+        CHEK("AAAAA" <= "AAAAA", __LINE__)
+        CHEK(!("AAAAB" <= "AAAAA"), __LINE__)
+
+        CHEK("AAAAA" <= "AAAAB", __LINE__)
+        CHEK(!("AAAAB" <= "AAAAA"), __LINE__)
+
+        // Strings are interpreted as integers.
+        CHEK("1" && "2", __LINE__)
+        CHEK(!("" && "1"), __LINE__)
+        CHEK(!("1" && ""), __LINE__)
+        CHEK(!("" && ""), __LINE__)
+
+        CHEK("1" || "2", __LINE__)
+        CHEK("" || "1", __LINE__)
+        CHEK("1" || "", __LINE__)
+        CHEK(!("" || ""), __LINE__)
+
+        Return(FCNT)
+    }
+}