Handle non-utf8 hosts in the adapter #8749
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
This commit is a fix to the WASIp1 adapter for components to better handle the case where the host does not use a utf-8 string encoding. This is never the case for `wasmtime`-the-crate since it's a Rust-based host but this adapter is used outside of Wasmtime in jco, for example, where JS is not utf-8-based. When transcoding from utf-16 to utf-8 hosts may make an overlarge allocation and then shrink to a smaller allocation. This shrinking step has never been supported by the adapter and it's always aborted in this case. Aside: why is this only a problem now? This hasn't been an issue before now because jco bindings never actually shrank. In doing so however this violated the canonical ABI because allocations are guaranteed to be precisely sized. New debug assertions in newer versions of Rust caught this mistake. This means that when jco tried to add downsizing of the allocation it quickly hit this panic in the adapter. The fix in this commit is to handle the specific case of shrinking memory. The specific fix is to simply ignore the shrinking of memory. This is pretty subtle though why it seems to work out well enough for now (and it's probably still buggy). For now though this is enough to get jco's test suite passing with a shrinking allocation. Unfortunately I don't know of a way to test this in this repository. Wasmtime does not support multiple encodings of host strings, only guest strings. This means that there's no wasmtime-based way to pass a non-utf-8 string into a guest.
|
//cc @kateinoigakukun |
pchickey
approved these changes
Jun 5, 2024
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.
This commit is a fix to the WASIp1 adapter for components to better handle the case where the host does not use a utf-8 string encoding. This is never the case for
wasmtime-the-crate since it's a Rust-based host but this adapter is used outside of Wasmtime in jco, for example, where JS is not utf-8-based. When transcoding from utf-16 to utf-8 hosts may make an overlarge allocation and then shrink to a smaller allocation. This shrinking step has never been supported by the adapter and it's always aborted in this case.Aside: why is this only a problem now? This hasn't been an issue before now because jco bindings never actually shrank. In doing so however this violated the canonical ABI because allocations are guaranteed to be precisely sized. New debug assertions in newer versions of Rust caught this mistake. This means that when jco tried to add downsizing of the allocation it quickly hit this panic in the adapter.
The fix in this commit is to handle the specific case of shrinking memory. The specific fix is to simply ignore the shrinking of memory. This is pretty subtle though why it seems to work out well enough for now (and it's probably still buggy). For now though this is enough to get jco's test suite passing with a shrinking allocation.
Unfortunately I don't know of a way to test this in this repository. Wasmtime does not support multiple encodings of host strings, only guest strings. This means that there's no wasmtime-based way to pass a non-utf-8 string into a guest.