Add a -Zwasi-exec-model codegen option for emitting WASI reactors

[coolreader18]

Fixes #79199, and relevant to #73432

Implements wasi reactors, as described in WebAssembly/WASI#13 and design/application-abi.md

Empty lib.rs, lib.crate-type = ["cdylib"]:

$ 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:

(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.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matthewjasper (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[petrochenkov]

LGTM from the linking infrastructure point of view, r? @alexcrichton on WASM specifics.

[alexcrichton]

Thanks! I'd double check with @sunfishcode since I believe you implemented this in Clang, does this look right?

[coolreader18]

@sunfishcode could you take a look at this?

[sunfishcode]

My main concern here is with the re-use of the "dylib" concept. In clang, for comparison, we didn't re-use -shared; we instead added a new flag, -mexec-model=reactor. This is because Reactors aren't exactly dynamic libraries; for example, they typically don't import linear memory, and they expect to be executable on their own.

Real dynamic libraries are coming, and I expect we'll want to use "dylib" and -shared and so on for them, so I'm concerned that if we use "dylib" and "-shared" for Reactors today, we'll have a compatibility problem when we're ready to implement real dynamic libraries in the future. Would it be feasible to add a new option, analogous to clang's -mexec-model=reactor?

[coolreader18]

Would it make sense to add a new LinkOutputKind::WasiReactor? That was my original plan, I think it makes more sense than a completely orthogonal rustc option, since you can't really have something be a reactor and a (Static|Dynamic)(Pic|NoPic)Exe

[coolreader18]

Although, maybe it would make sense for a reactor to be a dylib and vice versa? It should obviously be left to the wasi repo to determine the spec (and only after the dynamic linking proposal gets merged), but ELF .sos have ctors, so it might make sense for wasi dylibs to have some mechanism for that as well.

[sunfishcode]

"Reactor" is indeed an evolving concept, but it currently looks like a Reactor is indeed a kind of Exe. It's not yet clear if we'll have a concept of Pic|NoPic, but Static|Dynamic will apply, once we have dynamic linking.

[coolreader18]

Alright, that makes sense. @petrochenkov what do you think would be the best way of doing this? Should LinkOutputKind become a struct, with enum LinkKind and enum ExecModel fields? That way crt.o selection can still be done with a BTreeMap, and <WasmLd as Linker>::set_output_kind would still have a way to check if since it would still get passed LinkOutputKind. Though, @sunfishcode, is that --entry _initialize arg necessary? I'm not sure what exactly it does in the context of wasm, since there's no concept of entry point in the spec like there is for elf. Also, would it ever make sense for a *Dylib be a reactor? Maybe just to have a defined constructor symbol, or for a wasi module to play the role of executor rather than the host?

[sunfishcode]

Yes, the --entry _initialize is needed for now. WASI libc uses lazy initialization for a lot of things, but still does use static constructors for a few things, and _initialize is how they run. You're right that the Wasm binary format spec doesn't define an _initailize method, so we simply defined _initialize to be the init function at the WASI level instead.

For context, much of this will change when module-linking and interface-types are ready, and we rebase the "Reactor" concept on top of them.

I should be clear that everything I'm saying here about Reactors is based on my best guesses as to how we're going to want things to work once module-linking and interface-types are ready, rather than any kind of complete plan. With that said, I think it doesn't make sense to think of making *Dylibs be Reactors. *Dylib modules will use shared-everything linking, since that's how regular Rust/C/etc. dynamic linking works -- libraries all live in the same address space as the main executable. Reactors will be linked to the outside world either by being invoked directly or by shared-nothing linking, just as Commands are. Reactors are meant to be the roots of their runtime shared-everything dynamic linking trees, in the way that Commands also are. So in terms of the toolchain support, it seems to me like Reactors are a variant of the typical Executable class, in ELF-like terms. The only difference is that their lifetime at runtime isn't scoped to a single function call, as an ELF executable is.

[petrochenkov]

what do you think would be the best way of doing this?

Since the set of linked objects is keyed on LinkOutputKind in any case, the least invasive solution right now would be to add one new variant to it - WasiReactorExe.
(I tried to make LinkOutputKind more orthogonal in the past, e.g. factoring out Pic as a separate flag, but it only brought annoyance in this case.)

As for user-facing interface, a new target specific option -Z wasi-exec-model=command,reactor (or -Z wasm-exec-model?) would work, that would also be analogous to what clang does.

[coolreader18]

Alright, this works. The one thing that is notable is that since it's an exe but doesn't really have a main function, it errors if there's no fn main(), though just #![no_main] seems to work fine.

w.r.t the WasiExecModel enum in options.rs, I know that most of the other types for the fields of the options structs are imported from other crates, but I think the ExecModel really is just a config type -- it's a string enumeration passed as a codegen option, that gets turned into a "real" codegen type later.

[petrochenkov]

@coolreader18 Could you also squash commits into one before this is merged?

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[alexcrichton]

@bors: retry

[Ekleog]

Hey,

Thank you for this PR! It comes just in time, at the moment I actually wanted to try to use that, so that's a great timing!

That said, I'm trying to use this without success. I'm using rustc 1.51.0-nightly (7a9b552cb 2021-01-12), which contains this merged PR. Building with cargo rustc --release --target wasm32-wasi -- -Z wasi-exec-model=reactor and then wasm-objdump -x target/wasm32-wasi/release/foo.wasm shows these exports:

Export[4]:
 - memory[0] -> "memory"
 - func[10] <foo> -> "foo"
 - global[1] -> "__data_end"
 - global[2] -> "__heap_base"

with this source code:

#[no_mangle]
pub extern "C" fn foo() {
    println!("before read");
    println!("read: {:?}", std::fs::read_to_string("foo.txt"));
    println!("after read");
}

and this Cargo.toml:

[package]
name = "foo"
version = "0.1.0"

[lib]
crate-type = ["cdylib"]

I don't see the _initialize function, despite the fact that I'm using functions that would require it. And indeed, trying to run the reactor, I get:

[nix-shell:/tmp/foo]$ wasmtime --mapdir .::. target/wasm32-wasi/release/foo.wasm --invoke foo
before read
read: Err(Custom { kind: Other, error: "failed to find a pre-opened file descriptor through which \"foo.txt\" could be opened" })
after read

[nix-shell:/tmp/foo]$ cat foo.txt
hello

What am I missing?

[coolreader18]

@Ekleog oh, yeah, I changed how you enable reactors but never the title of the PR, so it's misleading. Try removing the crate-type = cdylib bit, it should work then.

[Ekleog]

That seems to work! So to sum up how this is to be used, for other readers:

• Do not have lib.crate-type at all
• Have the file be src/main.rs, not src/lib.rs
• Have a pub fn main() {} function (even though it won't actually be used)
• Use cargo rustc --target wasm32-wasi -- -Z wasi-exec-model=reactor
• Make sure that you call _initialize before anything else, wasmtime (as of the last current commit on rewrite wasi-common in terms of cap-std bytecodealliance/wasmtime#2487) doesn't do it (yet?)

Thank you! :D

[coolreader18]

Yep, no problem! Also note that you don't need a main function at all, you can put #![no_main] at the top of the file

[sunfishcode]

Also for other readers, keep in mind that this area is still in development, so some parts of the above may change over time 🙂

[XX]

@coolreader18 When I compile a WASM-WASI module with lto=true, then simple reading the file throws the Os { code: 44, kind: NotFound, message: "No such file or directory" } error in wasmtime and the Os { code: 31, kind: Other, message: "Is a directory" } error in wasmer. But if I compile the program without lto=true, then it works well.

[ValeryAntopol]

Are there any estimations or places to track about stabilizing this feature? We are using "handmade" wasm reactors, and suddenly realized that any call to our wasm reactors leads to a memory leak if the module was build with rustc 1.56 or higher. That is quite sad and we have to make a workaround instead of using this feature, because we really don't want to switch to nightly for our wasm SDK. I know about adding an explicit call to __wasm_cal_ctors from WebAssembly/WASI#471, but the best way will be to utilize the existing reactor model with __initialize call. Maybe there is a way to speed up the process by helping?