From 0a9c41b313ca866cc356fff01eb79719b901c1d8 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Tue, 10 Dec 2019 17:12:35 -0800 Subject: [PATCH 1/3] Integrate Ivan's arg parsing code into --invoke Co-authored-by: Ivan Enderlin --- src/bin/wasmer.rs | 22 ++++------- src/utils.rs | 99 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 15 deletions(-) diff --git a/src/bin/wasmer.rs b/src/bin/wasmer.rs index 793b15db0..f3e1c9170 100644 --- a/src/bin/wasmer.rs +++ b/src/bin/wasmer.rs @@ -38,6 +38,7 @@ use wasmer_runtime_core::{ backend::{Backend, Compiler, CompilerConfig, Features, MemoryBoundCheckMode}, debug, loader::{Instance as LoadedInstance, LocalLoader}, + Module, }; #[cfg(feature = "wasi")] use wasmer_wasi; @@ -246,18 +247,9 @@ struct Run { impl Run { /// Used with the `invoke` argument - fn parse_args(&self) -> Result, String> { - let mut args: Vec = Vec::new(); - for arg in self.args.iter() { - let x = arg.as_str().parse().map_err(|_| { - format!( - "Can't parse the provided argument {:?} as a integer", - arg.as_str() - ) - })?; - args.push(Value::I32(x)); - } - Ok(args) + fn parse_args(&self, module: &Module, fn_name: &str) -> Result, String> { + utils::parse_args(module, fn_name, &self.args) + .map_err(|e| format!("Invoke failed: {:?}", e)) } } @@ -493,7 +485,7 @@ fn execute_wasi( if let Some(invoke_fn) = options.invoke.as_ref() { eprintln!("WARNING: Invoking aribtrary functions with WASI is not officially supported in the WASI standard yet. Use this feature at your own risk!"); - let args = options.parse_args()?; + let args = options.parse_args(&module, invoke_fn)?; let invoke_result = instance .dyn_func(invoke_fn) .map_err(|e| format!("Invoke failed: {:?}", e))? @@ -825,12 +817,12 @@ fn execute_wasm(options: &Run) -> Result<(), String> { .instantiate(&import_object) .map_err(|e| format!("Can't instantiate module: {:?}", e))?; - let args = options.parse_args()?; - let invoke_fn = match options.invoke.as_ref() { Some(fun) => fun, _ => "main", }; + let args = options.parse_args(&module, invoke_fn)?; + let result = instance .dyn_func(&invoke_fn) .map_err(|e| format!("{:?}", e))? diff --git a/src/utils.rs b/src/utils.rs index fd96dc15b..2f2b0072f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,6 +1,105 @@ //! Utility functions for the WebAssembly module +use wasmer_runtime::{types::Type, Module, Value}; +use wasmer_runtime_core::{backend::SigRegistry, module::ExportIndex}; + /// Detect if a provided binary is a Wasm file pub fn is_wasm_binary(binary: &[u8]) -> bool { binary.starts_with(&[b'\0', b'a', b's', b'm']) } + +#[derive(Debug, Clone, Copy)] +pub enum InvokeError { + CouldNotFindFunction, + ExportNotFunction, + WrongNumArgs { expected: u16, found: u16 }, + CouldNotParseArgs, +} + +/// Parses arguments for the `--invoke` flag on the run command +pub fn parse_args( + module: &Module, + fn_name: &str, + args: &[String], +) -> Result, InvokeError> { + let export_index = module + .info() + .exports + .get(fn_name) + .ok_or(InvokeError::CouldNotFindFunction)?; + + let signature = if let ExportIndex::Func(func_index) = export_index { + let sig_index = module + .info() + .func_assoc + .get(*func_index) + .expect("broken invariant, incorrect func index"); + SigRegistry.lookup_signature_ref(&module.info().signatures[*sig_index]) + } else { + return Err(InvokeError::ExportNotFunction); + }; + + let parameter_types = signature.params(); + + if args.len() != parameter_types.len() { + return Err(InvokeError::WrongNumArgs { + expected: parameter_types.len() as _, + found: args.len() as _, + }); + } else { + args.iter() + .enumerate() + .try_fold( + Vec::with_capacity(args.len()), + |mut accumulator, (nth, argument)| { + if let Some(value) = match parameter_types[nth] { + Type::I32 => argument + .parse::() + .map(|v| Some(Value::I32(v))) + .unwrap_or_else(|_| { + eprintln!("Failed to parse `{:?}` as an `i32`", argument); + None + }), + Type::I64 => argument + .parse::() + .map(|v| Some(Value::I64(v))) + .unwrap_or_else(|_| { + eprintln!("Failed to parse `{:?}` as an `i64`", argument); + None + }), + Type::V128 => argument + .parse::() + .map(|v| Some(Value::V128(v))) + .unwrap_or_else(|_| { + eprintln!("Failed to parse `{:?}` as an `i64`", argument); + None + }), + Type::F32 => argument + .parse::() + .map(|v| Some(Value::F32(v))) + .unwrap_or_else(|_| { + eprintln!("Failed to parse `{:?}` as an `f32`", argument); + None + }), + Type::F64 => argument + .parse::() + .map(|v| Some(Value::F64(v))) + .unwrap_or_else(|_| { + eprintln!("Failed to parse `{:?}` as an `f64`", argument); + None + }), + } { + accumulator.push(value); + + Some(accumulator) + } else { + None + } + }, + ) + .map_or_else( + || Err(InvokeError::CouldNotParseArgs), + |arguments: Vec| Ok(arguments), + ) + } +} From bc8d5a542cad36bb6464acb6abf0d0316fed3c8f Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Tue, 10 Dec 2019 17:37:22 -0800 Subject: [PATCH 2/3] Return parse error strings rather than printing them in invoke parse --- src/utils.rs | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 2f2b0072f..1feec86b2 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -8,12 +8,12 @@ pub fn is_wasm_binary(binary: &[u8]) -> bool { binary.starts_with(&[b'\0', b'a', b's', b'm']) } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub enum InvokeError { CouldNotFindFunction, ExportNotFunction, WrongNumArgs { expected: u16, found: u16 }, - CouldNotParseArgs, + CouldNotParseArg(String), } /// Parses arguments for the `--invoke` flag on the run command @@ -40,6 +40,7 @@ pub fn parse_args( }; let parameter_types = signature.params(); + let mut arg_error = None; if args.len() != parameter_types.len() { return Err(InvokeError::WrongNumArgs { @@ -57,35 +58,50 @@ pub fn parse_args( .parse::() .map(|v| Some(Value::I32(v))) .unwrap_or_else(|_| { - eprintln!("Failed to parse `{:?}` as an `i32`", argument); + arg_error = Some(InvokeError::CouldNotParseArg(format!( + "Failed to parse `{:?}` as an `i32`", + argument + ))); None }), Type::I64 => argument .parse::() .map(|v| Some(Value::I64(v))) .unwrap_or_else(|_| { - eprintln!("Failed to parse `{:?}` as an `i64`", argument); + arg_error = Some(InvokeError::CouldNotParseArg(format!( + "Failed to parse `{:?}` as an `i64`", + argument + ))); None }), Type::V128 => argument .parse::() .map(|v| Some(Value::V128(v))) .unwrap_or_else(|_| { - eprintln!("Failed to parse `{:?}` as an `i64`", argument); + arg_error = Some(InvokeError::CouldNotParseArg(format!( + "Failed to parse `{:?}` as an `i128`", + argument + ))); None }), Type::F32 => argument .parse::() .map(|v| Some(Value::F32(v))) .unwrap_or_else(|_| { - eprintln!("Failed to parse `{:?}` as an `f32`", argument); + arg_error = Some(InvokeError::CouldNotParseArg(format!( + "Failed to parse `{:?}` as an `f32`", + argument + ))); None }), Type::F64 => argument .parse::() .map(|v| Some(Value::F64(v))) .unwrap_or_else(|_| { - eprintln!("Failed to parse `{:?}` as an `f64`", argument); + arg_error = Some(InvokeError::CouldNotParseArg(format!( + "Failed to parse `{:?}` as an `f64`", + argument + ))); None }), } { @@ -98,7 +114,7 @@ pub fn parse_args( }, ) .map_or_else( - || Err(InvokeError::CouldNotParseArgs), + || Err(arg_error.unwrap()), |arguments: Vec| Ok(arguments), ) } From 5aa70a8ae4409761e74cbac9900db18f519f0083 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Tue, 10 Dec 2019 17:57:26 -0800 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe44540cc..29014cb9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## **[Unreleased]** +- [#1056](https://github.com/wasmerio/wasmer/pull/1056) Improved `--invoke` args parsing (supporting `i32`, `i64`, `f32` and `f32`) in Wasmer CLI - [#1054](https://github.com/wasmerio/wasmer/pull/1054) Improve `--invoke` output in Wasmer CLI - [#1053](https://github.com/wasmerio/wasmer/pull/1053) For RuntimeError and breakpoints, use Box instead of Box. - [#1052](https://github.com/wasmerio/wasmer/pull/1052) Fix minor panic and improve Error handling in singlepass backend.