Add safe Memory::read and Memory::write methods #2528
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
from
ffceb5b to
7fac8d1
Compare
github-actions
bot
added
the
wasmtime:api
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
alexcrichton
reviewed
Jan 4, 2021
crates/wasmtime/src/externals.rs
Outdated
| @@ -864,6 +902,38 @@ impl Memory { | |||
| MemoryType::from_wasmtime_memory(&self.wasmtime_export.memory.memory) | |||
| } | |||
|
|
|||
| /// Copies memory contents at the given offset into a buffer. | |||
| /// | |||
| /// Returns an error if the copy failed. | |||
There was a problem hiding this comment.
Could this documentation detail the error cases as well? It also might be good to mention that the size of the copy is the size of the buffer itself.
There was a problem hiding this comment.
I expanded the docs for both read and write a bit.
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
3 times, most recently
from
8aea7ed to
3d10b20
Compare
alexcrichton
reviewed
Jan 4, 2021
|
I implemented your suggestion, the code definitely is more elegant this way. I also added a |
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
from
3d10b20 to
74fbb3c
Compare
This commit introduces two new methods on `Memory` that enable reading and writing memory contents without requiring `unsafe`. The methods return a new `MemoryError` if the memory access fails.
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
from
74fbb3c to
e0e2bc1
Compare
|
Looks great to me, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce
Memory::readandMemory::writemethods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memorydocs.Fixes #2484.
Currently most fallible methods use
anyhow::Error.I have instead added a new
MemoryError, since recovering from such errors is possible and it's more expressive.(future proofed with
#[non_exhaustive]).Could to change it to a
anyhow::Errorif that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Lockedvariant, which is not required yet, but apparently will be once threading support lands. Happy to remove it or slap on a#[doc(hidden)]to prevent confusion.