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

Hackatom: Improve memory management API #2

Closed
ethanfrey opened this issue Oct 4, 2019 · 1 comment
Closed

Hackatom: Improve memory management API #2

ethanfrey opened this issue Oct 4, 2019 · 1 comment

Comments

@ethanfrey
Copy link
Member

ethanfrey commented Oct 4, 2019

We expose allocate and deallocate, which return pointers. This is good to allocate memory so we can set arguments. And we can take a pointer as result and load in a string result. However, there are two issues here:

  1. Minor: we don't include length, so require 0 byte terminated C strings, copying one byte at a time. This causes "nice" crashes when we forget to explicitly add this null byte to some rust data.
  2. Major: If the guest (wasm) calls out to the host (via imports - c_read), it is very hard to call back into the guest. We can now use function tables but it is unclear how to get the index in the first place. Ideally, we do not need to call allocate inside the host function.

Solution:

  1. Don't pass a ptr to memory, but ptr to a Slice (offset, len). Both as a return value from alloc, as well as argument to function calls and free. This lets us clearly specify a size and avoid first problem.
  2. Wasm should never expect a callback to be able to allocate memory. It should pass a pre-allocated buffer to the function where it expects it to be written, the function can return the length written. This is much like old-school C style, which works with very similar constraints as we have. This avoids the callback issue, and also the length guarantess the host function will never write data outside of the pre-allocated buffer.

This describes the general idea the slice structure, intended for use over rust-ffi

@ethanfrey
Copy link
Member Author

Resolved with:
92d25c8 (wasm side)
07b78cf (integration test)

lukerhoads added a commit to lukerhoads/cosmwasm that referenced this issue Jul 29, 2021
webmaster128 added a commit that referenced this issue May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant