aml: make InvalidOperationOnObject error more useful

Author: IsaacWoods

aml/src/lib.rs

@@ -1120,13 +1120,14 @@ where
         };
 
         let result = Arc::new(Object::Integer(result));
+        // TODO: use result for arg
         self.do_store(context, target, result.clone())?;
         context.contribute_arg(Argument::Object(result));
         Ok(())
     }
 
     fn do_unary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> {
-        let [Argument::Object(operand)] = &op.arguments[..] else { Err(AmlError::InvalidOperationOnObject)? };
+        let [Argument::Object(operand)] = &op.arguments[..] else { panic!() };
         let operand = operand.clone().unwrap_transparent_reference().as_integer()?;
 
         let result = match op.op {
@@ -1137,6 +1138,7 @@ where
                     /*
                      * TODO: this is a particular instance where not respecting integers being
                      * 32-bit on revision 1 tables does cause properly incorrect behaviour...
+                     * TODO: we can fix this now we have the DSDT revision
                      */
                     (operand.leading_zeros() + 1) as u64
                 }
@@ -1164,7 +1166,7 @@ where
 
     fn do_logical_op(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> {
         if op.op == Opcode::LNot {
-            let [Argument::Object(operand)] = &op.arguments[..] else { Err(AmlError::InvalidOperationOnObject)? };
+            let [Argument::Object(operand)] = &op.arguments[..] else { panic!() };
             let operand = operand.clone().unwrap_transparent_reference().as_integer()?;
             let result = if operand == 0 { u64::MAX } else { 0 };
 
@@ -1177,9 +1179,7 @@ where
             return Ok(());
         }
 
-        let [Argument::Object(left), Argument::Object(right)] = &op.arguments[..] else {
-            Err(AmlError::InvalidOperationOnObject)?
-        };
+        let [Argument::Object(left), Argument::Object(right)] = &op.arguments[..] else { panic!() };
 
         /*
          * Some of these operations allow strings and buffers to be used as operands. Apparently
@@ -1220,7 +1220,7 @@ where
                 };
                 (left, right)
             }
-            _ => panic!(),
+            _ => Err(AmlError::InvalidOperationOnObject { op: Operation::LogicalOp, typ: left.typ() })?,
         };
 
         let result = match op.op {
@@ -1268,7 +1268,7 @@ where
                     Object::Buffer(bytes.to_vec())
                 }
             }
-            _ => Err(AmlError::InvalidOperationOnObject)?,
+            _ => Err(AmlError::InvalidOperationOnObject { op: Operation::Mid, typ: source.typ() })?,
         });
 
         self.do_store(context, target, result.clone())?;
@@ -1334,7 +1334,7 @@ where
     }
 
     fn do_from_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> {
-        let [Argument::Object(value)] = &op.arguments[..] else { Err(AmlError::InvalidOperationOnObject)? };
+        let [Argument::Object(value)] = &op.arguments[..] else { panic!() };
         let mut value = value.clone().unwrap_transparent_reference().as_integer()?;
 
         let mut result = 0;
@@ -1350,7 +1350,7 @@ where
     }
 
     fn do_to_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> {
-        let [Argument::Object(value)] = &op.arguments[..] else { Err(AmlError::InvalidOperationOnObject)? };
+        let [Argument::Object(value)] = &op.arguments[..] else { panic!() };
         let mut value = value.clone().unwrap_transparent_reference().as_integer()?;
 
         let mut result = 0;
@@ -1366,14 +1366,14 @@ where
     }
 
     fn do_size_of(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> {
-        let [Argument::Object(object)] = &op.arguments[..] else { Err(AmlError::InvalidOperationOnObject)? };
+        let [Argument::Object(object)] = &op.arguments[..] else { panic!() };
         let object = object.clone().unwrap_reference();
 
         let result = match *object {
             Object::Buffer(ref buffer) => buffer.len(),
             Object::String(ref str) => str.len(),
             Object::Package(ref package) => package.len(),
-            _ => Err(AmlError::InvalidOperationOnObject)?,
+            _ => Err(AmlError::InvalidOperationOnObject { op: Operation::SizeOf, typ: object.typ() })?,
         };
 
         context.contribute_arg(Argument::Object(Arc::new(Object::Integer(result as u64))));
@@ -2171,6 +2171,21 @@ enum Opcode {
     InternalMethodCall,
 }
 
+#[derive(Clone, Copy, PartialEq, Debug)]
+pub enum Operation {
+    Mid,
+    SizeOf,
+    Acquire,
+    Release,
+    ConvertToBuffer,
+
+    ReadBufferField,
+    WriteBufferField,
+    LogicalOp,
+    DecodePrt,
+    ParseResource,
+}
+
 /*
  * TODO: not sure if we should use a better error reporting system or just keep a giant enum?
  */
@@ -2197,7 +2212,7 @@ pub enum AmlError {
 
     MethodArgCountIncorrect,
 
-    InvalidOperationOnObject,
+    InvalidOperationOnObject { op: Operation, typ: ObjectType },
     IndexOutOfBounds,
     ObjectNotOfExpectedType { expected: ObjectType, got: ObjectType },
 

aml/src/object.rs

@@ -1,4 +1,4 @@
-use crate::{AmlError, op_region::OpRegion};
+use crate::{AmlError, Handle, Operation, op_region::OpRegion};
 use alloc::{borrow::Cow, string::String, sync::Arc, vec::Vec};
 use bit_field::BitField;
 
@@ -92,7 +92,7 @@ impl Object {
                 _ => panic!(),
             },
             Object::String(value) => Ok(value.as_bytes().to_vec()),
-            _ => Err(AmlError::InvalidOperationOnObject),
+            _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ConvertToBuffer, typ: self.typ() }),
         }
     }
 
