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

Second solution to the rewind buffer issue: allocate memory for it using memory.grow #4659

Closed
wants to merge 3 commits into from
Closed
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
2 changes: 0 additions & 2 deletions lib/journal/src/concrete/archived.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,4 @@ pub struct JournalWasiMemoryLayout {
pub stack_lower: u64,
pub guard_size: u64,
pub stack_size: u64,
pub tls_base: u64,
pub tls_size: u64,
}
6 changes: 0 additions & 6 deletions lib/journal/src/concrete/archived_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,6 @@ impl From<JournalWasiMemoryLayout> for WasiMemoryLayout {
stack_lower: value.stack_lower,
guard_size: value.guard_size,
stack_size: value.stack_size,
tls_base: value.tls_base,
tls_size: value.tls_size,
}
}
}
Expand All @@ -629,8 +627,6 @@ impl From<&'_ ArchivedJournalWasiMemoryLayout> for WasiMemoryLayout {
stack_lower: value.stack_lower,
guard_size: value.guard_size,
stack_size: value.stack_size,
tls_base: value.tls_base,
tls_size: value.tls_size,
}
}
}
Expand All @@ -642,8 +638,6 @@ impl From<WasiMemoryLayout> for JournalWasiMemoryLayout {
stack_lower: value.stack_lower,
guard_size: value.guard_size,
stack_size: value.stack_size,
tls_base: value.tls_base,
tls_size: value.tls_size,
}
}
}
Expand Down
8 changes: 0 additions & 8 deletions lib/journal/src/concrete/log_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,6 @@ mod tests {
stack_lower: 1024,
guard_size: 16,
stack_size: 1024,
tls_base: 16,
tls_size: 32,
},
start: wasmer_wasix_types::wasix::ThreadStartType::MainThread,
})
Expand Down Expand Up @@ -376,8 +374,6 @@ mod tests {
stack_lower: 1024,
guard_size: 16,
stack_size: 1024,
tls_base: 16,
tls_size: 32,
},
start: wasmer_wasix_types::wasix::ThreadStartType::MainThread,
})
Expand Down Expand Up @@ -428,8 +424,6 @@ mod tests {
stack_lower: 1024,
guard_size: 16,
stack_size: 1024,
tls_base: 16,
tls_size: 32,
},
start: wasmer_wasix_types::wasix::ThreadStartType::MainThread,
})
Expand Down Expand Up @@ -477,8 +471,6 @@ mod tests {
stack_lower: 1024,
guard_size: 16,
stack_size: 1024,
tls_base: 16,
tls_size: 32,
},
start: wasmer_wasix_types::wasix::ThreadStartType::MainThread,
})
Expand Down
2 changes: 0 additions & 2 deletions lib/journal/src/concrete/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ pub fn test_record_set_thread() {
stack_lower: 1024,
guard_size: 16,
stack_size: 1024,
tls_base: 16,
tls_size: 32,
},
start: wasmer_wasix_types::wasix::ThreadStartType::MainThread,
});
Expand Down
4 changes: 0 additions & 4 deletions lib/wasi-types/src/wasix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,4 @@ pub struct WasiMemoryLayout {
pub guard_size: u64,
/// Total size of the stack
pub stack_size: u64,
/// Base address of the TLS area
pub tls_base: u64,
/// Total size of the TLS area
pub tls_size: u64,
}
4 changes: 4 additions & 0 deletions lib/wasix/src/os/task/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ pub struct WasiThread {

// This is used for stack rewinds
rewind: Option<RewindResult>,
/// The location of the host-allocated rewind buffer for this thread,
/// if one has been allocated.
pub rewind_buffer: Option<u64>,
}

impl WasiThread {
Expand Down Expand Up @@ -276,6 +279,7 @@ impl WasiThread {
layout,
start,
rewind: None,
rewind_buffer: None,
}
}

Expand Down
24 changes: 22 additions & 2 deletions lib/wasix/src/state/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use rand::Rng;
use virtual_fs::{FileSystem, FsError, StaticFile, VirtualFile};
use virtual_net::DynVirtualNetworking;
use wasmer::{
AsStoreMut, AsStoreRef, FunctionEnvMut, Global, Imports, Instance, Memory, MemoryType,
MemoryView, Module, TypedFunction,
AsStoreMut, AsStoreRef, FunctionEnvMut, Global, Imports, Instance, Memory, MemoryError,
MemoryType, MemoryView, Module, TypedFunction, WASM_PAGE_SIZE,
};
use wasmer_config::package::PackageSource;
use wasmer_wasix_types::{
Expand Down Expand Up @@ -931,6 +931,26 @@ impl WasiEnv {
self.try_inner().map(|i| i.memory_clone())
}

pub(crate) fn get_or_allocate_rewind_buffer(
&mut self,
store: &mut impl AsStoreMut,
) -> Result<u64, MemoryError> {
match self.thread.rewind_buffer {
Some(b) => Ok(b),
None => {
let old_pages = self
.try_memory()
.ok_or_else(|| MemoryError::InvalidMemory {
reason: String::from("WasiEnv not initialized properly"),
})?
.grow(store, 1)?;
let ptr = old_pages.0 as u64 * WASM_PAGE_SIZE as u64;
self.thread.rewind_buffer = Some(ptr);
Ok(ptr)
}
}
}

/// Get the WASI state
pub(crate) fn state(&self) -> &WasiState {
&self.state
Expand Down
45 changes: 0 additions & 45 deletions lib/wasix/src/state/func_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,6 @@ use crate::{
const DEFAULT_STACK_SIZE: u64 = 1_048_576u64;
const DEFAULT_STACK_BASE: u64 = DEFAULT_STACK_SIZE;

/// The default size and base address of the TLS area for modules that
/// don't export __tls_base and __tls_size. This is the default value
/// used by clang-15 at the time of writing this code.
const DEFAULT_TLS_BASE: u64 = 1024u64;

/// This is merely an attempt at a guess. We're avoiding the __pthread
/// struct that each thread has (hoping it comes first), but there's
/// no telling how many thread statics a module has and how much space
/// they take.
const DEFAULT_TLS_SIZE: u64 = 1024u64;

#[derive(Clone, Debug)]
pub struct WasiFunctionEnv {
pub env: FunctionEnv<WasiEnv>,
Expand Down Expand Up @@ -158,18 +147,6 @@ impl WasiFunctionEnv {
|v| Ok(v.clone()),
)?;

let tls_base = instance
.exports
.get_global("__tls_base")
.map(|a| a.clone())
.ok();

let tls_size = instance
.exports
.get_global("__tls_size")
.map(|a| a.clone())
.ok();

let new_inner = WasiInstanceHandles::new(memory, store, instance);
let stack_pointer = new_inner.stack_pointer.clone();

Expand All @@ -196,34 +173,12 @@ impl WasiFunctionEnv {
));
}

let tls_base = if let Some(tls_base) = tls_base {
match tls_base.get(store) {
wasmer::Value::I32(a) => a as u64,
wasmer::Value::I64(a) => a as u64,
_ => DEFAULT_TLS_BASE,
}
} else {
DEFAULT_TLS_BASE
};

let tls_size = if let Some(tls_size) = tls_size {
match tls_size.get(store) {
wasmer::Value::I32(a) => a as u64,
wasmer::Value::I64(a) => a as u64,
_ => DEFAULT_TLS_SIZE,
}
} else {
DEFAULT_TLS_SIZE
};

// Update the stack layout which is need for asyncify
let env = self.data_mut(store);
let tid = env.tid();
let layout = &mut env.layout;
layout.stack_upper = stack_base;
layout.stack_size = layout.stack_upper - layout.stack_lower;
layout.tls_base = tls_base;
layout.tls_size = tls_size;

// Replace the thread object itself
env.thread.set_memory_layout(layout.clone());
Expand Down
71 changes: 26 additions & 45 deletions lib/wasix/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use futures::{
use tracing::instrument;
pub use wasi::*;
pub use wasix::*;
use wasmer::WASM_PAGE_SIZE;
use wasmer_journal::SnapshotTrigger;
use wasmer_wasix_types::wasix::ThreadStartType;

Expand Down Expand Up @@ -870,21 +871,6 @@ pub(crate) fn get_stack_upper(env: &WasiEnv) -> u64 {
env.layout.stack_upper
}

fn get_unwind_buffer_start(env: &WasiEnv) -> u64 {
let stack_lower = env.layout.stack_lower;
let stack_upper = env.layout.stack_upper;
let tls_base = env.layout.tls_base;

if stack_upper.saturating_sub(tls_base) < tls_base.saturating_sub(stack_lower) {
// tls_base is closer to stack_upper, so we assume TLS lives at the top of the stack.
// This is the case for threads spawned by wasix-libc.
stack_lower
} else {
// Otherwise, we assume TLS lives on the bottom, in which case we have to "skip" it.
tls_base + env.layout.tls_size
}
}

pub(crate) unsafe fn get_memory_stack_pointer(
ctx: &mut FunctionEnvMut<'_, WasiEnv>,
) -> Result<u64, String> {
Expand Down Expand Up @@ -1139,6 +1125,11 @@ where
// Get the current stack pointer (this will be used to determine the
// upper limit of stack space remaining to unwind into)
let (env, mut store) = ctx.data_and_store_mut();

let rewind_buffer_start = wasi_try_ok!(env
.get_or_allocate_rewind_buffer(&mut store)
.map_err(|_| Errno::Fault));

let memory_stack = match get_memory_stack::<M>(env, &mut store) {
Ok(a) => a,
Err(err) => {
Expand All @@ -1147,17 +1138,17 @@ where
}
};

// Perform a check to see if we have enough room
let env = ctx.data();
// Perform a check to see if we have enough room
let memory = unsafe { env.memory_view(&ctx) };

// Write the addresses to the start of the stack space
let unwind_pointer = get_unwind_buffer_start(env);
let unwind_data_start =
unwind_pointer + (std::mem::size_of::<__wasi_asyncify_t<M::Offset>>() as u64);
let asyncify_t_size = std::mem::size_of::<__wasi_asyncify_t<M::Offset>>() as u64;
let unwind_pointer = rewind_buffer_start;
let unwind_data_start = unwind_pointer + asyncify_t_size;
let unwind_data = __wasi_asyncify_t::<M::Offset> {
start: wasi_try_ok!(unwind_data_start.try_into().map_err(|_| Errno::Overflow)),
end: wasi_try_ok!((env.layout.stack_upper - memory_stack.len() as u64)
end: wasi_try_ok!((WASM_PAGE_SIZE as u64 - asyncify_t_size)
.try_into()
.map_err(|_| Errno::Overflow)),
};
Expand All @@ -1183,20 +1174,8 @@ where
// Set callback that will be invoked when this process finishes
let env = ctx.data();
let unwind_stack_begin: u64 = unwind_data.start.into();
let total_stack_space = env.layout.stack_size;
let func = ctx.as_ref();
trace!(
stack_upper = env.layout.stack_upper,
stack_lower = env.layout.stack_lower,
tls_base = env.layout.tls_base,
tls_size = env.layout.tls_size,
%unwind_pointer,
"wasi[{}:{}]::unwinding (used_stack_space={} total_stack_space={})",
ctx.data().pid(),
ctx.data().tid(),
memory_stack.len(),
total_stack_space
);
trace!("wasi[{}:{}]::unwinding", ctx.data().pid(), ctx.data().tid(),);
ctx.as_store_mut().on_called(move |mut store| {
let mut ctx = func.into_mut(&mut store);
let env = ctx.data();
Expand All @@ -1214,11 +1193,9 @@ where
let unwind_stack_finish: u64 = unwind_data_result.start.into();
let unwind_size = unwind_stack_finish - unwind_stack_begin;
trace!(
%unwind_pointer,
"wasi[{}:{}]::unwound (memory_stack_size={} unwind_size={})",
"wasi[{}:{}]::unwound (unwind_size={})",
ctx.data().pid(),
ctx.data().tid(),
memory_stack.len(),
unwind_size
);

Expand Down Expand Up @@ -1289,6 +1266,11 @@ pub fn rewind_ext<M: MemorySize>(
store_data: Bytes,
rewind_result: RewindResultType,
) -> Errno {
let (env, mut store) = ctx.data_and_store_mut();
let rewind_buffer_start = wasi_try!(env
.get_or_allocate_rewind_buffer(&mut store)
.map_err(|_| Errno::Fault));

// Store the memory stack so that it can be restored later
ctx.data_mut().thread.set_rewind(RewindResult {
memory_stack,
Expand All @@ -1314,22 +1296,21 @@ pub fn rewind_ext<M: MemorySize>(
};

// Write the addresses to the start of the stack space
let rewind_pointer = get_unwind_buffer_start(env);
let rewind_data_start =
rewind_pointer + (std::mem::size_of::<__wasi_asyncify_t<M::Offset>>() as u64);
let asyncify_t_size = std::mem::size_of::<__wasi_asyncify_t<M::Offset>>() as u64;
let rewind_pointer = rewind_buffer_start;
let rewind_data_start = rewind_pointer + asyncify_t_size;
let rewind_data_end = rewind_data_start + (rewind_stack.len() as u64);
if rewind_data_end > env.layout.stack_upper {
if rewind_stack.len() as u64 > WASM_PAGE_SIZE as u64 - asyncify_t_size {
warn!(
"attempting to rewind a stack bigger than the allocated stack space ({} > {})",
rewind_data_end, env.layout.stack_upper
"attempting to rewind a stack bigger than the allocated buffer space ({} > {})",
rewind_stack.len(),
WASM_PAGE_SIZE as u64 - asyncify_t_size
);
return Errno::Overflow;
}
let rewind_data = __wasi_asyncify_t::<M::Offset> {
start: wasi_try!(rewind_data_end.try_into().map_err(|_| Errno::Overflow)),
end: wasi_try!(env
.layout
.stack_upper
end: wasi_try!((WASM_PAGE_SIZE as u64 - asyncify_t_size)
.try_into()
.map_err(|_| Errno::Overflow)),
};
Expand Down
5 changes: 0 additions & 5 deletions lib/wasix/src/syscalls/wasix/thread_spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,11 @@ pub fn thread_spawn_internal_from_wasi<M: MemorySize>(
let tls_base: u64 = start.tls_base.try_into().map_err(|_| Errno::Overflow)?;
let stack_lower = stack_upper - stack_size;

// TLS size is constant
let tls_size = env.layout.tls_size;

WasiMemoryLayout {
stack_upper,
stack_lower,
guard_size,
stack_size,
tls_base,
tls_size,
}
};
tracing::trace!("spawn with layout {:?}", layout);
Expand Down
Loading