Support reading from 32-bit processes on 64-bit hosts and vice versa

[zed0]

Support setting the architecture of a DataMember to either 32 or 64 bit, defaulting to the pointer size of the host program.
Currently this setting only affects the traversal via get_offset(), but I had to make modifications to copy_address() due to rust-lang/rust#50824

I'm new to rust so let me know if I can improve anything.

[zed0]

For reference I haven't touched the code that Clippy is complaining about, but I can remove it if needed.

[Tommoa]

Hey, thanks for the pull request!

I wouldn't worry too much about the clippy warnings, they didn't exist at the time I last committed code, so that's on me to clean up.

I have a good number of nitpicks to improve the code. You can see the comments in the review.

[Tommoa]

The only place where arch is used, you divide it by 8. I'd recommend not multiplying by 8 here so that you don't have an extra divide every time you try to call get_offset().

[zed0]

Thanks for checking out the PR and the constructive feedback, looking at what I can do to fix the issues.

[zed0]

I've added another commit with most of the changes requested.

The main thing I didn't do was the masking because when I try to take that approach I get

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 299, kind: Other, message: "Only part of a ReadProcessMemory or WriteProcessMemory request was completed." }', src\main.rs:204:21

when trying to use DataMember.read() on a 64-bit Windows host examining a 32-bit process.

I'm not super happy with src/architecture.rs in terms of readability but I couldn't find a nicer way to do it.

[Tommoa]

The main thing I didn't do was the masking because when I try to take that approach I get

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 299, kind: Other, message: "Only part of a ReadProcessMemory or WriteProcessMemory request was completed." }', src\main.rs:204:21

when trying to use DataMember.read() on a 64-bit Windows host examining a 32-bit process.

The error you're talking about will be coming from https://github.com/Tommoa/rs-process-memory/blob/master/src/windows.rs#L72

[Tommoa]

Looks pretty good, you just need to run rustfmt on architecture.rs!

If possible, would you be able to put documentation examples of set_arch()? I think that you might be able to get it working on x64 with 32-bit pointers as the stack usually grows from the bottom of the address space (unless ASLR messes with that, but I'm not sure).

[zed0]

Updated, thanks for all your comments. Hope the example is ok.

[zed0]

Ah, I didn't realise that it would actually try to compile the examples. I'm not sure of a sensible way to demonstrate the feature within the same process.

[zed0]

Read up on how the documentation examples worked and I've added no_run to the ones I've added.

[Tommoa]

Looks great! Just one final thing that I missed in the last review.

Looking at the code examples has actually got me wondering about whether it'd be better to move the architecture stuff from DataMember<> to ProcessHandle. At the moment, ProcessHandle is just an alias to a type, which is a pid_t on Linux, a mach_port_name_t and a HANDLE on Windows. Perhaps by changing it to a struct that contains the handle and the architecture it'd make the API a little easier to work with, allowing you to define the architecture on the process handle itself (after all, you won't have different sized pointers within the same program unless someone is using relative pointers). Perhaps I'll work on that after this PR is merged!

[Tommoa]

This can also be a slice (&[u8]). You'd just need to remove all the as_slice()s down the bottom.

[zed0]

Sorted. 👍

[Tommoa]

Looks good! I'll do a rustfmt after merging.

[zed0]

Thanks!
Sorry about the rustfmt, need to configure my editor if I'm going to keep working on Windows.