diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 2e043463..4687ecbf 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -2381,7 +2381,11 @@ where /// - Locals are overwritten, unless they contain a reference, 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 + /// object is usually overwritten. References from Arg to Local without an intermediate + /// `RefOf` cause the Arg to be overwritten. + /// (see [issue #313](https://github.com/rust-osdev/acpi/issues/313)) + /// - Args that ultimately refer to a string *always* overwrite the string and not the Arg, + /// as per the Windows NT behaviour (see `tests/store.asl`) /// - Index references behave the same as locals /// - Named objects are stored into, with implicit casting fn do_store(&self, target: WrappedObject, object: WrappedObject) -> Result { @@ -2389,29 +2393,10 @@ where 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 bfc03b41..923f8b86 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -154,6 +154,63 @@ 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 fn unwrap_ref_for_store(self) -> Result<(WrappedObject, bool), AmlError> { + let Object::Reference { .. } = *self else { + return Err(AmlError::ObjectNotOfExpectedType { expected: ObjectType::Reference, got: self.typ() }); + }; + + let mut target = self; + let mut implicit_cast_reqd = false; + + // Unwrap references, but with the following caveats: + // - If an Arg -> Local reference is found, we return the Arg so that it can be stored in. + // Except... + // - The Windows NT interpreter allows strings stored in locals that are then passed as args + // to be modified even if they aren't passed by reference... so we must continue + // unwrapping to see if the end of the reference chain is a String or not. If it is, + // return that instead (a bit like a normal `unwrap_reference`) + // + // See issue 313 and the `store.asl` tests for more details. + let mut found_arg_to_local: Option> = None; + + loop { + let Object::Reference { kind, ref inner } = *target else { + if target.typ() == ObjectType::String { + return Ok((target.clone(), true)); + } + return found_arg_to_local.unwrap_or_else(|| Ok((target.clone(), implicit_cast_reqd))); + }; + + implicit_cast_reqd = match kind { + ReferenceKind::Named => true, + ReferenceKind::Local | ReferenceKind::Index | ReferenceKind::RefOf => false, + ReferenceKind::Arg => { + if found_arg_to_local.is_none() + && matches!(**inner, Object::Reference { kind: ReferenceKind::Local, inner: _ }) + { + found_arg_to_local = Some(Ok((inner.clone(), implicit_cast_reqd))); + } + false + } + ReferenceKind::Unresolved => { + if found_arg_to_local.is_none() { + found_arg_to_local = Some(Err(AmlError::StoreToInvalidReferenceType)); + } + implicit_cast_reqd + } + }; + + target = inner.clone(); + } + } } impl ops::Deref for WrappedObject { @@ -653,4 +710,60 @@ 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); + } } diff --git a/tests/store.asl b/tests/store.asl new file mode 100644 index 00000000..f0bf0bc8 --- /dev/null +++ b/tests/store.asl @@ -0,0 +1,122 @@ +// Check that store handles simple references correctly +// +// 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", 1, "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) + } + + // Test 8 is adapted from uACPI's `references-3.asl`. To quote that test: + // "This test seems bogus but it's actually correct, it produces the same output on NT." + Method (INR8, 1, NotSerialized) + { + Local0 = RefOf(Arg0) + + // WHY? in little-endian ASCII + Local0 = 0x3F594857 + } + + Method (T8) + { + Local0 = "MyST" + INR8(Local0) + CHEK(Local0, "WHY?") + } + + // This is the same as `T8` 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 (T8A) { + Local0 = "MyST" + IN8A(Local0) + CHEK(Local0, "WHY?") + } + + Method (IN8A, 1, NotSerialized) { + INR8(Arg0) + } + + // Test 8 not withstanding, non-string "pass by value" argument types show the expected behavior. + Method (INR9, 1) { + Local0 = RefOf(Arg0) + Local0 = 9 + } + + Method (T9) { + Local0 = 1 + INR9(Local0) + CHEK(Local0, 1) + } + + Method (MAIN, 0, NotSerialized) { + T1() + T2() + T3() + T4() + T5() + T6() + T7(0) + T8() + T8A() + T9() + + Return (FCNT) + } +}