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 memory leak in wat2wasm function #1855

Merged
merged 6 commits into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -23,6 +23,7 @@

### Fixed

- [#1855](https://github.com/wasmerio/wasmer/pull/1855) Fix memory leak when using `wat2wasm` in the C API, the function now takes its output parameter by pointer rather than returning an allocated `wasm_byte_vec_t`.
- [#1841](https://github.com/wasmerio/wasmer/pull/1841) We will now panic when attempting to use a native function with a captured env as a host function. Previously this would silently do the wrong thing. See [#1840](https://github.com/wasmerio/wasmer/pull/1840) for info about Wasmer's support of closures as host functions.
- [#1764](https://github.com/wasmerio/wasmer/pull/1764) Fix bug in WASI `path_rename` allowing renamed files to be 1 directory below a preopened directory.

Expand Down
2 changes: 1 addition & 1 deletion lib/c-api/src/deprecated/import/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ pub unsafe extern "C" fn wasmer_wasi_generate_default_import_object() -> *mut wa
let store = get_global_store();
let mut wasi_state_builder = wasi::WasiState::new("wasmer-wasi-default-program-name");
let wasi_state = wasi_state_builder.build().unwrap();
let mut wasi_env = wasi::WasiEnv::new(wasi_state);
let wasi_env = wasi::WasiEnv::new(wasi_state);
let import_object_inner: Box<dyn NamedResolver> = Box::new(
wasi::generate_import_object_from_env(store, wasi_env, wasi::WasiVersion::Latest),
);
Expand Down
13 changes: 7 additions & 6 deletions lib/c-api/src/wasm_c_api/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,13 @@ macro_rules! wasm_declare_vec {


#[no_mangle]
pub unsafe extern "C" fn [<wasm_ $name _vec_delete>](ptr: *mut [<wasm_ $name _vec_t>]) {
let vec = &mut *ptr;
if !vec.data.is_null() {
Vec::from_raw_parts(vec.data, vec.size, vec.size);
vec.data = ::std::ptr::null_mut();
vec.size = 0;
pub unsafe extern "C" fn [<wasm_ $name _vec_delete>](ptr: Option<&mut [<wasm_ $name _vec_t>]>) {
if let Some(vec) = ptr {
if !vec.data.is_null() {
Vec::from_raw_parts(vec.data, vec.size, vec.size);
vec.data = ::std::ptr::null_mut();
vec.size = 0;
}
}
}
}
Expand Down
49 changes: 28 additions & 21 deletions lib/c-api/src/wasm_c_api/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@ pub struct wasm_module_t {
///
/// wasm_byte_vec_t wat;
/// wasmer_byte_vec_new_from_string(&wat, "(module)");
/// wasm_byte_vec_t* wasm = wat2wasm(&wat);
/// wasm_byte_vec_t wasm;
/// wat2wasm(&wat, &wasm);
///
/// wasm_module_t* module = wasm_module_new(store, wasm);
/// wasm_module_t* module = wasm_module_new(store, &wasm);
/// assert(module);
///
/// wasm_byte_vec_delete(wasm);
/// wasm_byte_vec_delete(&wasm);
/// wasm_byte_vec_delete(&wat);
/// wasm_module_delete(module);
/// wasm_store_delete(store);
Expand Down Expand Up @@ -190,11 +191,12 @@ mod tests {

wasm_byte_vec_t wat;
wasmer_byte_vec_new_from_string(&wat, "(module)");
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

assert(wasm_module_validate(store, wasm));
assert(wasm_module_validate(store, &wasm));

wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_store_delete(store);
wasm_engine_delete(engine);
Expand All @@ -216,12 +218,13 @@ mod tests {

wasm_byte_vec_t wat;
wasmer_byte_vec_new_from_string(&wat, "(module)");
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

wasm_module_t* module = wasm_module_new(store, wasm);
wasm_module_t* module = wasm_module_new(store, &wasm);
assert(module);

wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_module_delete(module);
wasm_store_delete(store);
Expand Down Expand Up @@ -251,9 +254,10 @@ mod tests {
" (table (export \"table\") 0 funcref)\n"
" (memory (export \"memory\") 1))"
);
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

wasm_module_t* module = wasm_module_new(store, wasm);
wasm_module_t* module = wasm_module_new(store, &wasm);
assert(module);

wasm_exporttype_vec_t export_types;
Expand Down Expand Up @@ -328,7 +332,7 @@ mod tests {
}

wasm_exporttype_vec_delete(&export_types);
wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_module_delete(module);
wasm_store_delete(store);
Expand Down Expand Up @@ -356,9 +360,10 @@ mod tests {
" (import \"ns\" \"table\" (table 1 2 anyfunc))\n"
" (import \"ns\" \"memory\" (memory 3 4)))"
);
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

wasm_module_t* module = wasm_module_new(store, wasm);
wasm_module_t* module = wasm_module_new(store, &wasm);
assert(module);

wasm_importtype_vec_t import_types;
Expand Down Expand Up @@ -444,7 +449,7 @@ mod tests {

wasm_importtype_vec_delete(&import_types);
wasm_module_delete(module);
wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_store_delete(store);
wasm_engine_delete(engine);
Expand All @@ -464,9 +469,10 @@ mod tests {

wasm_byte_vec_t wat;
wasmer_byte_vec_new_from_string(&wat, "(module)");
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

wasm_module_t* module = wasm_module_new(store, wasm);
wasm_module_t* module = wasm_module_new(store, &wasm);
assert(module);

wasm_byte_vec_t serialized_module;
Expand All @@ -475,7 +481,7 @@ mod tests {

wasm_module_delete(module);
wasm_byte_vec_delete(&serialized_module);
wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_store_delete(store);
wasm_engine_delete(engine);
Expand All @@ -502,9 +508,10 @@ mod tests {
" (table (export \"table\") 0 funcref)\n"
" (memory (export \"memory\") 1))"
);
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

wasm_module_t* module = wasm_module_new(store, wasm);
wasm_module_t* module = wasm_module_new(store, &wasm);
assert(module);

wasm_byte_vec_t serialized_module;
Expand All @@ -526,7 +533,7 @@ mod tests {

wasm_exporttype_vec_delete(&export_types);
wasm_module_delete(deserialized_module);
wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_store_delete(store);
wasm_engine_delete(engine);
Expand Down
4 changes: 2 additions & 2 deletions lib/c-api/src/wasm_c_api/wasi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ pub unsafe extern "C" fn wasi_env_read_stdout(
buffer_len: usize,
) -> isize {
let inner_buffer = slice::from_raw_parts_mut(buffer as *mut _, buffer_len as usize);
let mut state = env.inner.state_mut();
let mut state = env.inner.state();

let stdout = if let Ok(stdout) = state.fs.stdout_mut() {
if let Some(stdout) = stdout.as_mut() {
Expand All @@ -232,7 +232,7 @@ pub unsafe extern "C" fn wasi_env_read_stderr(
buffer_len: usize,
) -> isize {
let inner_buffer = slice::from_raw_parts_mut(buffer as *mut _, buffer_len as usize);
let mut state = env.inner.state_mut();
let mut state = env.inner.state();
let stderr = if let Ok(stderr) = state.fs.stderr_mut() {
if let Some(stderr) = stderr.as_mut() {
stderr
Expand Down
53 changes: 35 additions & 18 deletions lib/c-api/src/wasm_c_api/wat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,26 @@ use super::types::wasm_byte_vec_t;
/// In case of failure, `wat2wasm` returns `NULL`.
#[cfg(feature = "wat")]
#[no_mangle]
pub unsafe extern "C" fn wat2wasm(wat: &wasm_byte_vec_t) -> Option<Box<wasm_byte_vec_t>> {
let wat: &[u8] = wat.into_slice()?;
let result: wasm_byte_vec_t = c_try!(wasmer::wat2wasm(wat)).into_owned().into();
pub unsafe extern "C" fn wat2wasm(wat: &wasm_byte_vec_t, out: &mut wasm_byte_vec_t) {
let wat: &[u8] = match wat.into_slice() {
Some(v) => v,
_ => {
out.data = std::ptr::null_mut();
out.size = 0;
return;
}
};
let result: wasm_byte_vec_t = match wasmer::wat2wasm(wat) {
Ok(val) => val.into_owned().into(),
Err(err) => {
crate::error::update_last_error(err);
out.data = std::ptr::null_mut();
out.size = 0;
return;
}
};

Some(Box::new(result))
*out = result;
}

#[cfg(test)]
Expand All @@ -28,22 +43,23 @@ mod tests {

wasm_byte_vec_t wat;
wasmer_byte_vec_new_from_string(&wat, "(module)");
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

assert(wasm);
assert(wasm->size == 8);
assert(wasm.data);
assert(wasm.size == 8);
assert(
wasm->data[0] == 0 &&
wasm->data[1] == 'a' &&
wasm->data[2] == 's' &&
wasm->data[3] == 'm' &&
wasm->data[4] == 1 &&
wasm->data[5] == 0 &&
wasm->data[6] == 0 &&
wasm->data[7] == 0
wasm.data[0] == 0 &&
wasm.data[1] == 'a' &&
wasm.data[2] == 's' &&
wasm.data[3] == 'm' &&
wasm.data[4] == 1 &&
wasm.data[5] == 0 &&
wasm.data[6] == 0 &&
wasm.data[7] == 0
);

wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);
wasm_byte_vec_delete(&wat);
wasm_store_delete(store);
wasm_engine_delete(engine);
Expand All @@ -65,9 +81,10 @@ mod tests {

wasm_byte_vec_t wat;
wasmer_byte_vec_new_from_string(&wat, "(module");
wasm_byte_vec_t* wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

assert(!wasm);
assert(!wasm.data);
assert(wasmer_last_error_length() > 0);

wasm_byte_vec_delete(&wat);
Expand Down
25 changes: 13 additions & 12 deletions lib/c-api/tests/wasm_c_api/test-wat2wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,31 +20,32 @@ int main(int argc, const char* argv[]) {
.data = "(module)",
.size = 8,
};
wasm_byte_vec_t *wasm = wat2wasm(&wat);
wasm_byte_vec_t wasm;
wat2wasm(&wat, &wasm);

if (!wasm) {
if (!wasm.data) {
printf("> Error compiler WAT to Wasm!\n");
MarkMcCaskey marked this conversation as resolved.
Show resolved Hide resolved
return 1;
}

if (wasm->size != 8) {
if (wasm.size != 8) {
printf("The Wasm size is incorrect!\n");
return 1;
}

if (!(wasm->data[0] == 0 &&
wasm->data[1] == 'a' &&
wasm->data[2] == 's' &&
wasm->data[3] == 'm' &&
wasm->data[4] == 1 &&
wasm->data[5] == 0 &&
wasm->data[6] == 0 &&
wasm->data[7] == 0)) {
if (!(wasm.data[0] == 0 &&
wasm.data[1] == 'a' &&
wasm.data[2] == 's' &&
wasm.data[3] == 'm' &&
wasm.data[4] == 1 &&
wasm.data[5] == 0 &&
wasm.data[6] == 0 &&
wasm.data[7] == 0)) {
printf("The Wasm data is incorrect!\n");
return 1;
}

wasm_byte_vec_delete(wasm);
wasm_byte_vec_delete(&wasm);

// All done.
printf("Done.\n");
Expand Down
2 changes: 1 addition & 1 deletion lib/c-api/wasmer_wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,6 @@ int wasmer_last_error_message(char *buffer, int length);
*
* In case of failure, `wat2wasm` returns `NULL`.
MarkMcCaskey marked this conversation as resolved.
Show resolved Hide resolved
*/
wasm_byte_vec_t *wat2wasm(const wasm_byte_vec_t *wat);
void wat2wasm(const wasm_byte_vec_t *wat, wasm_byte_vec_t *out);

#endif /* WASMER_WASM_H */