Skip to content

Commit

Permalink
fix memory leak in import resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
@matklad
matklad committed Jun 24, 2021
1 parent 8d4b943 commit aa57388
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 60 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/runtime-core/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "wasmer-runtime-core-near"
version = "0.17.1"
version = "0.17.3"
description = "Wasmer runtime core library"
license = "MIT"
authors = ["The Wasmer Engineering Team <engineering@wasmer.io>"]
Expand Down
42 changes: 8 additions & 34 deletions lib/runtime-core/src/backing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,8 @@ impl LocalBacking {
),
LocalOrImport::Import(imported_func_index) => {
let vm::ImportedFunc { func, func_ctx } =
imports.vm_functions[imported_func_index];
(func, unsafe { func_ctx.as_ref() }.vmctx.as_ptr())
&imports.vm_functions[imported_func_index];
(*func, func_ctx.vmctx.as_ptr())
}
};

Expand Down Expand Up @@ -418,8 +418,8 @@ impl LocalBacking {
),
LocalOrImport::Import(imported_func_index) => {
let vm::ImportedFunc { func, func_ctx } =
imports.vm_functions[imported_func_index];
(func, unsafe { func_ctx.as_ref() }.vmctx.as_ptr())
&imports.vm_functions[imported_func_index];
(*func, func_ctx.vmctx.as_ptr())
}
};

Expand Down Expand Up @@ -549,24 +549,6 @@ impl ImportBacking {
})
}
}

/// Gets a `ImportedFunc` from the given `ImportedFuncIndex`.
pub fn imported_func(&self, index: ImportedFuncIndex) -> vm::ImportedFunc {
self.vm_functions[index].clone()
}
}

impl Drop for ImportBacking {
fn drop(&mut self) {
// Properly drop the `vm::FuncCtx` in `vm::ImportedFunc`.
for (_imported_func_index, imported_func) in (*self.vm_functions).iter_mut() {
let func_ctx_ptr = imported_func.func_ctx.as_ptr();

if !func_ctx_ptr.is_null() {
let _: Box<vm::FuncCtx> = unsafe { Box::from_raw(func_ctx_ptr) };
}
}
}
}