@@ -107,7 +107,7 @@ impl Object {
             copy_bits(buffer, *offset, dst, 0, *length);
             Ok(())
         } else {
-            Err(AmlError::InvalidOperationOnObject)
+            Err(AmlError::InvalidOperationOnObject { op: Operation::ReadBufferField, typ: self.typ() })
         }
     }
 
@@ -124,7 +124,7 @@ impl Object {
             copy_bits(value, 0, buffer, *offset, *length);
             Ok(())
         } else {
-            Err(AmlError::InvalidOperationOnObject)
+            Err(AmlError::InvalidOperationOnObject { op: Operation::WriteBufferField, typ: self.typ() })
         }
     }
 

aml/src/pci_routing.rs

@@ -2,6 +2,7 @@ use crate::{
     AmlError,
     Handler,
     Interpreter,
+    Operation,
     namespace::AmlName,
     object::{Object, ReferenceKind},
     resource::{self, InterruptPolarity, InterruptTrigger, Resource},
@@ -136,13 +137,13 @@ impl PciRoutingTable {
                         _ => return Err(AmlError::PrtInvalidSource),
                     }
                 } else {
-                    return Err(AmlError::InvalidOperationOnObject);
+                    return Err(AmlError::InvalidOperationOnObject { op: Operation::DecodePrt, typ: value.typ() });
                 }
             }
 
             Ok(PciRoutingTable { entries })
         } else {
-            Err(AmlError::InvalidOperationOnObject)
+            return Err(AmlError::InvalidOperationOnObject { op: Operation::DecodePrt, typ: prt.typ() });
         }
     }
 

aml/src/resource.rs

@@ -1,4 +1,4 @@
-use crate::{AmlError, object::Object};
+use crate::{AmlError, Operation, object::Object};
 use alloc::{sync::Arc, vec::Vec};
 use bit_field::BitField;
 use byteorder::{ByteOrder, LittleEndian};
@@ -32,7 +32,7 @@ pub fn resource_descriptor_list(descriptor: Arc<Object>) -> Result<Vec<Resource>
 
         Ok(descriptors)
     } else {
-        Err(AmlError::InvalidOperationOnObject)
+        Err(AmlError::InvalidOperationOnObject { op: Operation::ParseResource, typ: descriptor.typ() })
     }
 }