Merge #476

476: fix bug in wasi::environ_get, fix off by one error in env_size_get r=MarkMcCaskey a=MarkMcCaskey

adds `--env` flag (resolves #475)
fixes other wasi env bugs
adds wasi envvar test

Co-authored-by: Mark McCaskey <[email protected]>

CHANGELOG.md

@@ -6,6 +6,7 @@ Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
 
+- [#476](https://github.com/wasmerio/wasmer/pull/476) Fix bug with wasi::environ_get, fix off by one error in wasi::environ_sizes_get
 - [#470](https://github.com/wasmerio/wasmer/pull/470) Add mapdir support to Emscripten, implement getdents for Unix
 - [#467](https://github.com/wasmerio/wasmer/pull/467) `wasmer_instantiate` returns better error messages in the runtime C API
 - [#463](https://github.com/wasmerio/wasmer/pull/463) Fix bug in WASI path_open allowing one level above preopened dir to be accessed

lib/wasi/build/wasitests.rs

@@ -58,6 +58,8 @@ pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
     let result = Command::new(&normalized_name)
         .output()
         .expect("Failed to execute native program");
+
+    std::fs::remove_file(&normalized_name).expect("could not delete executable");
     let wasm_out_name = format!("{}.wasm", &normalized_name);
 
     Command::new("rustc")
@@ -76,25 +78,31 @@ pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
     };
 
     let src_code = fs::read_to_string(file).expect("read src file");
-    let args = extract_args_from_source_file(&src_code);
-
-    let mapdir_args = if let Some(a) = args {
-        if !a.mapdir.is_empty() {
-            let mut out_str = String::new();
-            out_str.push_str("vec![");
-            for (alias, real_dir) in a.mapdir {
-                out_str.push_str(&format!(
-                    "(\"{}\".to_string(), \"{}\".to_string()),",
-                    alias, real_dir
-                ));
-            }
-            out_str.push_str("]");
-            out_str
-        } else {
-            "vec![]".to_string()
+    let args = extract_args_from_source_file(&src_code).unwrap_or_default();
+
+    let mapdir_args = {
+        let mut out_str = String::new();
+        out_str.push_str("vec![");
+        for (alias, real_dir) in args.mapdir {
+            out_str.push_str(&format!(
+                "(\"{}\".to_string(), \"{}\".to_string()),",
+                alias, real_dir
+            ));
         }
-    } else {
-        "vec![]".to_string()
+        out_str.push_str("]");
+        out_str
+    };
+
+    let envvar_args = {
+        let mut out_str = String::new();
+        out_str.push_str("vec![");
+
+        for entry in args.envvars {
+            out_str.push_str(&format!("\"{}={}\".to_string(),", entry.0, entry.1));
+        }
+
+        out_str.push_str("]");
+        out_str
     };
 
     let contents = format!(
@@ -104,6 +112,7 @@ fn test_{rs_module_name}() {{
         \"../../{module_path}\",
         \"{rs_module_name}\",
         {mapdir_args},
+        {envvar_args},
         \"../../{test_output_path}\"
     );
 }}
@@ -113,6 +122,7 @@ fn test_{rs_module_name}() {{
         rs_module_name = rs_module_name,
         test_output_path = format!("{}.out", normalized_name),
         mapdir_args = mapdir_args,
+        envvar_args = envvar_args
     );
     let rust_test_filepath = format!(
         concat!(env!("CARGO_MANIFEST_DIR"), "/tests/{}.rs"),
@@ -167,14 +177,16 @@ fn read_ignore_list() -> HashSet<String> {
         .collect()
 }
 
+#[derive(Debug, Default)]
 struct Args {
     pub mapdir: Vec<(String, String)>,
+    pub envvars: Vec<(String, String)>,
 }
 
 /// Pulls args to the program out of a comment at the top of the file starting with "// Args:"
 fn extract_args_from_source_file(source_code: &str) -> Option<Args> {
     if source_code.starts_with("// Args:") {
-        let mut args = Args { mapdir: vec![] };
+        let mut args = Args::default();
         for arg_line in source_code
             .lines()
             .skip(1)
@@ -196,6 +208,13 @@ fn extract_args_from_source_file(source_code: &str) -> Option<Args> {
                         );
                     }
                 }
+                "env" => {
+                    if let [name, val] = &tokenized[2].split('=').collect::<Vec<&str>>()[..] {
+                        args.envvars.push((name.to_string(), val.to_string()));
+                    } else {
+                        eprintln!("Parse error in env {} not parsed correctly", &tokenized[2]);
+                    }
+                }
                 e => {
                     eprintln!("WARN: comment arg: {} is not supported", e);
                 }

lib/wasi/src/syscalls/mod.rs

@@ -195,7 +195,13 @@ pub fn clock_time_get(
     let memory = ctx.memory(0);
 
     let out_addr = wasi_try!(time.deref(memory));
-    platform_clock_time_get(clock_id, precision, out_addr)
+    let result = platform_clock_time_get(clock_id, precision, out_addr);
+    debug!(
+        "time: {} => {}",
+        wasi_try!(time.deref(memory)).get(),
+        result
+    );
+    result
 }
 
 /// ### `environ_get()`
@@ -215,7 +221,7 @@ pub fn environ_get(
     let state = get_wasi_state(ctx);
     let memory = ctx.memory(0);
 
-    write_buffer_array(memory, &*state.args, environ, environ_buf)
+    write_buffer_array(memory, &*state.envs, environ, environ_buf)
 }
 
 /// ### `environ_sizes_get()`
@@ -238,8 +244,15 @@ pub fn environ_sizes_get(
 
     let state = get_wasi_state(ctx);
 
-    environ_count.set(state.envs.len() as u32);
-    environ_buf_size.set(state.envs.iter().map(|v| v.len() as u32).sum());
+    let env_var_count = state.envs.len() as u32;
+    let env_buf_size = state.envs.iter().map(|v| v.len() as u32 + 1).sum();
+    environ_count.set(env_var_count);
+    environ_buf_size.set(env_buf_size);
+
+    debug!(
+        "env_var_count: {}, env_buf_size: {}",
+        env_var_count, env_buf_size
+    );
 
     __WASI_ESUCCESS
 }
@@ -1665,7 +1678,7 @@ pub fn proc_raise(ctx: &mut Ctx, sig: __wasi_signal_t) -> __wasi_errno_t {
 /// - `size_t buf_len`
 ///     The number of bytes that will be written
 pub fn random_get(ctx: &mut Ctx, buf: WasmPtr<u8, Array>, buf_len: u32) -> __wasi_errno_t {
-    debug!("wasi::random_get");
+    debug!("wasi::random_get buf_len: {}", buf_len);
     let mut rng = thread_rng();
     let memory = ctx.memory(0);
 

lib/wasi/tests/wasitests/_common.rs

@@ -1,5 +1,5 @@
 macro_rules! assert_wasi_output {
-    ($file:expr, $name:expr, $mapdir_args:expr, $expected:expr) => {{
+    ($file:expr, $name:expr, $mapdir_args:expr, $envvar_args:expr, $expected:expr) => {{
         use wasmer_dev_utils::stdio::StdioCapturer;
         use wasmer_runtime_core::{backend::Compiler, Func};
         use wasmer_wasi::generate_import_object;

lib/wasi/tests/wasitests/create_dir.rs

@@ -5,6 +5,7 @@ fn test_create_dir() {
         "../../wasitests/create_dir.wasm",
         "create_dir",
         vec![],
+        vec![],
         "../../wasitests/create_dir.out"
     );
 }

lib/wasi/tests/wasitests/envvar.rs

@@ -0,0 +1,10 @@
+#[test]
+fn test_envvar() {
+    assert_wasi_output!(
+        "../../wasitests/envvar.wasm",
+        "envvar",
+        vec![],
+        vec![],
+        "../../wasitests/envvar.out"
+    );
+}

lib/wasi/tests/wasitests/file_metadata.rs

@@ -4,6 +4,7 @@ fn test_file_metadata() {
         "../../wasitests/file_metadata.wasm",
         "file_metadata",
         vec![],
+        vec![],
         "../../wasitests/file_metadata.out"
     );
 }

lib/wasi/tests/wasitests/fs_sandbox_test.rs

@@ -4,6 +4,7 @@ fn test_fs_sandbox_test() {
         "../../wasitests/fs_sandbox_test.wasm",
         "fs_sandbox_test",
         vec![],
+        vec![],
         "../../wasitests/fs_sandbox_test.out"
     );
 }

lib/wasi/tests/wasitests/hello.rs

@@ -4,6 +4,7 @@ fn test_hello() {
         "../../wasitests/hello.wasm",
         "hello",
         vec![],
+        vec![],
         "../../wasitests/hello.out"
     );
 }

lib/wasi/tests/wasitests/mapdir.rs

@@ -4,6 +4,7 @@ fn test_mapdir() {
         "../../wasitests/mapdir.wasm",
         "mapdir",
         vec![],
+        vec![],
         "../../wasitests/mapdir.out"
     );
 }

lib/wasi/tests/wasitests/mod.rs

@@ -6,6 +6,7 @@
 #[macro_use]
 mod _common;
 mod create_dir;
+mod envvar;
 mod file_metadata;
 mod fs_sandbox_test;
 mod hello;

lib/wasi/tests/wasitests/quine.rs

@@ -4,6 +4,7 @@ fn test_quine() {
         "../../wasitests/quine.wasm",
         "quine",
         vec![],
+        vec![],
         "../../wasitests/quine.out"
     );
 }

lib/wasi/wasitests/envvar.out

@@ -0,0 +1,6 @@
+Env vars:
+CAT=2
+DOG=1
+DOG Ok("1")
+DOG_TYPE Err(NotPresent)
+SET VAR Ok("HELLO")

lib/wasi/wasitests/envvar.rs

@@ -0,0 +1,38 @@
+// Args:
+// env: DOG=1
+// env: CAT=2
+
+use std::env;
+
+fn get_env_var(var_name: &str) -> Result<String, env::VarError> {
+    #[cfg(not(target = "wasi"))]
+    match var_name {
+        "DOG" => Ok("1".to_string()),
+        "CAT" => Ok("2".to_string()),
+        _ => Err(env::VarError::NotPresent),
+    }
+    #[cfg(target = "wasi")]
+    env::var(var_name)
+}
+
+fn main() {
+    #[cfg(not(target = "wasi"))]
+    let mut env_vars = vec!["DOG=1".to_string(), "CAT=2".to_string()];
+    #[cfg(target = "wasi")]
+    let mut env_vars = env::vars()
+        .map(|(key, value)| format!("{}={}", key, value))
+        .collect::<Vec<String>>();
+
+    env_vars.sort();
+
+    println!("Env vars:");
+    for e in env_vars {
+        println!("{}", e);
+    }
+
+    env::set_var("WASI_ENVVAR_TEST", "HELLO");
+
+    println!("DOG {:?}", get_env_var("DOG"));
+    println!("DOG_TYPE {:?}", get_env_var("DOG_TYPE"));
+    println!("SET VAR {:?}", env::var("WASI_ENVVAR_TEST"));
+}