rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.71k stars 12.75k forks source link

Compiled wasm32-wasip2 component from simple code requires excessive WASI interfaces #133235

Open ifsheldon opened 6 hours ago

ifsheldon commented 6 hours ago

During my learning and experiments of wasip2, I tried this simple code and compiled it to a wasip2 component:

wit_bindgen::generate!({
    // the name of the world in the `*.wit` input file
    world: "formatter",
});

struct Formatter;

impl Guest for Formatter {
    fn format_str(a: String, b: String) -> String {
        let s = format!("{} + {}", a, b);
        print(s.as_str());
        s
    }
}

export!(Formatter);

and

// format.wit
package component:formatter;

world formatter {
    import print: func(s: string);
    export format-str: func(a: string, b: string) -> string; 
}

The host code, which runs the component, is:

use wasmtime::component::*;
use wasmtime::{Engine, Store};
use wasmtime_wasi::{WasiCtx, WasiImpl, WasiView};
use anyhow::Result;
// reference: https://docs.rs/wasmtime/latest/wasmtime/component/bindgen_examples/_0_hello_world/index.html
// reference: https://docs.wasmtime.dev/examples-rust-wasi.html

bindgen!({
    path: "../implementation/wit/format.wit",
    world: "formatter",
});

struct MyState {
    // These two are required basically as a standard way to enable the impl of WasiView
    wasi_ctx: WasiCtx,
    table: ResourceTable,
}

impl WasiView for MyState {
    fn table(&mut self) -> &mut ResourceTable {
        &mut self.table
    }
    fn ctx(&mut self) -> &mut WasiCtx {
        &mut self.wasi_ctx
    }
}

impl FormatterImports for MyState {
    fn print(&mut self, s: String) {
        println!("{}", s);
    }
}

/// copied from wasmtime_wasi::type_annotate, which is a private function
fn type_annotate<T: WasiView, F>(val: F) -> F
where
    F: Fn(&mut T) -> WasiImpl<&mut T>,
{
    val
}

fn main() -> Result<()> {
    let engine = Engine::default();
    let component = Component::from_file(
        &engine,
        "../implementation/target/wasm32-wasip2/release/implementation.wasm",
    )?;

    let mut linker = Linker::new(&engine);

    let ctx = wasmtime_wasi::WasiCtxBuilder::new().build();
    let state = MyState {
        wasi_ctx: ctx,
        table: ResourceTable::new(),
    };
    let mut store = Store::new(&engine, state);
    Formatter::add_to_linker(&mut linker, |s| s)?;

    // Note: 
    // The below block is copied from `wasmtime_wasi::add_to_linker_sync`.
    // why a "format!()" in the implementation needs `sync::filesystem::types`, `sync::io::streams`, `cli::exit`, `cli::environment`, `cli::stdin`, `cli::stdout`, `cli::stderr`?
    {
        let l = &mut linker;
        let closure = type_annotate::<MyState, _>(|t| WasiImpl(t));
        wasmtime_wasi::bindings::sync::filesystem::types::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::filesystem::preopens::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::io::error::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::sync::io::streams::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::cli::exit::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::cli::environment::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::cli::stdin::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::cli::stdout::add_to_linker_get_host(l, closure)?;
        wasmtime_wasi::bindings::cli::stderr::add_to_linker_get_host(l, closure)?;
    }

    let bindings = Formatter::instantiate(&mut store, &component, &linker)?;
    let result = bindings.call_format_str(&mut store, "a", "b")?;
    println!("format_str: {}", result);
    Ok(())
}

The block with a note binds many interfaces to avoid runtime errors that says something like component imports instance wasi:cli/environment@0.2.0, but a matching implementation was not found in the linker. Removing any one of the lines in the block will result in a runtime error.

I expect this compiled component requires none of these WASI interfaces to run, since it has nothing to do with io, cli, etc. Binding these unnecessary interfaces may raise security concerns.

The full minimized code is here.

As a kind person pointed out on ByteAlliance Zulip, these interfaces are required by std.

Probably there's a way to minimize or prune the interface requirements in the compilation? I think rustc has all the information of which effects are used by any one of functions/macros that is used by a user.

At the very lease, I think we should document these requirements somewhere, so there are no hidden/dark interface dependencies that are not specified and unknown in WIT files.

Meta

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: aarch64-apple-darwin
release: 1.82.0
LLVM version: 19.1.1
jieyouxu commented 3 hours ago

I'm not a wasi/wasm expert by any means, but doesn't your example in the repo use wit_bindgen which probably has non-trivial proc-macros? Is there a more minimal repro that only uses rustc or cargo but with only std?

EDIT: Nevermind I read the zulip thread, so this is T-libs

This feels weird to me, why does format!() need to access the environment? To avoid these runtime errors, it turns out the following interfaces need to be linked to the linker: sync::filesystem::types sync::io::streams cli::exit cli::environment cli::stdin cli::stdout cli::stderr Is it expected and why? Or, is it a bug?

jieyouxu commented 3 hours ago

cc @alexcrichton, but my immediate feeling is that this is kinda working as intended?

jieyouxu commented 3 hours ago

Ah right we have a wasi ping group now @rustbot ping wasi

rustbot commented 3 hours ago

Hey WASI notification group! This issue or PR could use some WASI-specific guidance. Could one of you weigh in? Thanks <3

(In case it's useful, here are some instructions for tackling these sorts of issues).

cc @alexcrichton @burakemir @juntyr

juntyr commented 3 hours ago

My guess would be that format is pulling in the entire panic machinery which includes a lot more string formatting. You could try compiling the wasm binary and std with panic immediate abort (I don’t know the exact flags by heart) and see if that removes those extraneous environmental dependencies.

jieyouxu commented 3 hours ago

Based on https://github.com/rust-lang/wg-cargo-std-aware/issues/29 it might be something like cargo ... -Zbuild-std -Zbuild-std-features=panic_immediate_abort (I haven't tried this)

juntyr commented 3 hours ago

Based on rust-lang/wg-cargo-std-aware#29 it might be something like cargo ... -Zbuild-std -Zbuild-std-features=panic_immediate_abort (I haven't tried this)

Those look correct - you might also need RUSTFLAGS=-C panic=abort

ifsheldon commented 1 hour ago

Alright, I changed my build command to RUSTFLAGS="-C panic=abort" cargo build --target wasm32-wasip2 --release -Zbuild-std -Zbuild-std-features=panic_immediate_abort but I got this error

error[E0152]: duplicate lang item in crate `core`: `sized`
  |
  = note: the lang item is first defined in crate `core` (which `implementation` depends on)
  = note: first definition in `core` loaded from /Users/zhiqiu/offline_code/opensource/wit_issue/implementation/target/wasm32-wasip2/release/deps/libcore-55d3ecbce88cf86e.rlib, /Users/zhiqiu/offline_code/opensource/wit_issue/implementation/target/wasm32-wasip2/release/deps/libcore-55d3ecbce88cf86e.rmeta
  = note: second definition in `core` loaded from /Users/zhiqiu/.rustup/toolchains/nightly-2024-11-19-aarch64-apple-darwin/lib/rustlib/wasm32-wasip2/lib/libcore-65e53cbf30438bae.rlib

This is out of my mind and I don't know if it's another solvable issue or bug....... Kind of reminds me of the horrible building errors in C.

I think you can reproduce this in this branch of my repo.