diff --git a/src/aml/mod.rs b/src/aml/mod.rs index cb3af956..9ead67eb 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -498,7 +498,7 @@ where } Opcode::Increment | Opcode::Decrement => { let [Argument::Object(operand)] = &op.arguments[..] else { panic!() }; - let operand = operand.clone().unwrap_transparent_reference(); + let operand = operand.clone().unwrap_reference(); let token = self.object_token.lock(); let Object::Integer(operand) = (unsafe { operand.gain_mut(&token) }) else { @@ -2393,40 +2393,32 @@ where /// Perform a store of `object` into `target`, matching the expected behaviour of `DefStore`, /// which depends on the target: - /// - Locals are overwritten, unless they contain a reference, in which case a store is - /// performed to the referenced object with implicit casting + /// - Locals and Index references are overwritten, unless the Local contains a reference + /// (e.g. obtained via `RefOf`), in which case a store is performed to the referenced + /// object with implicit casting. /// - Args are overwritten, unless they contain a reference, in which case the referenced - /// object is overwritten - /// - Index references behave the same as locals + /// object is *usually* overwritten. References from Arg to Local without an intermediate + /// `RefOf` cause the Arg to be overwritten instead of the Local (see + /// [issue #313](https://github.com/rust-osdev/acpi/issues/313)). + /// - There is an exception to this, needed to match the Windows NT interpreter: if an Arg + /// referencing a Local is found whilst unwrapping a real reference (e.g. a Local holding + /// `RefOf(Arg0)`, where `Arg0` was itself passed a `Local` directly) and the final result + /// of unwrapping is a string, the string is modified instead of the Arg. This does not + /// apply if the Arg is the direct target of the store and only applies to Strings. (This + /// sounds odd, but is the Windows way) /// - Named objects are stored into, with implicit casting + /// + /// This is complex so may be explained better by `tests/store.asl` and + /// [`Object::unwrap_ref_for_store`]! fn do_store(&self, target: WrappedObject, object: WrappedObject) -> Result { let object = object.unwrap_transparent_reference(); let token = self.object_token.lock(); match unsafe { target.gain_mut(&token) } { - Object::Reference { kind, inner } => { - let (target_object, overwrite) = match kind { - ReferenceKind::Named => (inner.clone().unwrap_reference(), false), - ReferenceKind::Local | ReferenceKind::Index => { - if let Object::Reference { kind: _, inner: ref inner_inner } = **inner { - (inner_inner.clone(), false) - } else { - (inner.clone().unwrap_transparent_reference(), true) - } - } - ReferenceKind::Arg => { - if let Object::Reference { kind: _, inner: ref inner_inner } = **inner { - (inner_inner.clone(), true) - } else { - (inner.clone().unwrap_transparent_reference(), true) - } - } - ReferenceKind::RefOf | ReferenceKind::Unresolved => { - return Err(AmlError::StoreToInvalidReferenceType); - } - }; + Object::Reference { .. } => { + let (target_object, implicit_cast_reqd) = target.unwrap_ref_for_store()?; - if overwrite { + if !implicit_cast_reqd { unsafe { *target_object.gain_mut(&token) = (*object).clone(); } diff --git a/src/aml/object.rs b/src/aml/object.rs index 8ac820d1..ebaca18b 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -154,6 +154,61 @@ impl WrappedObject { } } } + + /// Unwrap a reference that is about to be stored to - find the target object. + /// + /// Take into account the store rules as enumerated by `Interpreter::do_store` + /// + /// Returns a tuple containing: + /// - The object that should be modified + /// - A boolean indicating whether an implicit cast should occur before the store + pub(crate) fn unwrap_ref_for_store(self) -> Result<(WrappedObject, bool), AmlError> { + let Object::Reference { kind: outer_kind, .. } = *self else { + return Err(AmlError::ObjectNotOfExpectedType { expected: ObjectType::Reference, got: self.typ() }); + }; + + // If we're storing directly to an Arg then the strange Windows NT behaviour for strings + // doesn't apply. (see the comments in `Interpreter::do_store` for more) + let store_to_arg = outer_kind == ReferenceKind::Arg; + + // Set once we've unwrapped through a true reference. This helps decide whether the target + // should be overwritten in full, or if an implicit cast to the target type is needed. + let mut crossed_true_reference = false; + + // If we reach an Arg that directly references a Local, we should store in the Arg instead + // of the Local (see [issue #313](https://github.com/rust-osdev/acpi/issues/313)). Except + // for the strange Windows NT behaviour handled in the `match arg_to_local` statement below. + let mut arg_to_local: Option = None; + + let mut target = self; + + loop { + let Object::Reference { kind, ref inner } = *target else { + return Ok(match arg_to_local { + Some(_) if !store_to_arg && target.typ() == ObjectType::String => (target.clone(), true), + Some(arg) => (arg, false), + None => (target.clone(), !store_to_arg && crossed_true_reference), + }); + }; + + match kind { + ReferenceKind::Named | ReferenceKind::RefOf | ReferenceKind::Index => { + crossed_true_reference = true + } + ReferenceKind::Local => {} + ReferenceKind::Arg => { + if arg_to_local.is_none() + && matches!(**inner, Object::Reference { kind: ReferenceKind::Local, .. }) + { + arg_to_local = Some(inner.clone()); + } + } + ReferenceKind::Unresolved => return Err(AmlError::StoreToInvalidReferenceType), + } + + target = inner.clone(); + } + } } impl ops::Deref for WrappedObject { @@ -653,4 +708,76 @@ mod tests { assert_eq!(buffer_field.to_integer(IntegerSize::EightBytes).unwrap(), 0x0000000f_00000000); } + + #[test] + fn store_local_ref_to_local() { + // As may be encountered in the last line of: + // Local1 = RefOf(Local0) + // Local1 = 2 (the actual store is omitted) + let local0 = Object::Reference { kind: ReferenceKind::Local, inner: Object::Integer(1).wrap() }.wrap(); + let ref_of = Object::Reference { kind: ReferenceKind::RefOf, inner: local0 }.wrap(); + let local1 = Object::Reference { kind: ReferenceKind::Local, inner: ref_of }.wrap(); + + let target = local1.unwrap_ref_for_store(); + let target = target.unwrap(); + + let target_obj = &*target.0; + let Object::Integer(x) = target_obj else { + panic!("Incorrect type"); + }; + assert_eq!(*x, 1); + } + + #[test] + fn store_arg_ref_to_local() { + // As if a Local was passed as an argument to a method, and then Arg0 were stored to e.g.: + // Local0 = 1 + // MEFD(Local0) + // ... and inside MEFD: Arg0 = 2 (the actual store is omitted) + let local0 = Object::Reference { kind: ReferenceKind::Local, inner: Object::Integer(1).wrap() }.wrap(); + let arg0 = Object::Reference { kind: ReferenceKind::Arg, inner: local0.clone() }.wrap(); + + let target = arg0.unwrap_ref_for_store(); + let (target, implicit_cast_reqd) = target.unwrap(); + + assert!(Arc::ptr_eq(&target.0, &local0.0)); + assert!(!implicit_cast_reqd); + } + + #[test] + fn store_arg_ref_of_local() { + // As may be encountered in the last line of: + // Local0 = 1 + // Arg0 = RefOf(Local0) + // Arg0 = 2 (the actual store is omitted) + let local0 = Object::Reference { kind: ReferenceKind::Local, inner: Object::Integer(1).wrap() }.wrap(); + let ref_of = Object::Reference { kind: ReferenceKind::RefOf, inner: local0 }.wrap(); + let arg0 = Object::Reference { kind: ReferenceKind::Arg, inner: ref_of }.wrap(); + + let target = arg0.unwrap_ref_for_store(); + let (target, implicit_cast_reqd) = target.unwrap(); + + let target_obj = &*target; + let Object::Integer(x) = target_obj else { + panic!("Incorrect type"); + }; + assert_eq!(*x, 1); + assert!(!implicit_cast_reqd); + } + + #[test] + fn store_local_refof_arg_to_local_string() { + // This covers the weird Windows NT handling of references that ultimately end up as Strings + let string = Object::String("string".to_string()).wrap(); + let outer_local = Object::Reference { kind: ReferenceKind::Local, inner: string.clone() }.wrap(); + let arg0 = Object::Reference { kind: ReferenceKind::Arg, inner: outer_local }.wrap(); + let ref_of = Object::Reference { kind: ReferenceKind::RefOf, inner: arg0 }.wrap(); + let inner_local = Object::Reference { kind: ReferenceKind::Local, inner: ref_of }.wrap(); + + let target = inner_local.unwrap_ref_for_store(); + let (target, implicit_cast_reqd) = target.unwrap(); + + assert!(Arc::ptr_eq(&target.0, &string.0)); + assert!(implicit_cast_reqd); + } } diff --git a/tests/store.asl b/tests/store.asl new file mode 100644 index 00000000..bbff1545 --- /dev/null +++ b/tests/store.asl @@ -0,0 +1,186 @@ +// Check that store handles simple references correctly +// +// Tests with a name that begins with a T are written by us for this crate. Tests with names that begin with a U have +// been adapted from the uACPI test case named in the associated comment. +// +// Tests T1 - T4 are very basic, to ensure any trivial errors in `do_store` are caught. +// +// These tests don't check any conversions - it's assumed that references and conversions are orthogonal. +DefinitionBlock ("", "DSDT", 2, "RSACPI", "TESTTABL", 0xF0F0F0F0) +{ + Name(FCNT, 0) + + Method (CHEK, 2) { + If (Arg0 != Arg1) { + FCNT++ + } + } + + Method (T1) { + Name(V1, 1) + V1 = 2 + CHEK(V1, 2) + } + + Method (T2) { + Name(V1, 1) + Alias(V1, V2) + V2 = 2 + CHEK(V1, 2) + } + + Method (T3) { + Local1 = 1 + Local2 = Local1 + Local2 = 2 + CHEK(Local1, 1) + } + + Method (T4) { + Local1 = 1 + Local2 = RefOf(Local1) + Local2 = 2 + CHEK(Local1, 2) + } + + Method (INR5, 1) { + Arg0 = 5 + } + + Method (T5) { + Local1 = 1 + INR5(Local1) + CHEK (Local1, 1) + } + + Method (T6) { + Local1 = 1 + INR5(RefOf(Local1)) + CHEK (Local1, 5) + } + + Method (T7, 1) { + Local1 = 1 + Arg0 = RefOf(Local1) + Arg0 = 2 + CHEK (Local1, 2) + } + + // uACPI equivalent: references-0.asl + Method (U1) { + Local0 = "MyString" + INR5(Local0) // Arg0 = 5 + CHEK (Local0, "MyString") + } + + // uACPI equivalent: references-3.asl. To quote that test: + // "This test seems bogus but it's actually correct, it produces the same output on NT." + Method (U2) + { + Local0 = "MyST" + U2IN(Local0) // Local0 = RefOf(Arg0) + CHEK(Local0, "WHY?") + } + + Method (U2IN, 1, NotSerialized) + { + Local0 = RefOf(Arg0) + + // WHY? in little-endian ASCII + Local0 = 0x3F594857 + } + + // This is the same as `U2` but with an extra function call to see if the Windows behaviour is + // limited to one level of the stack - but it is not, multiple calls behave the same as a + // single call. + Method (U2A) { + Local0 = "MyST" + IN2A(Local0) + CHEK(Local0, "WHY?") + } + + Method (IN2A, 1, NotSerialized) { + U2IN(Arg0) + } + + // uACPI test equivalent: references-4 + // Test U2 not withstanding, non-string "pass by value" argument types show the expected behavior. + Method (U3) { + Local0 = 1 + U3IN(Local0) + CHEK(Local0, 1) + } + + Method (U3IN, 1) { + Local0 = RefOf(Arg0) + Local0 = 9 + } + + // uACPI equivalent: references-8 + METHOD(U4) { + Local0 = "MyString" + U4IN(RefOf(Local0)) + CHEK(Local0, 0xDEADBEEF) + } + + Method (U4IN, 1, NotSerialized) + { + Store(0xDEADC0DE, Arg0) + Store(0xDEADBEEF, Arg0) + } + + // uACPI test equivalent: references-9 + Method (U5, 0, NotSerialized) + { + Local0 = 0xDEADC0DEDEADBEEF + U5IN(RefOf(Local0)) + CHEK (Local0, 0x676E6F6C79726576) + } + + Method (U5IN, 1, NotSerialized) + { + Local0 = RefOf(Arg0) + Local0 = "verylongstringbiggerthanint" + } + + // uACPI test equivalent: references-10 + Method (U6, 0, NotSerialized) + { + Local0 = 0x1000 + U6IN(RefOf(Local0), 10) + CHEK (Local0, 0x100A) + } + + Method (U6IN, 2, NotSerialized) + { + Local0 = RefOf(Arg0) + Local0++ + + Debug = Arg1 + Debug = DerefOf(Local0) + If (Arg1--) { + U6IN(RefOf(Arg0), Arg1) + } + } + + Method (MAIN, 0, NotSerialized) { + T1() + T2() + T3() + T4() + T5() + T6() + T7(0) + + // uACPI equivalents given in the comments: + U1() // references-0 + U2() // references-3 + U2A() // An extra test with another layer of indirection + U3() // references-4 + U4() // references-8 + U5() // references-9 + U6() // references-10 + + Return (FCNT) + } +} diff --git a/tests/uacpi_examples.rs b/tests/uacpi_examples.rs index 128fe30f..15b40909 100644 --- a/tests/uacpi_examples.rs +++ b/tests/uacpi_examples.rs @@ -72,7 +72,6 @@ DefinitionBlock("", "DSDT", 1, "RSACPI", "UACPI", 1) { } #[test] -#[ignore] // ParseFail(ObjectNotOfExpectedType { expected: Integer, got: Integer } (a referencing failure) fn increment_decrement() { const ASL: &str = r#" DefinitionBlock("", "DSDT", 1, "RSACPI", "UACPI", 1) {