Safe Memory Access

[theduke]

Feature

Currently the only way to access instance memory is with data_unchecked[_mut] or data_ptr.

It would be very convenient to have methods that can atomically read or write a range of bytes, with internal locking and safety guarantees provided by the implementation.

Something like:

fn data_read(&self, range: Range<_>, buffer: &mut [u8]) -> Result<(), MemoryAccessError>;
fn data_write(&self, range: Range<_>, source: &[u8]) -> Result<(), MemoryAccessError>;

I'd assume this doesn't exist to prevent the need for internal locking or other implementation constraints and may even be unfeasible, but it would be a very helpful feature.

Benefit

Prevent users from needing unsafe and make wasmtime easier to use.

Implementation

I'm not familiar with the codebase or the complications this would involve, but I'd be happy to contribute if I get some pointers.

[alexcrichton]

Thanks for the report! It should definitely be safe to add these, and no locking necessary at this time even! The main thing is handling long-lived borrows and reentrancy into wasm, but these clearly wouldn't be reentrant, so they should be safe to implement.

Would you be up for making a PR for these?

[pchickey]

Taking both a Range and a slice as arguments mean the length to read/write is specified in both arguments. Instead of specifying the range, specify an offset.

Additionally - there's some amount of overlap between this functionality and what the wiggle family of crates provides. Have you tried using wiggle to abstract over access to wasm memory?

[fitzgen]

While this is currently safe now, as long as no one has a live unsafe { memory.data_unchecked() } borrow active, as soon as we implement threading this will become unsafe because of potential data races.

• Do we expect to just mark it unsafe once we add threading support?

• Or assert that the memory is not shared and panic if it is? We could even do this now, even though it is technically safe to do with a shared memories today since we don't support threading.

  • Also we could check for shared memory and return an error rather than panicking.

I think the check-for-shared-memory-and-return-error is probably the best approach, and it is future compatible.

[alexcrichton]

Ah that's a good point yeah we'd just need an offset rather than a whole Range @pchickey!

@fitzgen I think this could even be safe for shared memories? It's basically just a memory.copy but for the host? It does mean we can't use memcpy for atomic memories (we'd have to do a loop with large-width atomics probably, or maybe even LLVM's atomic memcpy intrinsic if that's ever stabilized), but we should still be able to do whatever the JIT does safely.

[fitzgen]

If we gave it the memory.copy semantics that could work yeah. But yeah we can't memcpy for shared memories. Sounds good to me.