-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add a pure Rust WASI target? #73432
Comments
I would personally see the addition of a duplicate target as a very heavyweight operation that needs to be very well motivated. I don't really know what benefit we stand to gain other than shaving off some binary size, which could also arguably just be construed as a bug to fix in upstream wasi-libc. Are there other reasons than binary size to introduce a new target? |
I am not sure setting up C environment can be called a I guess in addition to smaller sizes, having a pure-Rust target could:
Also note that there is a general interest in having |
Also there is this issue: WebAssembly/WASI#24 I guess it could be solved by tweaking |
wasi-libc actually already has a change to help with WebAssembly/WASI#24 (see WebAssembly/wasi-libc#74), so at this point I think it's just a matter of rust taking advantage of it. And I managed to figure out how to hack the rust compiler to get it to work, involved setting wasi-root in rust's config.toml (to point to a recent build of wasi-libc) and changing the crt object linked against for the wasi target in librustc_target to crt1-reactor.o instead of crt1.o. Then I was able to produce a wasm file using a binary target with |
The first step here is to optimize Rust to avoid pulling in as much code. Right now with a default |
@sunfishcode |
WASI libc is structured such that it avoids pulling in the environment variable code if I've now debugged this a little more: the |
The source of bloat there is panicking. The runtime thinks it can panic in a few places during I don't think the preopen bits are needed by |
Some preopen bits are being pulled in because:
|
Ah indeed that makes sense. That seems unlikely to get fixed (it's just the behavior of the linker), but @sunfishcode perhaps we could refactor the APIs in wasi-libc? Could the population of libpreopen have an explicit entry point which we call from Rust's Basically Rust really has no need for constructor functions, and we'd rather lazily initialize them ourselves, and that way the linker GC should prevent everything from getting pulled in. |
Refactoring libc do do the libpreopen initialization lazily is tricky because once user code is running, it may have renumbered the file descriptors, which would break the way libpreopen currently discovers them. I've now submitted https://reviews.llvm.org/D85062 which is a patch to wasm-ld which allows it to DCE the |
I'm looking into adding support for reactor binaries, since it seems pretty simple -- use crt1-reactor instead of crt1 and set the entry to |
Or if we do need to change the entry symbol, maybe it should be a separate target entirely? |
Or does |
Ok, nevermind, the |
…ichton Emit a reactor for cdylib target on wasi Fixes rust-lang#79199, and relevant to rust-lang#73432 Implements wasi reactors, as described in WebAssembly/WASI#13 and [`design/application-abi.md`](https://github.com/WebAssembly/WASI/blob/master/design/application-abi.md) Empty `lib.rs`, `lib.crate-type = ["cdylib"]`: ```shell $ cargo +reactor build --release --target wasm32-wasi Compiling wasm-reactor v0.1.0 (/home/coolreader18/wasm-reactor) Finished release [optimized] target(s) in 0.08s $ wasm-dis target/wasm32-wasi/release/wasm_reactor.wasm >reactor.wat ``` `reactor.wat`: ```wat (module (type $none_=>_none (func)) (type $i32_=>_none (func (param i32))) (type $i32_i32_=>_i32 (func (param i32 i32) (result i32))) (type $i32_=>_i32 (func (param i32) (result i32))) (type $i32_i32_i32_=>_i32 (func (param i32 i32 i32) (result i32))) (import "wasi_snapshot_preview1" "fd_prestat_get" (func $__wasi_fd_prestat_get (param i32 i32) (result i32))) (import "wasi_snapshot_preview1" "fd_prestat_dir_name" (func $__wasi_fd_prestat_dir_name (param i32 i32 i32) (result i32))) (import "wasi_snapshot_preview1" "proc_exit" (func $__wasi_proc_exit (param i32))) (import "wasi_snapshot_preview1" "environ_sizes_get" (func $__wasi_environ_sizes_get (param i32 i32) (result i32))) (import "wasi_snapshot_preview1" "environ_get" (func $__wasi_environ_get (param i32 i32) (result i32))) (memory $0 17) (table $0 1 1 funcref) (global $global$0 (mut i32) (i32.const 1048576)) (global $global$1 i32 (i32.const 1049096)) (global $global$2 i32 (i32.const 1049096)) (export "memory" (memory $0)) (export "_initialize" (func $_initialize)) (export "__data_end" (global $global$1)) (export "__heap_base" (global $global$2)) (func $__wasm_call_ctors (call $__wasilibc_initialize_environ_eagerly) (call $__wasilibc_populate_preopens) ) (func $_initialize (call $__wasm_call_ctors) ) (func $malloc (param $0 i32) (result i32) (call $dlmalloc (local.get $0) ) ) ;; lots of dlmalloc, memset/memcpy, & libpreopen code ) ``` I went with repurposing cdylib because I figured that it doesn't make much sense to have a wasi shared library that can't be initialized, and even if someone was using it adding an `_initialize` export is a very small change.
and now:
and of course:
Or indeed any Strictly speaking, these are not duplicates (in one case the duplication IS total, but it is due to a bug), but the variations between them can be very subtle, and it is in many cases still useful to have distinct support for them, especially for more experimental targets. Thus, @newpavlov, if you wish to do the work, and can describe its merits, I believe it will be considered for inclusion (whether it will be accepted is of course another story). However, the target tier policy regarding adding targets mostly does not require GitHub issues open for them. In a sense, we are not accepting suggestions anymore, we are accepting PRs. As the rest of the conversation seems to be about ancillary bugs which are only secondarily related, and those can be split into new issues, I don't see a convenient "repurpose this issue" angle. Thus I am closing this. |
Currently
wasm32-wasi
target is tied to thelibc
implementation for compatibility with C/C++ code. But for pure-Rust applications C environment is unnecessary and causes a certain bloat of resulting binaries.For example a very simple code like this:
Gets compiled into a 64 KB binary (after strip), I guess a significant amount of which has to do with correctly setting up and destroying C environment.
Thus it could be beneficial to add a target like
wasm32-wasi-rust
intended for pure-Rust WASI applications. Most of the preliminary work has been already done and dependence ofwasm32-wasi
onlibc
is minimal (allocator and environment functions,abort
,exit
,__wasilibc_find_relpath
), so IIUC addition of such target should be relatively simple. Depending on the rate of WASI evolution, such addition could be postponed until a certain level of stabilization to reduce maintenance burden.Previously it was proposed in #63676.
cc @alexcrichton @sunfishcode
The text was updated successfully, but these errors were encountered: