Skip to content

Commit

Permalink
Review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
cfallin committed Feb 7, 2022
1 parent 9a5ace5 commit 3fdc220
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 28 deletions.
1 change: 1 addition & 0 deletions crates/runtime/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ impl Instance {
/// (imported or defined in this module) and store into the given
/// location. Used during lazy initialization the first time the
/// VMCallerCheckedAnyfunc in the VMContext is referenced.
#[cold]
fn construct_anyfunc(&mut self, index: FuncIndex, into: *mut VMCallerCheckedAnyfunc) {
let sig = self.module.functions[index];
let type_index = self.runtime_info.shared_signatures().lookup(sig);
Expand Down
62 changes: 35 additions & 27 deletions crates/runtime/src/instance/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,12 +481,17 @@ fn initialize_instance(

/// Initialize the VMContext data associated with an Instance.
///
/// Precondition: the VMContext memory must be zeroed. We omit writes
/// here to fields that should be initialized to zero. This allows the
/// caller to use an efficient means of zeroing memory (such as using
/// anonymous-mmap'd zero pages) if available, rather than falling
/// back onto a memset (or the manual equivalent) here.
unsafe fn initialize_vmcontext(instance: &mut Instance, req: InstanceAllocationRequest) {
/// Optimization: if `pre_zeroed` is set, then the VMContext memory
/// (following the `Instance`) is assumed to be zeroed. If so, we omit
/// writes here to fields that should be initialized to zero. This
/// allows the caller to use an efficient means of zeroing memory
/// (such as using anonymous-mmap'd zero pages) if available, rather
/// than falling back onto a memset (or the manual equivalent) here.
unsafe fn initialize_vmcontext(
instance: &mut Instance,
req: InstanceAllocationRequest,
pre_zeroed: bool,
) {
if let Some(store) = req.store.as_raw() {
*instance.interrupts() = (*store).vminterrupts();
*instance.epoch_ptr() = (*store).epoch_ptr();
Expand Down Expand Up @@ -536,9 +541,27 @@ unsafe fn initialize_vmcontext(instance: &mut Instance, req: InstanceAllocationR
req.imports.globals.len(),
);

// N.B.: no need to initialize the anyfuncs array; it is zeroed
// already (by the precondition for this function) and the lazy
// init will fill in entries as we take pointers to them.
// N.B.: if `pre_zeroed`, then there is no need to initialize the
// anyfuncs array; it is zeroed already (by the precondition for
// this function) and the lazy init will fill in entries as we
// take pointers to them.
//
// If so, we'll debug_assert that it's all zeroes; if not, we'll
// fill it.
let anyfuncs_bytes = std::slice::from_raw_parts_mut(
instance.anyfunc_base() as *mut u8,
instance
.module
.functions
.len()
.checked_mul(std::mem::size_of::<VMCallerCheckedAnyfunc>())
.unwrap(),
);
if pre_zeroed {
debug_assert!(anyfuncs_bytes.iter().all(|b| *b == 0));
} else {
anyfuncs_bytes.fill(0);
}

// Initialize the defined tables
let mut ptr = instance.vmctx_plus_offset(instance.offsets.vmctx_tables_begin());
Expand Down Expand Up @@ -705,7 +728,7 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator {

let host_state = std::mem::replace(&mut req.host_state, Box::new(()));

let (mut handle, vmctx_data) = {
let mut handle = {
let instance = Instance::create_raw(
&req.module,
req.runtime_info.clone(),
Expand All @@ -724,25 +747,10 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator {

ptr::write(instance_ptr, instance);

let vmctx_data = {
let vmctx_ptr = (*instance_ptr).vmctx_ptr() as *mut u8;
let vmctx_len = (*instance_ptr).offsets.size_of_vmctx() as usize;
std::slice::from_raw_parts_mut(vmctx_ptr, vmctx_len)
};

(handle, vmctx_data)
handle
};

// Zero the VMContext memory first; `initialize_vmcontext()`
// requires this.
vmctx_data.fill(0);
// Drop the &mut slice over the VMContext data before writing
// to it via `initialize_vmcontext()` below; we should not let
// its lifetime extend over the function call that initializes
// the VMContext.
drop(vmctx_data);

initialize_vmcontext(handle.instance_mut(), req);
initialize_vmcontext(handle.instance_mut(), req, /* pre_zeroed = */ false);

Ok(handle)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/runtime/src/instance/allocator/pooling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl InstancePool {
// will use either madvise(MADV_DONTNEED) (on Linux) or
// mmap-of-fresh-anonymous-memory to efficiently zero the
// instance state.
initialize_vmcontext(instance, req);
initialize_vmcontext(instance, req, /* pre_zeroed = */ true);

Ok(InstanceHandle {
instance: instance as _,
Expand Down

0 comments on commit 3fdc220

Please sign in to comment.