Add a "custom" platform configuration for Wasmtime #7995
Conversation
github-actions
bot
added
the
wasmtime:docs
| * with `wasmtime_memory_image_free` and `wasmtime_memory_image_map_at` | ||
| * will be used to map the image into new regions of the address space. | ||
| */ | ||
| extern struct wasmtime_memory_image *wasmtime_memory_image_new(const uint8_t *ptr, uintptr_t len); |
There was a problem hiding this comment.
Is ptr guaranteed to stay valid until the memory image is freed or does the implementation need to copy the data elsewhere?
There was a problem hiding this comment.
Good question, I'll expand the docs to indicate that a copy is required.
| */ | ||
| extern void wasmtime_memory_image_remap_zeros(struct wasmtime_memory_image *image, | ||
| uint8_t *addr, | ||
| uintptr_t len); |
There was a problem hiding this comment.
Why does this need access to the memory image? It only modifies the VM mapping, right? And doesn't need to read the original content of the memory image.
There was a problem hiding this comment.
I was mostly just reflecting the existing Wasmtime API, which has not been meticulously thought out, onto the C API. You're right though that this entire function I think is not necessary, and I've replaced it with a call to wasmtime_mmap_remap.
| /** | ||
| * Deallocates the provided `wasmtime_memory_image`. | ||
| */ | ||
| extern void wasmtime_memory_image_free(struct wasmtime_memory_image *image); |
There was a problem hiding this comment.
Are all derived VM mappings guaranteed to be unmapped before calling this?
There was a problem hiding this comment.
Not currently, no, I'll expand the documentation.
examples/min-platform/src/main.rs
Outdated
| } | ||
|
|
||
| match sym.name()? { | ||
| "" | "memmove" | "memset" | "memcmp" | "memcpy" | "bcmp" | "__tls_get_addr" => {} |
There was a problem hiding this comment.
What is the empty symbol used for? Is it for the STT_FILE symbols? If so skipping SymbolKind::File above makes more sense I think.
There was a problem hiding this comment.
Ah I didn't question it much, but turns out it's SymbolKind::Null so I filtered that out to remove this case.
| /// If this function returns then the trap was not handled. This probably means | ||
| /// that a fatal exception happened and the process should be aborted. |
There was a problem hiding this comment.
Can we remove "probably" here? If I were attempting to implement this API, I wouldn't know what to do with that.
There was a problem hiding this comment.
Sort of, but also sort of not. I've expanded the words here to describe that the meaning of an unhandled trap is embedder-specific. Basically if Wasmtime doesn't handle a SIGSEGV, for example, it's up to the application of what to do next. Wasmtime can't know whether that is surely an "abort the process" kind of exception.
| /// Abstract pointer type used in the `wasmtime_memory_image_*` APIs which | ||
| /// is defined by the embedder. | ||
| pub enum wasmtime_memory_image {} |
There was a problem hiding this comment.
The nomicon recommends against uninhabited types to represent C opaque types: https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs
Any reason not to follow their guidance here?
There was a problem hiding this comment.
Unfortunately that recommendation doesn't generate the right C header bindings.
I'm also not aware of any compiler issues using *mut Uninhabited, which is what these bindings are doing. While I know &mut Uninhabited is bad, the former is not plagued with the same issues as far as I'm aware.
| /// Attempts to create a new in-memory image of the `ptr`/`len` combo which | ||
| /// can be mapped to virtual addresses in the future. The returned | ||
| /// `wasmtime_memory_image` pointer can be `NULL` to indicate that an image | ||
| /// cannot be created. The structure otherwise will later be deallocated | ||
| /// with `wasmtime_memory_image_free` and `wasmtime_memory_image_map_at` | ||
| /// will be used to map the image into new regions of the address space. | ||
| pub fn wasmtime_memory_image_new(ptr: *const u8, len: usize) -> *mut wasmtime_memory_image; |
There was a problem hiding this comment.
Does len have to be a multiple of the system's page size? Is the resulting memory image rounded up to the page size? Is this stuff visible at all to Wasmtime?
There was a problem hiding this comment.
Yes they're both guaranteed to be page aligned, updated docs to match.
Is this stuff visible at all to Wasmtime?
Could you elaborate on this? (e.g. what you mean by "stuff")
|
Ok I've updated to make the C API fallible and Wasmtime propagates the error codes (thanks for pushing back on that @bjorn3!) |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This commit leverages adds a new "platform" to Wasmtime to be supported in the `crates/runtime/src/sys` folder. This joins preexisting platforms such as Unix and Windows. The goal of this platform is to be an opt-in way to build Wasmtime for targets that don't have a predefined way to run. The new "custom" platform requires `--cfg wasmtime_custom_platform` to be passed to the Rust compiler, for example by using `RUSTFLAGS`. This new platform bottoms out in a C API that is intended to be small and Linux-like. The C API is effectively the interface to virtual memory that Wasmtime requires. This C API is also available as a header file at `examples/min-platform/embedding/wasmtime-platform.h` (generated by `cbindgen`). The main purpose of this is to make it easier to experiment with porting Wasmtime to new platforms. By decoupling a platform implementation from Wasmtime itself it should be possible to run these experiments out-of-tree. An example of this I've been working on is getting Wasmtime running on bare-metal with a custom kernel. This support enables defining the platform interface of the custom kernel's syscalls outside of Wasmtime.
alexcrichton
force-pushed
the
min-platform-upstream
branch
from
d209962 to
3b4fd1d
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This commit leverages adds a new "platform" to Wasmtime to be supported in the
crates/runtime/src/sysfolder. This joins preexisting platforms such as Unix and Windows. The goal of this platform is to be an opt-in way to build Wasmtime for targets that don't have a predefined way to run.The new "custom" platform requires
--cfg wasmtime_custom_platformto be passed to the Rust compiler, for example by usingRUSTFLAGS. This new platform bottoms out in a C API that is intended to be small and Linux-like. The C API is effectively the interface to virtual memory that Wasmtime requires. This C API is also available as a header file atexamples/min-platform/embedding/wasmtime-platform.h(generated bycbindgen).The main purpose of this is to make it easier to experiment with porting Wasmtime to new platforms. By decoupling a platform implementation from Wasmtime itself it should be possible to run these experiments out-of-tree. An example of this I've been working on is getting Wasmtime running on bare-metal with a custom kernel. This support enables defining the platform interface of the custom kernel's syscalls outside of Wasmtime.