Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API for reading/writing binary data from/to Wasm memory #2035

Closed
webmaster128 opened this issue Jan 21, 2021 · 9 comments
Closed

API for reading/writing binary data from/to Wasm memory #2035

webmaster128 opened this issue Jan 21, 2021 · 9 comments
Assignees
Labels
🎉 enhancement New feature! 📦 lib-api About wasmer priority-medium Medium priority issue 🏚 stale Inactive issues or PR 🚧 work in progress

Comments

@webmaster128
Copy link
Contributor

webmaster128 commented Jan 21, 2021

Motivation

Reading raw binary data from Wasm memory and writing binary data to Wasm memory seems to be common use case of WasmPtr. Right now there is no clean API to do that without looping through a slice of Cell<u8>. Users have the desire (1, 2) to replace this loop with a more efficient way to copy chunks of memory, like std::ptr::copy or std::ptr::copy_nonoverlapping.

Right now, users have two choices after calling WasmPtr::<u8, Array>::new(..)deref():

  1. Iterate over the given&[Cell<u8>] for reading and writing
  2. Reason about the memory layout of Cell and slice, writing unsafe code that might work but is hard to understand and maintain. It took me more than an hour to convince myself what the layout of &[Cell<u8>] looks like and it included reading Rust std source code.

Proposed solution

Add an API to WasmPtr that allows reading raw binary data from Wasm memory and an API that allows writing raw binary data to Wasm memory. This can be similar to WasmPtr::get_utf8_str and probably share 9 of its 10 line implementation.

This API can be unsafe but should take away as much of the burden from the user as possible.

Alternatives

Not aware of any. Happy for input.

Additional context

This is us struggeling to go from choice 1 to choice 2 with our heads 🤯. I'm now convinced that &[Cell<u8>] has the same layout as &[u8] but I don't want future readers of our code to go through this again (including me).

Bildschirmfoto 2021-01-21 um 17 20 10

@Hywan
Copy link
Contributor

Hywan commented Jul 16, 2021

I believe it's somehow addressed by #2442. Is it enough for you or do you need more?

@Hywan Hywan self-assigned this Jul 16, 2021
@Hywan Hywan added the 📦 lib-api About wasmer label Jul 16, 2021
@webmaster128
Copy link
Contributor Author

webmaster128 commented Jul 16, 2021

I never worked with MemoryView before, but looking at the PR it seems that

  • Only copy from host to guest is supported via MemoryView::copy_from, not guest to host.
  • This copy is performed byte by byte instead of using some kind of memcopy. If this is the case, is it better than copying cell by cell?

But I guess I missed something here.

@Hywan
Copy link
Contributor

Hywan commented Jul 19, 2021

You're correct. We will provide a better API to solve your needs :-).

@webmaster128
Copy link
Contributor Author

Hey there! Great to see this moving in the board. The ticket was moved to Ready to Merge and 🎉 Done but there is not PR linked. Is it done and can be closed?

@Amanieu
Copy link
Contributor

Amanieu commented May 4, 2022

The API for accessing memory was redone in #2646 but since this is a breaking change we are going to release it as part of Wasmer 3.0. We also have a bunch of other changes coming up which will improve the API.

@webmaster128
Copy link
Contributor Author

Great, thanks. Looking forward to it.

@webmaster128
Copy link
Contributor Author

Do I understand correctly this ticket is done by introducing WasmSlice in Wasmer 3.x?

@stale
Copy link

stale bot commented Oct 28, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the 🏚 stale Inactive issues or PR label Oct 28, 2023
@webmaster128
Copy link
Contributor Author

webmaster128 commented Oct 28, 2023

wasmer::MemoryView::read and wasmer::MemoryView::write implemented via volatile_memcpy_read and volatile_memcpy_write do this in recent versions of Wasmer. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-api About wasmer priority-medium Medium priority issue 🏚 stale Inactive issues or PR 🚧 work in progress
Projects
None yet
Development

No branches or pull requests

4 participants