Test `copy_object_to_self` fails with varying errors

[martin-hughes]

As discussed in #299, the following ASL does not run reliably (it's from the test copy_object_to_self in uacpi_examples.rs)

DefinitionBlock("", "DSDT", 1, "RSACPI", "UACPI", 1) {
    Method (TEST) {
        CopyObject(123, TEST)
        Return (1)
    }

    // Segfault in ACPICA, prints 1 in uACPI
    Debug = TEST()

    // Unreachable in ACPICA, prints 123 in uACPI
    Debug = TEST
}

It either runs to completion, or terminates with either of the following errors:

Failed(ParseFail(IllegalOpcode(25))) // but with various numbers for the opcode
Failed(ParseFail(RunOutOfStream))

Given that the error seems non-deterministic (AFAICT) I suspect there may be some kind of memory corruption. Sadly I can't get Miri to run because of the file access operations.

[martin-hughes]

Note to anyone looking at this - I don't reliably hit this on Windows without attaching a debugger. On Ubuntu, with the latest nightly Rust, I can hit it without the debugger.

[martin-hughes]

I just remembered the Rust address sanitizer... which spat out a heap use-after-free: SUMMARY: AddressSanitizer: heap-use-after-free. There's a lot of info, but the failing read is in <acpi::aml::MethodContext>::next.

I'm not sure the full output is that useful, but it's attached.

issue-300-output.txt

[martin-hughes]

Speculating here because I feel like I might be missing something, but I think something like this is happening:

• CopyObject called, replaces Method inside the destination WrappedObject with Integer
• Original Method object is dropped, as it has been replaced in the WrappedObject
  • This is somewhat unfortunate, as it's the method we're executing
• MethodContext's Block still holds a raw pointer to the dropped Method's code element, even though it is no longer valid
• Depending on what the memory manager chooses to do with that dropped memory, we see a variety of results

@IsaacWoods does that seem plausible? (Also your thoughts on the below please!)

I can think of two ways to fix this. Probably both are needed to be fully confident there's no edge cases.

1. When doing a copy object op, create a new WrappedObject and insert it into the namespace with the same name, instead of replacing the contents of the destination. I think this will allow the original Method to live on as there is still an extant Arc pointing to it, but no future code will be able to retrieve it from the namespace.

2. In Block, swap out the raw pointer to the code with either a reference to the code (which will lead to a proliferation of lifetime specifiers(!) but is safe) or just copy the code into a Vec (less efficient memory-wise, but easier and safer)