Stream: git-wasmtime

Topic: wasmtime / Issue #2484 Safe Memory Access

Wasmtime GitHub notifications bot (Dec 07 2020 at 15:47):

theduke opened Issue #2484:

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<u8>, buffer: &mut [u8]) -> Result<(), MemoryAccessError>;
fn data_write(&self, range: Range<u8>, source: &[u8]) -> Result<(), MemoryAccessError>;

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

Benefit

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

Implementation

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

Wasmtime GitHub notifications bot (Dec 07 2020 at 15:48):

theduke edited Issue #2484:

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, but it would be a very helpful feature.

Benefit

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

Implementation

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

Wasmtime GitHub notifications bot (Dec 07 2020 at 15:48):

theduke edited Issue #2484:

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, 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, but I'd be happy to contribute if I get some pointers.

Wasmtime GitHub notifications bot (Dec 07 2020 at 15:49):

theduke edited Issue #2484:

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, 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.

Wasmtime GitHub notifications bot (Dec 07 2020 at 15:51):

theduke edited Issue #2484:

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.

Wasmtime GitHub notifications bot (Dec 07 2020 at 16:02):

alexcrichton commented on Issue #2484:

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?

Wasmtime GitHub notifications bot (Dec 07 2020 at 18:10):

pchickey commented on Issue #2484:

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?

Wasmtime GitHub notifications bot (Dec 07 2020 at 19:16):

fitzgen commented on Issue #2484:

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.

Wasmtime GitHub notifications bot (Dec 07 2020 at 20:02):

alexcrichton commented on Issue #2484:

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.

Wasmtime GitHub notifications bot (Dec 07 2020 at 20:05):

fitzgen commented on Issue #2484:

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.

Wasmtime GitHub notifications bot (Jan 26 2021 at 15:09):

alexcrichton closed Issue #2484:

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.

Last updated: Dec 23 2024 at 13:07 UTC