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.

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

fix bug in wasi::environ_get, fix off by one error in env_size_get #476

Merged
merged 3 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 38 additions & 19 deletions lib/wasi/build/wasitests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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!(
Expand All @@ -104,6 +112,7 @@ fn test_{rs_module_name}() {{
\"../../{module_path}\",
\"{rs_module_name}\",
{mapdir_args},
{envvar_args},
\"../../{test_output_path}\"
);
}}
Expand All @@ -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"),
Expand Down Expand Up @@ -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)
Expand All @@ -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);
}
Expand Down
23 changes: 18 additions & 5 deletions lib/wasi/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()`
Expand All @@ -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()`
Expand All @@ -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
}
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion lib/wasi/tests/wasitests/_common.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/create_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ fn test_create_dir() {
"../../wasitests/create_dir.wasm",
"create_dir",
vec![],
vec![],
"../../wasitests/create_dir.out"
);
}
10 changes: 10 additions & 0 deletions lib/wasi/tests/wasitests/envvar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#[test]
fn test_envvar() {
assert_wasi_output!(
"../../wasitests/envvar.wasm",
"envvar",
vec![],
vec![],
"../../wasitests/envvar.out"
);
}
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/file_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ fn test_file_metadata() {
"../../wasitests/file_metadata.wasm",
"file_metadata",
vec![],
vec![],
"../../wasitests/file_metadata.out"
);
}
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/fs_sandbox_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ fn test_fs_sandbox_test() {
"../../wasitests/fs_sandbox_test.wasm",
"fs_sandbox_test",
vec![],
vec![],
"../../wasitests/fs_sandbox_test.out"
);
}
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/hello.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ fn test_hello() {
"../../wasitests/hello.wasm",
"hello",
vec![],
vec![],
"../../wasitests/hello.out"
);
}
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/mapdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ fn test_mapdir() {
"../../wasitests/mapdir.wasm",
"mapdir",
vec![],
vec![],
"../../wasitests/mapdir.out"
);
}
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#[macro_use]
mod _common;
mod create_dir;
mod envvar;
mod file_metadata;
mod fs_sandbox_test;
mod hello;
Expand Down
1 change: 1 addition & 0 deletions lib/wasi/tests/wasitests/quine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ fn test_quine() {
"../../wasitests/quine.wasm",
"quine",
vec![],
vec![],
"../../wasitests/quine.out"
);
}
Binary file removed lib/wasi/wasitests/create_dir
Binary file not shown.
Binary file modified lib/wasi/wasitests/create_dir.wasm
Binary file not shown.
6 changes: 6 additions & 0 deletions lib/wasi/wasitests/envvar.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Env vars:
CAT=2
DOG=1
DOG Ok("1")
DOG_TYPE Err(NotPresent)
SET VAR Ok("HELLO")
38 changes: 38 additions & 0 deletions lib/wasi/wasitests/envvar.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Copy link
Member

@syrusakbary syrusakbary May 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be useful to add case to retrieve a single env var when:

  • env var is present (such as DOG)
  • env var is not present (such as UNKNOWN)


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"));
}
Binary file added lib/wasi/wasitests/envvar.wasm
Binary file not shown.
Binary file removed lib/wasi/wasitests/file_metadata
Binary file not shown.
Binary file modified lib/wasi/wasitests/file_metadata.wasm
Binary file not shown.
Binary file removed lib/wasi/wasitests/fs_sandbox_test
Binary file not shown.
Binary file modified lib/wasi/wasitests/fs_sandbox_test.wasm
Binary file not shown.
Binary file removed lib/wasi/wasitests/hello
Binary file not shown.
Binary file modified lib/wasi/wasitests/hello.wasm
Binary file not shown.
Binary file removed lib/wasi/wasitests/mapdir
Binary file not shown.
Binary file modified lib/wasi/wasitests/mapdir.wasm
Binary file not shown.
Binary file removed lib/wasi/wasitests/quine
Binary file not shown.
Binary file modified lib/wasi/wasitests/quine.wasm
Binary file not shown.
Loading