fn import_functions(
Expand Down Expand Up @@ -602,10 +584,7 @@ fn import_functions(
if *expected_sig == *signature {
functions.push(vm::ImportedFunc {
func: func.inner(),
func_ctx: NonNull::new(Box::into_raw(Box::new(vm::FuncCtx {
// ^^^^^^^^ `vm::FuncCtx` is purposely leaked.
// It is dropped by the specific `Drop`
// implementation of `ImportBacking`.
func_ctx: Box::new(vm::FuncCtx {
vmctx: NonNull::new(match ctx {
Context::External(vmctx) => vmctx,
Context::ExternalWithEnv(vmctx_, _) => {
Expand All @@ -622,8 +601,7 @@ fn import_functions(
Context::ExternalWithEnv(_, func_env) => func_env,
_ => None,
},
})))
.unwrap(),
}),
});
} else {
link_errors.push(LinkError::IncorrectImportSignature {
Expand Down Expand Up @@ -655,14 +633,10 @@ fn import_functions(

functions.push(vm::ImportedFunc {
func: always_trap.get_vm_func().as_ptr(),
func_ctx: NonNull::new(Box::into_raw(Box::new(vm::FuncCtx {
// ^^^^^^^^ `vm::FuncCtx` is purposely leaked.
// It is dropped by the specific `Drop`
// implementation of `ImportBacking`.
func_ctx: Box::new(vm::FuncCtx {
vmctx: NonNull::new(vmctx).expect("`vmctx` must not be null."),
func_env: None,
})))
.unwrap(),
}),
});
} else {
link_errors.push(LinkError::ImportNotFound {
Expand Down
35 changes: 15 additions & 20 deletions lib/runtime-core/src/instance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,12 @@ impl Instance {

let ctx_ptr = match start_index.local_or_import(&instance.module.info) {
LocalOrImport::Local(_) => instance.inner.vmctx,
LocalOrImport::Import(imported_func_index) => unsafe {
LocalOrImport::Import(imported_func_index) => {
instance.inner.import_backing.vm_functions[imported_func_index]
.func_ctx
.as_ref()
.vmctx
.as_ptr()
}
.vmctx
.as_ptr(),
};

let sig_index = *instance
Expand Down Expand Up @@ -427,7 +426,7 @@ impl InstanceInner {
),
LocalOrImport::Import(imported_func_index) => {
let imported_func = &self.import_backing.vm_functions[imported_func_index];
let func_ctx = unsafe { imported_func.func_ctx.as_ref() };
let func_ctx = &*imported_func.func_ctx;

(
imported_func.func as *const _,
Expand Down Expand Up @@ -555,13 +554,11 @@ fn call_func_with_index(

let ctx_ptr = match func_index.local_or_import(info) {
LocalOrImport::Local(_) => local_ctx,
LocalOrImport::Import(imported_func_index) => unsafe {
import_backing.vm_functions[imported_func_index]
.func_ctx
.as_ref()
}
.vmctx
.as_ptr(),
LocalOrImport::Import(imported_func_index) => import_backing.vm_functions
[imported_func_index]
.func_ctx
.vmctx
.as_ptr(),
};

let wasm = runnable
Expand Down Expand Up @@ -882,13 +879,11 @@ impl<'a, Args: WasmTypeList, Rets: WasmTypeList> Exportable<'a> for Func<'a, Arg

let ctx = match func_index.local_or_import(&module.info) {
LocalOrImport::Local(_) => inst_inner.vmctx,
LocalOrImport::Import(imported_func_index) => unsafe {
inst_inner.import_backing.vm_functions[imported_func_index]
.func_ctx
.as_ref()
}
.vmctx
.as_ptr(),
LocalOrImport::Import(imported_func_index) => inst_inner.import_backing.vm_functions
[imported_func_index]
.func_ctx
.vmctx
.as_ptr(),
};

let func_wasm_inner = module
Expand All @@ -909,7 +904,7 @@ impl<'a, Args: WasmTypeList, Rets: WasmTypeList> Exportable<'a> for Func<'a, Arg

(
NonNull::new(imported_func.func as *mut _).unwrap(),
unsafe { imported_func.func_ctx.as_ref() }.func_env,
imported_func.func_ctx.func_env,
)
}
};
Expand Down
4 changes: 2 additions & 2 deletions lib/runtime-core/src/typed_func.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ macro_rules! impl_traits {
.iter()
.find_map(|(_, imported_func)| {
if imported_func.func == self_pointer {
Some(imported_func.func_ctx)
Some(NonNull::from(&*imported_func.func_ctx))
} else {
None
}
Expand Down Expand Up @@ -752,7 +752,7 @@ macro_rules! impl_traits {
.iter()
.find_map(|(_, imported_func)| {
if imported_func.func == self_pointer {
Some(imported_func.func_ctx)
Some(NonNull::from(&*imported_func.func_ctx))
} else {
None
}
Expand Down
4 changes: 2 additions & 2 deletions lib/runtime-core/src/vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,14 +588,14 @@ impl FuncCtx {

/// An imported function is a function pointer associated to a
/// function context.
#[derive(Debug, Clone)]
#[derive(Debug)]
#[repr(C)]
pub struct ImportedFunc {
/// Pointer to the function itself.
pub(crate) func: *const Func,

/// Mutable non-null pointer to [`FuncCtx`].
pub(crate) func_ctx: NonNull<FuncCtx>,
pub(crate) func_ctx: Box<FuncCtx>,
}

// Manually implemented because ImportedFunc contains raw pointers
Expand Down

0 comments on commit aa57388

Please sign in to comment.