Skip to content
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.

Sign up for GitHub

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

Require Wasmtime options come before wasm modules #6946

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Require Wasmtime options come before wasm modules #6946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions src/bin/wasmtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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<Subcommand>,
#[clap(flatten)]
run: RunCommand,
}

#[derive(Parser)]
enum Subcommand {
/// Controls Wasmtime configuration settings
Config(ConfigCommand),
/// Compiles a WebAssembly module.
Expand All @@ -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]
Expand Down
100 changes: 44 additions & 56 deletions src/commands/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
)]

use anyhow::{anyhow, bail, Context as _, Error, Result};
use clap::builder::{OsStringValueParser, TypedValueParser};
use clap::Parser;
use std::ffi::OsStr;
use std::ffi::OsString;
use std::fs::File;
use std::io::Write;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -38,18 +35,6 @@ use wasmtime_wasi_threads::WasiThreadsCtx;
#[cfg(feature = "wasi-http")]
use wasmtime_wasi_http::WasiHttpCtx;

fn parse_module(s: OsString) -> anyhow::Result<PathBuf> {
// 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<String>)> {
let mut parts = s.splitn(2, '=');
Ok((
Expand Down Expand Up @@ -100,7 +85,7 @@ fn parse_profile(s: &str) -> Result<Profile> {

/// Runs a WebAssembly module
#[derive(Parser)]
#[structopt(name = "run", trailing_var_arg = true)]
#[structopt(name = "run")]
pub struct RunCommand {
#[clap(flatten)]
common: CommonOptions,
Expand Down Expand Up @@ -137,14 +122,6 @@ pub struct RunCommand {
#[clap(long, value_name = "FUNCTION")]
invoke: Option<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",
Expand Down Expand Up @@ -176,10 +153,13 @@ pub struct RunCommand {
)]
profile: Option<Profile>,

// NOTE: this must come last for trailing varargs
/// The arguments to pass to the module
#[clap(value_name = "ARGS")]
module_args: Vec<String>,
/// 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<PathBuf>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -242,7 +222,7 @@ impl RunCommand {
let engine = Engine::new(&config)?;

// Read the wasm module binary either as `*.wat` or a raw binary.
let main = self.load_module(&engine, &self.module)?;
let main = self.load_module(&engine, &self.module_and_args[0])?;

// Validate coredump-on-trap argument
if let Some(coredump_path) = &self.common.debug.coredump {
Expand Down Expand Up @@ -335,8 +315,12 @@ impl RunCommand {
// Load the main wasm module.
match self
.load_main_module(&mut store, &mut linker, &main, modules)
.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;
Expand Down Expand Up @@ -377,27 +361,25 @@ impl RunCommand {
Ok(listeners)
}

fn compute_argv(&self) -> Vec<String> {
fn compute_argv(&self) -> Result<Vec<String>> {
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(|c| c.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(
Expand All @@ -406,7 +388,7 @@ impl RunCommand {
modules: Vec<(String, Module)>,
) -> Box<dyn FnOnce(&mut Store<Host>)> {
if let Some(Profile::Guest { path, interval }) = &self.profile {
let module_name = self.module.to_str().unwrap_or("<main module>");
let module_name = self.module_and_args[0].to_str().unwrap_or("<main module>");
let interval = *interval;
store.data_mut().guest_profiler =
Some(Arc::new(GuestProfiler::new(module_name, interval, modules)));
Expand Down Expand Up @@ -512,9 +494,10 @@ impl RunCommand {
CliLinker::Core(linker) => {
// Use "" as a default module name.
let module = module.unwrap_core();
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 {
Expand Down Expand Up @@ -569,7 +552,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() {
Expand All @@ -582,6 +565,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
Expand Down Expand Up @@ -639,7 +625,9 @@ impl RunCommand {
if !err.is::<wasmtime::Trap>() {
return err;
}
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) {
eprintln!("warning: coredump failed to generate: {}", coredump_err);
Expand Down Expand Up @@ -884,7 +872,7 @@ impl RunCommand {

fn set_preview1_ctx(&self, store: &mut Store<Host>) -> Result<()> {
let mut builder = WasiCtxBuilder::new();
builder.inherit_stdio().args(&self.compute_argv())?;
builder.inherit_stdio().args(&self.compute_argv()?)?;

for (key, value) in self.vars.iter() {
let value = match value {
Expand Down Expand Up @@ -916,7 +904,7 @@ impl RunCommand {

fn set_preview2_ctx(&self, store: &mut Store<Host>) -> Result<()> {
let mut builder = preview2::WasiCtxBuilder::new();
builder.inherit_stdio().args(&self.compute_argv());
builder.inherit_stdio().args(&self.compute_argv()?);

for (key, value) in self.vars.iter() {
let value = match value {
Expand Down
28 changes: 10 additions & 18 deletions tests/all/cli_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,6 @@ fn wasm_flags() -> Result<()> {
let stdout = run_wasmtime(&[
"run",
"tests/all/cli_tests/print-arguments.wat",
"--",
"--argument",
"-for",
"the",
Expand All @@ -606,15 +605,15 @@ fn wasm_flags() -> Result<()> {
command\n\
"
);
let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "--", "-"])?;
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", "--", "--"])?;
let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "--"])?;
assert_eq!(
stdout,
"\
Expand All @@ -635,6 +634,7 @@ fn wasm_flags() -> Result<()> {
"\
print-arguments.wat\n\
--\n\
--\n\
-a\n\
b\n\
"
Expand All @@ -644,37 +644,30 @@ fn wasm_flags() -> Result<()> {

#[test]
fn name_same_as_builtin_command() -> Result<()> {
// A bare subcommand shouldn't run successfully.
//
// This is ambiguous between a missing module argument and a module named
// `run` with no other options.
// 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());

// Currently even `--` isn't enough to disambiguate, so this still is an
// error.
//
// NB: this will change in Wasmtime 14 when #6737 is relanded.
// 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 failure got {output:#?}");
assert!(output.status.success(), "expected success got {output:#?}");

// Passing `--foo` options before the module is also not enough to
// disambiguate for now.
//
// NB: this will change in Wasmtime 14 when #6737 is relanded.
// 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("-Ccache=n")
.arg("run")
.output()?;
assert!(!output.status.success(), "expected failure got {output:#?}");
assert!(output.status.success(), "expected success got {output:#?}");
Ok(())
}

Expand All @@ -694,7 +687,6 @@ fn wasm_flags_without_subcommand() -> Result<()> {
let output = get_wasmtime_command()?
.current_dir("tests/all/cli_tests/")
.arg("print-arguments.wat")
.arg("--")
.arg("-foo")
.arg("bar")
.output()?;
Expand Down