Skip to content

Commit

Permalink
Merge #1855
Browse files Browse the repository at this point in the history 
1855: Fix memory leak in wat2wasm function r=MarkMcCaskey a=MarkMcCaskey

Alternative to #1853 ; 

Found with @nlewycky 

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
  • Loading branch information
@bors @MarkMcCaskey
3 people authored Dec 1, 2020
2 parents 5371543 + cd55746 commit 774a4e6
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 64 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
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
View file
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
View file
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
View file
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
View file
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
55 changes: 36 additions & 19 deletions lib/c-api/src/wasm_c_api/wat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,29 @@ use super::types::wasm_byte_vec_t;
/// Parses in-memory bytes as either the WAT format, or a binary Wasm
/// module. This is wasmer-specific.
///
/// In case of failure, `wat2wasm` returns `NULL`.
/// In case of failure, `wat2wasm` sets the `out->data = NULL` and `out->size = 0`.
#[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
27 changes: 14 additions & 13 deletions lib/c-api/tests/wasm_c_api/test-wat2wasm.c
View file
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) {
printf("> Error compiler WAT to Wasm!\n");
if (!wasm.data) {
printf("> Error compiling WAT to Wasm!\n");
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
4 changes: 2 additions & 2 deletions lib/c-api/wasmer_wasm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ int wasmer_last_error_message(char *buffer, int length);
* Parses in-memory bytes as either the WAT format, or a binary Wasm
* module. This is wasmer-specific.
*
* In case of failure, `wat2wasm` returns `NULL`.
* In case of failure, `wat2wasm` sets the `out->data = NULL` and `out->size = 0`.
*/
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 */

0 comments on commit 774a4e6

Please sign in to comment.