From 8091556514f03b919bad001a580a8a38f18d11a0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 20 Jul 2023 08:24:35 -0500 Subject: [PATCH] Require wasmtime options are first when running modules (#6737) * Require wasmtime options are first when running modules Currently the way we've configured argument parsing it's valid to execute a command such as: wasmtime run foo.wasm -O which is the same as: wasmtime run -O foo.wasm or otherwise all flags are attempted to be parsed as Wasmtime flags and an error is generated when they're not wasmtime flags. I've personally found this a bit confusing in the past and I find myself frequently executing: wasmtime run -- foo.wasm -other -arguments While this works my general impression is that many other "wrapper commands" don't behave this way and typically don't require `--` to pass flags to the target executable. This commit reconfigures argument parsing to consider any argument after the WebAssembly module itself to be an argument to the wasm program rather than an argument to Wasmtime. This means that all Wasmtime options must come between the `run` command and the `foo.wasm` WebAssembly argument. * Update wasi testsuite runner * Support `wasmtime -- run` Additionally use more clap features to avoid manually checking against subcommands. * Remove stale comment * Reorder wasi-nn arguments * Reorder more flags * Fix unused import on Windows * Don't run a stdio test on Windows * Update gdb/lldb tests * Don't assert that the write succeeds prtest:full --- ci/run-wasi-crypto-example.sh | 2 +- ci/run-wasi-nn-example.sh | 2 +- src/bin/wasmtime.rs | 46 +++-- src/commands/run.rs | 106 ++++++----- tests/all/cli_tests.rs | 228 ++++++++++++++++++------ tests/all/cli_tests/print-arguments.wat | 75 ++++++++ tests/all/cli_tests/run | 3 + tests/all/debug/gdb.rs | 2 +- tests/all/debug/lldb.rs | 4 +- tests/all/wasi_testsuite.rs | 9 +- 10 files changed, 339 insertions(+), 138 deletions(-) create mode 100644 tests/all/cli_tests/print-arguments.wat create mode 100644 tests/all/cli_tests/run diff --git a/ci/run-wasi-crypto-example.sh b/ci/run-wasi-crypto-example.sh index b027e4834002..30bbada3c76a 100755 --- a/ci/run-wasi-crypto-example.sh +++ b/ci/run-wasi-crypto-example.sh @@ -7,4 +7,4 @@ pushd "$RUST_BINDINGS" cargo build --release --target=wasm32-wasi popd -cargo run --features wasi-crypto -- run "$RUST_BINDINGS/target/wasm32-wasi/release/wasi-crypto-guest.wasm" --wasi-modules=experimental-wasi-crypto +cargo run --features wasi-crypto -- run --wasi-modules=experimental-wasi-crypto "$RUST_BINDINGS/target/wasm32-wasi/release/wasi-crypto-guest.wasm" diff --git a/ci/run-wasi-nn-example.sh b/ci/run-wasi-nn-example.sh index 82990d0bbda3..b8012120d3fa 100755 --- a/ci/run-wasi-nn-example.sh +++ b/ci/run-wasi-nn-example.sh @@ -33,7 +33,7 @@ cp target/wasm32-wasi/release/wasi-nn-example.wasm $TMP_DIR popd # Run the example in Wasmtime (note that the example uses `fixture` as the expected location of the model/tensor files). -cargo run -- run --mapdir fixture::$TMP_DIR $TMP_DIR/wasi-nn-example.wasm --wasi-modules=experimental-wasi-nn +cargo run -- run --mapdir fixture::$TMP_DIR --wasi-modules=experimental-wasi-nn $TMP_DIR/wasi-nn-example.wasm # Clean up the temporary directory only if it was not specified (users may want to keep the directory around). if [[ $REMOVE_TMP_DIR -eq 1 ]]; then diff --git a/src/bin/wasmtime.rs b/src/bin/wasmtime.rs index 6d425625dfa4..d32a0b63f373 100644 --- a/src/bin/wasmtime.rs +++ b/src/bin/wasmtime.rs @@ -4,7 +4,7 @@ //! See `wasmtime --help` for usage. use anyhow::Result; -use clap::{error::ErrorKind, Parser}; +use clap::Parser; use wasmtime_cli::commands::{ CompileCommand, ConfigCommand, ExploreCommand, RunCommand, SettingsCommand, WastCommand, }; @@ -27,10 +27,24 @@ use wasmtime_cli::commands::{ \n\ Invoking a specific function (e.g. `add`) in a WebAssembly module:\n\ \n \ - wasmtime example.wasm --invoke add 1 2\n" + wasmtime example.wasm --invoke add 1 2\n", + + // This option enables the pattern below where we ask clap to parse twice + // sorta: once where it's trying to find a subcommand and once assuming + // a subcommand doesn't get passed. Clap should then, apparently, + // fill in the `subcommand` if found and otherwise fill in the + // `RunCommand`. + args_conflicts_with_subcommands = true )] -enum Wasmtime { - // !!! IMPORTANT: if subcommands are added or removed, update `parse_module` in `src/commands/run.rs`. !!! +struct Wasmtime { + #[clap(subcommand)] + subcommand: Option, + #[clap(flatten)] + run: RunCommand, +} + +#[derive(Parser)] +enum Subcommand { /// Controls Wasmtime configuration settings Config(ConfigCommand), /// Compiles a WebAssembly module. @@ -48,26 +62,20 @@ enum Wasmtime { impl Wasmtime { /// Executes the command. pub fn execute(self) -> Result<()> { - match self { - Self::Config(c) => c.execute(), - Self::Compile(c) => c.execute(), - Self::Explore(c) => c.execute(), - Self::Run(c) => c.execute(), - Self::Settings(c) => c.execute(), - Self::Wast(c) => c.execute(), + let subcommand = self.subcommand.unwrap_or(Subcommand::Run(self.run)); + match subcommand { + Subcommand::Config(c) => c.execute(), + Subcommand::Compile(c) => c.execute(), + Subcommand::Explore(c) => c.execute(), + Subcommand::Run(c) => c.execute(), + Subcommand::Settings(c) => c.execute(), + Subcommand::Wast(c) => c.execute(), } } } fn main() -> Result<()> { - Wasmtime::try_parse() - .unwrap_or_else(|e| match e.kind() { - ErrorKind::InvalidSubcommand | ErrorKind::UnknownArgument => { - Wasmtime::Run(RunCommand::parse()) - } - _ => e.exit(), - }) - .execute() + Wasmtime::parse().execute() } #[test] diff --git a/src/commands/run.rs b/src/commands/run.rs index 55b09b53616c..00ef5e1fead5 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -1,14 +1,11 @@ //! The module that implements the `wasmtime run` command. use anyhow::{anyhow, bail, Context as _, Result}; -use clap::builder::{OsStringValueParser, TypedValueParser}; use clap::Parser; use once_cell::sync::Lazy; -use std::ffi::OsStr; -use std::ffi::OsString; use std::fs::File; use std::io::Write; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use std::thread; use std::time::Duration; use wasmtime::{ @@ -39,18 +36,6 @@ use wasmtime_wasi_threads::WasiThreadsCtx; #[cfg(feature = "wasi-http")] use wasmtime_wasi_http::WasiHttp; -fn parse_module(s: OsString) -> anyhow::Result { - // Do not accept wasmtime subcommand names as the module name - match s.to_str() { - Some("help") | Some("config") | Some("run") | Some("wast") | Some("compile") => { - bail!("module name cannot be the same as a subcommand") - } - #[cfg(unix)] - Some("-") => Ok(PathBuf::from("/dev/stdin")), - _ => Ok(s.into()), - } -} - fn parse_env_var(s: &str) -> Result<(String, Option)> { let mut parts = s.splitn(2, '='); Ok(( @@ -111,7 +96,7 @@ static AFTER_HELP: Lazy = Lazy::new(|| crate::FLAG_EXPLANATIONS.to_strin /// Runs a WebAssembly module #[derive(Parser)] -#[structopt(name = "run", trailing_var_arg = true, after_help = AFTER_HELP.as_str())] +#[structopt(name = "run", after_help = AFTER_HELP.as_str())] pub struct RunCommand { #[clap(flatten)] common: CommonOptions, @@ -174,14 +159,6 @@ pub struct RunCommand { #[clap(long = "mapdir", number_of_values = 1, value_name = "GUEST_DIR::HOST_DIR", value_parser = parse_map_dirs)] map_dirs: Vec<(String, String)>, - /// The path of the WebAssembly module to run - #[clap( - required = true, - value_name = "MODULE", - value_parser = OsStringValueParser::new().try_map(parse_module), - )] - module: PathBuf, - /// Load the given WebAssembly module before the main module #[clap( long = "preload", @@ -225,11 +202,6 @@ pub struct RunCommand { #[clap(long = "coredump-on-trap", value_name = "PATH")] coredump_on_trap: Option, - // NOTE: this must come last for trailing varargs - /// The arguments to pass to the module - #[clap(value_name = "ARGS")] - module_args: Vec, - /// Maximum size, in bytes, that a linear memory is allowed to reach. /// /// Growth beyond this limit will cause `memory.grow` instructions in @@ -261,6 +233,14 @@ pub struct RunCommand { /// memory, for example. #[clap(long)] trap_on_grow_failure: bool, + + /// The WebAssembly module to run and arguments to pass to it. + /// + /// Arguments passed to the wasm module will be configured as WASI CLI + /// arguments unless the `--invoke` CLI argument is passed in which case + /// arguments will be interpreted as arguments to the function specified. + #[clap(value_name = "WASM", trailing_var_arg = true, required = true)] + module_and_args: Vec, } #[derive(Clone)] @@ -303,13 +283,13 @@ impl RunCommand { // Make wasi available by default. let preopen_dirs = self.compute_preopen_dirs()?; - let argv = self.compute_argv(); + let argv = self.compute_argv()?; let mut linker = Linker::new(&engine); linker.allow_unknown_exports(self.allow_unknown_exports); // Read the wasm module binary either as `*.wat` or a raw binary. - let module = self.load_module(linker.engine(), &self.module)?; + let module = self.load_module(linker.engine(), &self.module_and_args[0])?; let mut modules = vec![(String::new(), module.clone())]; let host = Host::default(); @@ -370,8 +350,12 @@ impl RunCommand { // Load the main wasm module. match self .load_main_module(&mut store, &mut linker, module, modules, &argv[0]) - .with_context(|| format!("failed to run main module `{}`", self.module.display())) - { + .with_context(|| { + format!( + "failed to run main module `{}`", + self.module_and_args[0].display() + ) + }) { Ok(()) => (), Err(e) => { // Exit the process if Wasmtime understands the error; @@ -420,27 +404,25 @@ impl RunCommand { Ok(listeners) } - fn compute_argv(&self) -> Vec { + fn compute_argv(&self) -> Result> { let mut result = Vec::new(); - // Add argv[0], which is the program name. Only include the base name of the - // main wasm module, to avoid leaking path information. - result.push( - self.module - .components() - .next_back() - .map(Component::as_os_str) - .and_then(OsStr::to_str) - .unwrap_or("") - .to_owned(), - ); - - // Add the remaining arguments. - for arg in self.module_args.iter() { - result.push(arg.clone()); + for (i, arg) in self.module_and_args.iter().enumerate() { + // For argv[0], which is the program name. Only include the base + // name of the main wasm module, to avoid leaking path information. + let arg = if i == 0 { + arg.components().next_back().unwrap().as_os_str() + } else { + arg.as_ref() + }; + result.push( + arg.to_str() + .ok_or_else(|| anyhow!("failed to convert {arg:?} to utf-8"))? + .to_string(), + ); } - result + Ok(result) } fn setup_epoch_handler( @@ -541,9 +523,10 @@ impl RunCommand { } // Use "" as a default module name. - linker - .module(&mut *store, "", &module) - .context(format!("failed to instantiate {:?}", self.module))?; + linker.module(&mut *store, "", &module).context(format!( + "failed to instantiate {:?}", + self.module_and_args[0] + ))?; // If a function to invoke was given, invoke it. let func = if let Some(name) = &self.invoke { @@ -584,7 +567,7 @@ impl RunCommand { is experimental and may break in the future" ); } - let mut args = self.module_args.iter(); + let mut args = self.module_and_args.iter().skip(1); let mut values = Vec::new(); for ty in ty.params() { let val = match args.next() { @@ -597,6 +580,9 @@ impl RunCommand { } } }; + let val = val + .to_str() + .ok_or_else(|| anyhow!("argument is not valid utf-8: {val:?}"))?; values.push(match ty { // TODO: integer parsing here should handle hexadecimal notation // like `0x0...`, but the Rust standard library currently only @@ -623,7 +609,9 @@ impl RunCommand { if let Err(err) = invoke_res { let err = if err.is::() { if let Some(coredump_path) = self.coredump_on_trap.as_ref() { - let source_name = self.module.to_str().unwrap_or_else(|| "unknown"); + let source_name = self.module_and_args[0] + .to_str() + .unwrap_or_else(|| "unknown"); if let Err(coredump_err) = generate_coredump(&err, &source_name, coredump_path) { @@ -664,6 +652,12 @@ impl RunCommand { } fn load_module(&self, engine: &Engine, path: &Path) -> Result { + let path = match path.to_str() { + #[cfg(unix)] + Some("-") => "/dev/stdin".as_ref(), + _ => path, + }; + if self.allow_precompiled { unsafe { Module::from_trusted_file(engine, path) } } else { diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index cfba232b0d01..b5a33f59226d 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -1,41 +1,21 @@ #![cfg(not(miri))] -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; use std::fs::File; -use std::io::{Read, Write}; +use std::io::Write; use std::path::Path; -use std::process::{Command, Output, Stdio}; +use std::process::{Command, Output}; use tempfile::{NamedTempFile, TempDir}; // Run the wasmtime CLI with the provided args and return the `Output`. // If the `stdin` is `Some`, opens the file and redirects to the child's stdin. pub fn run_wasmtime_for_output(args: &[&str], stdin: Option<&Path>) -> Result { let mut cmd = get_wasmtime_command()?; - let stdin = stdin - .map(File::open) - .transpose() - .context("Cannot open a file to use as stdin")?; - - if let Some(mut f) = stdin { - let mut buf = Vec::new(); - f.read_to_end(&mut buf)?; - - let mut child = cmd - .stdout(Stdio::piped()) - .stdin(Stdio::piped()) - .args(args) - .spawn()?; - - let mut stdin = child.stdin.take().unwrap(); - std::thread::spawn(move || { - stdin - .write_all(&buf) - .expect("failed to write module to child stdin") - }); - child.wait_with_output().map_err(Into::into) - } else { - cmd.args(args).output().map_err(Into::into) + cmd.args(args); + if let Some(file) = stdin { + cmd.stdin(File::open(file)?); } + cmd.output().map_err(Into::into) } /// Get the Wasmtime CLI as a [Command]. @@ -94,10 +74,10 @@ fn run_wasmtime_simple() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/simple.wat")?; run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "simple", "--disable-cache", + wasm.path().to_str().unwrap(), "4", ])?; Ok(()) @@ -110,10 +90,10 @@ fn run_wasmtime_simple_fail_no_args() -> Result<()> { assert!( run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--disable-cache", "--invoke", "simple", + wasm.path().to_str().unwrap(), ]) .is_err(), "shall fail" @@ -128,11 +108,11 @@ fn run_coredump_smoketest() -> Result<()> { let coredump_arg = format!("--coredump-on-trap={}", coredump_file.path().display()); let err = run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "a", "--disable-cache", &coredump_arg, + wasm.path().to_str().unwrap(), ]) .unwrap_err(); assert!(err.to_string().contains(&format!( @@ -148,29 +128,29 @@ fn run_wasmtime_simple_wat() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/simple.wat")?; run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "simple", "--disable-cache", + wasm.path().to_str().unwrap(), "4", ])?; assert_eq!( run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "get_f32", "--disable-cache", + wasm.path().to_str().unwrap(), ])?, "100\n" ); assert_eq!( run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "get_f64", "--disable-cache", + wasm.path().to_str().unwrap(), ])?, "100\n" ); @@ -205,7 +185,7 @@ fn run_wasmtime_unreachable_wat() -> Result<()> { #[test] fn hello_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/hello_wasi_snapshot0.wat")?; - let stdout = run_wasmtime(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + let stdout = run_wasmtime(&["--disable-cache", wasm.path().to_str().unwrap()])?; assert_eq!(stdout, "Hello, world!\n"); Ok(()) } @@ -214,7 +194,7 @@ fn hello_wasi_snapshot0() -> Result<()> { #[test] fn hello_wasi_snapshot1() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/hello_wasi_snapshot1.wat")?; - let stdout = run_wasmtime(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + let stdout = run_wasmtime(&["--disable-cache", wasm.path().to_str().unwrap()])?; assert_eq!(stdout, "Hello, world!\n"); Ok(()) } @@ -225,10 +205,10 @@ fn timeout_in_start() -> Result<()> { let output = run_wasmtime_for_output( &[ "run", - wasm.path().to_str().unwrap(), "--wasm-timeout", "1ms", "--disable-cache", + wasm.path().to_str().unwrap(), ], None, )?; @@ -249,10 +229,10 @@ fn timeout_in_invoke() -> Result<()> { let output = run_wasmtime_for_output( &[ "run", - wasm.path().to_str().unwrap(), "--wasm-timeout", "1ms", "--disable-cache", + wasm.path().to_str().unwrap(), ], None, )?; @@ -272,7 +252,7 @@ fn timeout_in_invoke() -> Result<()> { fn exit2_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit2_wasi_snapshot0.wat")?; let output = - run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?; + run_wasmtime_for_output(&["--disable-cache", wasm.path().to_str().unwrap()], None)?; assert_eq!(output.status.code().unwrap(), 2); Ok(()) } @@ -282,7 +262,7 @@ fn exit2_wasi_snapshot0() -> Result<()> { fn exit2_wasi_snapshot1() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit2_wasi_snapshot1.wat")?; let output = - run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?; + run_wasmtime_for_output(&["--disable-cache", wasm.path().to_str().unwrap()], None)?; assert_eq!(output.status.code().unwrap(), 2); Ok(()) } @@ -292,7 +272,7 @@ fn exit2_wasi_snapshot1() -> Result<()> { fn exit125_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit125_wasi_snapshot0.wat")?; let output = - run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?; + run_wasmtime_for_output(&["--disable-cache", wasm.path().to_str().unwrap()], None)?; if cfg!(windows) { assert_eq!(output.status.code().unwrap(), 1); } else { @@ -306,7 +286,7 @@ fn exit125_wasi_snapshot0() -> Result<()> { fn exit125_wasi_snapshot1() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit125_wasi_snapshot1.wat")?; let output = - run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?; + run_wasmtime_for_output(&["--disable-cache", wasm.path().to_str().unwrap()], None)?; if cfg!(windows) { assert_eq!(output.status.code().unwrap(), 1); } else { @@ -320,7 +300,7 @@ fn exit125_wasi_snapshot1() -> Result<()> { fn exit126_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit126_wasi_snapshot0.wat")?; let output = - run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?; + run_wasmtime_for_output(&["--disable-cache", wasm.path().to_str().unwrap()], None)?; assert_eq!(output.status.code().unwrap(), 1); assert!(output.stdout.is_empty()); assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status")); @@ -343,7 +323,7 @@ fn exit126_wasi_snapshot1() -> Result<()> { #[test] fn minimal_command() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/minimal-command.wat")?; - let stdout = run_wasmtime(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + let stdout = run_wasmtime(&["--disable-cache", wasm.path().to_str().unwrap()])?; assert_eq!(stdout, ""); Ok(()) } @@ -352,7 +332,7 @@ fn minimal_command() -> Result<()> { #[test] fn minimal_reactor() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/minimal-reactor.wat")?; - let stdout = run_wasmtime(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + let stdout = run_wasmtime(&["--disable-cache", wasm.path().to_str().unwrap()])?; assert_eq!(stdout, ""); Ok(()) } @@ -363,10 +343,10 @@ fn command_invoke() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/minimal-command.wat")?; run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "_start", "--disable-cache", + wasm.path().to_str().unwrap(), ])?; Ok(()) } @@ -377,10 +357,10 @@ fn reactor_invoke() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/minimal-reactor.wat")?; run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--invoke", "_initialize", "--disable-cache", + wasm.path().to_str().unwrap(), ])?; Ok(()) } @@ -391,10 +371,10 @@ fn greeter() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/greeter_command.wat")?; let stdout = run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--disable-cache", "--preload", "reactor=tests/all/cli_tests/greeter_reactor.wat", + wasm.path().to_str().unwrap(), ])?; assert_eq!( stdout, @@ -409,10 +389,10 @@ fn greeter_preload_command() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/greeter_reactor.wat")?; let stdout = run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--disable-cache", "--preload", "reactor=tests/all/cli_tests/hello_wasi_snapshot1.wat", + wasm.path().to_str().unwrap(), ])?; assert_eq!(stdout, "Hello _initialize\n"); Ok(()) @@ -424,10 +404,10 @@ fn greeter_preload_callable_command() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/greeter_command.wat")?; let stdout = run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--disable-cache", "--preload", "reactor=tests/all/cli_tests/greeter_callable_command.wat", + wasm.path().to_str().unwrap(), ])?; assert_eq!(stdout, "Hello _start\nHello callable greet\nHello done\n"); Ok(()) @@ -439,7 +419,7 @@ fn greeter_preload_callable_command() -> Result<()> { fn exit_with_saved_fprs() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit_with_saved_fprs.wat")?; let output = - run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?; + run_wasmtime_for_output(&["--disable-cache", wasm.path().to_str().unwrap()], None)?; assert_eq!(output.status.code().unwrap(), 0); assert!(output.stdout.is_empty()); Ok(()) @@ -469,7 +449,7 @@ fn hello_wasi_snapshot0_from_stdin() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/hello_wasi_snapshot0.wat")?; let stdout = { let path = wasm.path(); - let args: &[&str] = &["-", "--disable-cache"]; + let args: &[&str] = &["--disable-cache", "-"]; let output = run_wasmtime_for_output(args, Some(path))?; if !output.status.success() { bail!( @@ -531,6 +511,8 @@ fn specify_env() -> Result<()> { #[cfg(unix)] #[test] fn run_cwasm_from_stdin() -> Result<()> { + use std::process::Stdio; + let td = TempDir::new()?; let cwasm = td.path().join("foo.cwasm"); let stdout = run_wasmtime(&[ @@ -540,11 +522,32 @@ fn run_cwasm_from_stdin() -> Result<()> { cwasm.to_str().unwrap(), ])?; assert_eq!(stdout, ""); + + // If stdin is literally the file itself then that should work let args: &[&str] = &["run", "--allow-precompiled", "-"]; - let output = run_wasmtime_for_output(args, Some(&cwasm))?; + let output = get_wasmtime_command()? + .args(args) + .stdin(File::open(&cwasm)?) + .output()?; + assert!(output.status.success(), "a file as stdin should work"); + + // If stdin is a pipe, however, that should fail + let input = std::fs::read(&cwasm)?; + let mut child = get_wasmtime_command()? + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + let mut stdin = child.stdin.take().unwrap(); + let t = std::thread::spawn(move || { + let _ = stdin.write_all(&input); + }); + let output = child.wait_with_output()?; if output.status.success() { bail!("wasmtime should fail loading precompiled modules from piped files, but suceeded"); } + t.join().unwrap(); Ok(()) } @@ -582,7 +585,6 @@ fn run_simple_with_wasi_threads() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/simple.wat")?; let stdout = run_wasmtime(&[ "run", - wasm.path().to_str().unwrap(), "--wasi-modules", "experimental-wasi-threads", "--wasm-features", @@ -590,8 +592,128 @@ fn run_simple_with_wasi_threads() -> Result<()> { "--disable-cache", "--invoke", "simple", + wasm.path().to_str().unwrap(), "4", ])?; assert_eq!(stdout, "4\n"); Ok(()) } + +#[test] +fn wasm_flags() -> Result<()> { + // Any argument after the wasm module should be interpreted as for the + // command itself + let stdout = run_wasmtime(&[ + "run", + "tests/all/cli_tests/print-arguments.wat", + "--argument", + "-for", + "the", + "command", + ])?; + assert_eq!( + stdout, + "\ + print-arguments.wat\n\ + --argument\n\ + -for\n\ + the\n\ + command\n\ + " + ); + let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "-"])?; + assert_eq!( + stdout, + "\ + print-arguments.wat\n\ + -\n\ + " + ); + let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "--"])?; + assert_eq!( + stdout, + "\ + print-arguments.wat\n\ + --\n\ + " + ); + let stdout = run_wasmtime(&[ + "run", + "tests/all/cli_tests/print-arguments.wat", + "--", + "--", + "-a", + "b", + ])?; + assert_eq!( + stdout, + "\ + print-arguments.wat\n\ + --\n\ + --\n\ + -a\n\ + b\n\ + " + ); + Ok(()) +} + +#[test] +fn name_same_as_builtin_command() -> Result<()> { + // a bare subcommand shouldn't run successfully + let output = get_wasmtime_command()? + .current_dir("tests/all/cli_tests") + .arg("run") + .output()?; + assert!(!output.status.success()); + + // a `--` prefix should let everything else get interpreted as a wasm + // module and arguments, even if the module has a name like `run` + let output = get_wasmtime_command()? + .current_dir("tests/all/cli_tests") + .arg("--") + .arg("run") + .output()?; + assert!(output.status.success(), "expected success got {output:#?}"); + + // Passing options before the subcommand should work and doesn't require + // `--` to disambiguate + let output = get_wasmtime_command()? + .current_dir("tests/all/cli_tests") + .arg("--disable-cache") + .arg("run") + .output()?; + assert!(output.status.success(), "expected success got {output:#?}"); + Ok(()) +} + +#[test] +#[cfg(unix)] +fn run_just_stdin_argument() -> Result<()> { + let output = get_wasmtime_command()? + .arg("-") + .stdin(File::open("tests/all/cli_tests/simple.wat")?) + .output()?; + assert!(output.status.success()); + Ok(()) +} + +#[test] +fn wasm_flags_without_subcommand() -> Result<()> { + let output = get_wasmtime_command()? + .current_dir("tests/all/cli_tests/") + .arg("print-arguments.wat") + .arg("-foo") + .arg("bar") + .output()?; + assert!(output.status.success()); + assert_eq!( + String::from_utf8_lossy(&output.stdout), + "\ + print-arguments.wat\n\ + -foo\n\ + bar\n\ + " + ); + Ok(()) +} diff --git a/tests/all/cli_tests/print-arguments.wat b/tests/all/cli_tests/print-arguments.wat new file mode 100644 index 000000000000..93e4558c46f0 --- /dev/null +++ b/tests/all/cli_tests/print-arguments.wat @@ -0,0 +1,75 @@ +(module + (import "wasi_snapshot_preview1" "fd_write" + (func $fd_write (param i32 i32 i32 i32) (result i32))) + + (import "wasi_snapshot_preview1" "args_get" + (func $args_get (param i32 i32) (result i32))) + + (memory (export "memory") 1) + + (func (export "_start") + (local $argptrs i32) + (local $argmem i32) + (local $arg i32) + + (local.set $argptrs (i32.mul (memory.grow (i32.const 1)) (i32.const 65536))) + (local.set $argmem (i32.mul (memory.grow (i32.const 1)) (i32.const 65536))) + + (if (i32.ne + (call $args_get (local.get $argptrs) (local.get $argmem)) + (i32.const 0)) + (unreachable)) + + (loop + (local.set $arg (i32.load (local.get $argptrs))) + (local.set $argptrs (i32.add (local.get $argptrs) (i32.const 4))) + (if (i32.eq (local.get $arg) (i32.const 0)) (return)) + + (call $write_all (local.get $arg) (call $strlen (local.get $arg))) + (call $write_all (i32.const 10) (i32.const 1)) + br 0 + ) + ) + + (func $write_all (param $ptr i32) (param $len i32) + (local $rc i32) + (local $iov i32) + (local $written i32) + + (local.set $written (i32.const 80)) + (local.set $iov (i32.const 100)) + + (loop + (local.get $len) + if + (i32.store (local.get $iov) (local.get $ptr)) + (i32.store offset=4 (local.get $iov) (local.get $len)) + (local.set $rc + (call $fd_write + (i32.const 1) + (local.get $iov) + (i32.const 1) + (local.get $written))) + (if (i32.ne (local.get $rc) (i32.const 0)) (unreachable)) + + (local.set $len (i32.sub (local.get $len) (i32.load (local.get $written)))) + (local.set $ptr (i32.add (local.get $ptr) (i32.load (local.get $written)))) + end + ) + ) + + (func $strlen (param $ptr i32) (result i32) + (local $len i32) + (loop + (i32.load8_u (i32.add (local.get $ptr) (local.get $len))) + if + (local.set $len (i32.add (local.get $len) (i32.const 1))) + br 1 + end + ) + local.get $len + ) + + (data (i32.const 10) "\n") +) + diff --git a/tests/all/cli_tests/run b/tests/all/cli_tests/run new file mode 100644 index 000000000000..4139404d3725 --- /dev/null +++ b/tests/all/cli_tests/run @@ -0,0 +1,3 @@ +(module + (func (export "_start")) +) diff --git a/tests/all/debug/gdb.rs b/tests/all/debug/gdb.rs index 6f5bbdb338dd..3ef3518b21ed 100644 --- a/tests/all/debug/gdb.rs +++ b/tests/all/debug/gdb.rs @@ -59,9 +59,9 @@ pub fn test_debug_dwarf_gdb() -> Result<()> { &[ "--disable-cache", "-g", - "tests/all/debug/testsuite/fib-wasm.wasm", "--invoke", "fib", + "tests/all/debug/testsuite/fib-wasm.wasm", "3", ], r#"set breakpoint pending on diff --git a/tests/all/debug/lldb.rs b/tests/all/debug/lldb.rs index 08fc344f880b..b6a097eb60e7 100644 --- a/tests/all/debug/lldb.rs +++ b/tests/all/debug/lldb.rs @@ -64,9 +64,9 @@ pub fn test_debug_dwarf_lldb() -> Result<()> { &[ "--disable-cache", "-g", - "tests/all/debug/testsuite/fib-wasm.wasm", "--invoke", "fib", + "tests/all/debug/testsuite/fib-wasm.wasm", "3", ], r#"b fib @@ -105,9 +105,9 @@ pub fn test_debug_dwarf5_lldb() -> Result<()> { &[ "--disable-cache", "-g", - "tests/all/debug/testsuite/fib-wasm-dwarf5.wasm", "--invoke", "fib", + "tests/all/debug/testsuite/fib-wasm-dwarf5.wasm", "3", ], r#"b fib diff --git a/tests/all/wasi_testsuite.rs b/tests/all/wasi_testsuite.rs index b1708a3fa017..8edeecaa8cbd 100644 --- a/tests/all/wasi_testsuite.rs +++ b/tests/all/wasi_testsuite.rs @@ -118,11 +118,6 @@ fn build_command>(module: P, extra_flags: &[&str], spec: &Spec) - cmd.arg(format!("{}::{}", dir, parent_dir.join(dir).display())); } } - cmd.arg(module.as_ref().to_str().unwrap()); - if let Some(spec_args) = &spec.args { - cmd.args(spec_args); - } - // Add environment variables as CLI arguments. if let Some(env) = &spec.env { for env_pair in env { @@ -131,6 +126,10 @@ fn build_command>(module: P, extra_flags: &[&str], spec: &Spec) - } cmd.envs(env); } + cmd.arg(module.as_ref().to_str().unwrap()); + if let Some(spec_args) = &spec.args { + cmd.args(spec_args); + } Ok(cmd) }