Skip to content

Commit

Permalink
Provenance preparation for Pulley (bytecodealliance#10043)
Browse files Browse the repository at this point in the history
* Provenance preparation for Pulley

This commit is an internal refactoring of Wasmtime's runtime to prepare
to execute Pulley in MIRI. Currently today this is not possible because
Pulley does not properly respect either strict or permissive provenance
models. The goal of this refactoring is to enable fixing this in a
future commit that doesn't touch everything in the codebase. Instead
everything is touched here in this commit.

The basic problem with Pulley is that it is incompatible with the strict
provenance model of Rust which means that we'll be using "exposed
provenance" APIs to satisfy Rust's soundness requirements. In this model
we must explicitly call `ptr.expose_provenance()` on any pointers
which are exposed to compiled code. Arguably we should also be already
doing this for natively-compiled code but I am not certain about how
strictly this is required.

Currently in Wasmtime today we call `ptr.expose_provenance()` nowhere.
It also turns out, though, that we share quite a few pointers in quite a
few places with compiled code. This creates a bit of a problem! The
solution settled on in this commit looks like:

* A new marker trait, `VmSafe`, is introduced. This trait is used to
  represent "safe to share with compiled code" types and enumerates some
  properties such as defined ABIs, primitives wrappers match primitive
  ABIs, and notably "does not contain a pointer".

* A new type, `VmPtr<T>`, is added to represent pointers shared with
  compiled code. Internally for now this is just `SendSyncPtr<T>` but in
  the future it will be `usize`. By using `SendSyncPtr<T>` it shouldn't
  actually really change anything today other than requiring a lot of
  refactoring to get the types to line up.

* The core `vmctx_plus_offset*` methods are updated to require
  `T: VmSafe`. Previously they allowed any `T` which is relatively
  dangerous to store any possible Rust type in Cranelift-accessible
  areas.

These three fundamental changes were introduced in this commit. All
further changes were refactoring necessary to get everything working
after these changes. For example many types in `vmcontext.rs`, such as
`VMFuncRef`, have changed to using `VmPtr<T>` instead of `NonNull<T>` or
`*mut T`. This is a pretty expansive change which resulted in touching a
lot of places.

One premise of `VmPtr<T>` is that it's non-null. This was an
additional refactoring that updated a lot of places where previously
`*mut T` was used and now either `VmPtr<T>` or `NonNull<T>` is used.

In the end the intention is that `VmPtr<T>` is used whenever pointers
are store in memory that can be accessed from Cranelift. When operating
inside of the runtime `NonNull<T>` or `SendSyncPtr<T>` is preferred
instead.

As a final note, no provenance changes have actually happened yet. For
example this doesn't fix Pulley in MIRI. What it does enable, though, is
that the future commit to fix Pulley in MIRI will be much smaller with
this having already landed.

* Run the full test suite in PR CI

prtest:full

* Minor CI issues

* Fix no_std build

* Fix miri build

* Don't use `VmPtr` in FFI function signatures

Use `NonNull` or `*mut u8` as appropriate for function signatures
instead. It shouldn't be required to use `VmPtr` during the handoff to
compiled code as we've already annotated the pointer going out.

* Fix rebase conflict

* Review comments
  • Loading branch information
alexcrichton authored Jan 23, 2025
1 parent e4699f7 commit b86b968
Show file tree
Hide file tree
Showing 45 changed files with 707 additions and 375 deletions.
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/component/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ impl Component {
.info
.resource_drop_wasm_to_array_trampoline
.as_ref()
.map(|i| self.func(i).cast());
.map(|i| self.func(i).cast().into());
VMFuncRef {
wasm_call,
..*dtor.func_ref()
Expand Down
8 changes: 5 additions & 3 deletions crates/wasmtime/src/runtime/component/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,11 @@ impl Func {
crate::Func::call_unchecked_raw(
store,
export.func_ref,
core::ptr::slice_from_raw_parts_mut(
NonNull::new(core::ptr::slice_from_raw_parts_mut(
space.as_mut_ptr().cast(),
mem::size_of_val(space) / mem::size_of::<ValRaw>(),
),
))
.unwrap(),
)?;

// Note that `.assume_init_ref()` here is unsafe but we're relying
Expand Down Expand Up @@ -622,7 +623,8 @@ impl Func {
crate::Func::call_unchecked_raw(
&mut store,
func.func_ref,
core::ptr::slice_from_raw_parts(&post_return_arg, 1).cast_mut(),
NonNull::new(core::ptr::slice_from_raw_parts(&post_return_arg, 1).cast_mut())
.unwrap(),
)?;
}

Expand Down
30 changes: 15 additions & 15 deletions crates/wasmtime/src/runtime/component/func/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::prelude::*;
use crate::runtime::vm::component::{
ComponentInstance, InstanceFlags, VMComponentContext, VMLowering, VMLoweringCallee,
};
use crate::runtime::vm::{VMFuncRef, VMMemoryDefinition, VMOpaqueContext};
use crate::runtime::vm::{VMFuncRef, VMGlobalDefinition, VMMemoryDefinition, VMOpaqueContext};
use crate::{AsContextMut, CallHook, StoreContextMut, ValRaw};
use alloc::sync::Arc;
use core::any::Any;
Expand Down Expand Up @@ -39,22 +39,22 @@ impl HostFunc {
}

extern "C" fn entrypoint<T, F, P, R>(
cx: *mut VMOpaqueContext,
data: *mut u8,
cx: NonNull<VMOpaqueContext>,
data: NonNull<u8>,
ty: u32,
flags: *mut u8,
flags: NonNull<VMGlobalDefinition>,
memory: *mut VMMemoryDefinition,
realloc: *mut VMFuncRef,
string_encoding: u8,
storage: *mut MaybeUninit<ValRaw>,
storage: NonNull<MaybeUninit<ValRaw>>,
storage_len: usize,
) -> bool
where
F: Fn(StoreContextMut<T>, P) -> Result<R>,
P: ComponentNamedList + Lift + 'static,
R: ComponentNamedList + Lower + 'static,
{
let data = data as *const F;
let data = data.as_ptr() as *const F;
unsafe {
call_host_and_handle_result::<T>(cx, |instance, types, store| {
call_host::<_, _, _, _>(
Expand All @@ -66,7 +66,7 @@ impl HostFunc {
memory,
realloc,
StringEncoding::from_u8(string_encoding).unwrap(),
core::slice::from_raw_parts_mut(storage, storage_len),
NonNull::slice_from_raw_parts(storage, storage_len).as_mut(),
|store, args| (*data)(store, args),
)
})
Expand Down Expand Up @@ -290,15 +290,15 @@ fn validate_inbounds<T: ComponentType>(memory: &[u8], ptr: &ValRaw) -> Result<us
}

unsafe fn call_host_and_handle_result<T>(
cx: *mut VMOpaqueContext,
cx: NonNull<VMOpaqueContext>,
func: impl FnOnce(
*mut ComponentInstance,
&Arc<ComponentTypes>,
StoreContextMut<'_, T>,
) -> Result<()>,
) -> bool {
let cx = VMComponentContext::from_opaque(cx);
let instance = (*cx).instance();
let instance = cx.as_ref().instance();
let types = (*instance).component_types();
let raw_store = (*instance).store();
let mut store = StoreContextMut(&mut *raw_store.cast());
Expand Down Expand Up @@ -422,20 +422,20 @@ fn validate_inbounds_dynamic(abi: &CanonicalAbiInfo, memory: &[u8], ptr: &ValRaw
}

extern "C" fn dynamic_entrypoint<T, F>(
cx: *mut VMOpaqueContext,
data: *mut u8,
cx: NonNull<VMOpaqueContext>,
data: NonNull<u8>,
ty: u32,
flags: *mut u8,
flags: NonNull<VMGlobalDefinition>,
memory: *mut VMMemoryDefinition,
realloc: *mut VMFuncRef,
string_encoding: u8,
storage: *mut MaybeUninit<ValRaw>,
storage: NonNull<MaybeUninit<ValRaw>>,
storage_len: usize,
) -> bool
where
F: Fn(StoreContextMut<'_, T>, &[Val], &mut [Val]) -> Result<()> + Send + Sync + 'static,
{
let data = data as *const F;
let data = data.as_ptr() as *const F;
unsafe {
call_host_and_handle_result(cx, |instance, types, store| {
call_host_dynamic::<T, _>(
Expand All @@ -447,7 +447,7 @@ where
memory,
realloc,
StringEncoding::from_u8(string_encoding).unwrap(),
core::slice::from_raw_parts_mut(storage, storage_len),
NonNull::slice_from_raw_parts(storage, storage_len).as_mut(),
|store, params, results| (*data)(store, params, results),
)
})
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmtime/src/runtime/component/func/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ impl Options {
// is an optional configuration in canonical ABI options.
unsafe {
let memory = self.memory.unwrap().as_ref();
core::slice::from_raw_parts(memory.base, memory.current_length())
core::slice::from_raw_parts(memory.base.as_ptr(), memory.current_length())
}
}

Expand All @@ -149,7 +149,7 @@ impl Options {
// See comments in `memory` about the unsafety
unsafe {
let memory = self.memory.unwrap().as_ref();
core::slice::from_raw_parts_mut(memory.base, memory.current_length())
core::slice::from_raw_parts_mut(memory.base.as_ptr(), memory.current_length())
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/wasmtime/src/runtime/component/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::store::{StoreOpaque, Stored};
use crate::{AsContextMut, Engine, Module, StoreContextMut};
use alloc::sync::Arc;
use core::marker;
use core::ptr::{self, NonNull};
use core::ptr::NonNull;
use wasmtime_environ::{component::*, EngineOrModuleTypeIndex};
use wasmtime_environ::{EntityIndex, EntityType, Global, PrimaryMap, WasmValType};

Expand Down Expand Up @@ -376,7 +376,7 @@ impl InstanceData {
CoreDef::InstanceFlags(idx) => {
crate::runtime::vm::Export::Global(crate::runtime::vm::ExportGlobal {
definition: self.state.instance_flags(*idx).as_raw(),
vmctx: ptr::null_mut(),
vmctx: None,
global: Global {
wasm_ty: WasmValType::I32,
mutability: true,
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/component/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ impl ResourceAny {
// destructors have al been previously type-checked and are guaranteed
// to take one i32 argument and return no results, so the parameters
// here should be configured correctly.
unsafe { crate::Func::call_unchecked_raw(store, dtor, &mut args) }
unsafe { crate::Func::call_unchecked_raw(store, dtor, NonNull::from(&mut args)) }
}

fn lower_to_index<U>(&self, cx: &mut LowerContext<'_, U>, ty: InterfaceType) -> Result<u32> {
Expand Down
17 changes: 9 additions & 8 deletions crates/wasmtime/src/runtime/externals/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl Global {
unsafe {
let store = store.as_context_mut();
let mut store = AutoAssertNoGc::new(store.0);
let definition = &*store[self.0].definition;
let definition = store[self.0].definition.as_ref();
match self._ty(&store).content() {
ValType::I32 => Val::from(*definition.as_i32()),
ValType::I64 => Val::from(*definition.as_i64()),
Expand Down Expand Up @@ -181,7 +181,7 @@ impl Global {
val.ensure_matches_ty(&store, global_ty.content())
.context("type mismatch: attempt to set global to value of wrong type")?;
unsafe {
let definition = &mut *store[self.0].definition;
let definition = store[self.0].definition.as_mut();
match val {
Val::I32(i) => *definition.as_i32_mut() = i,
Val::I64(i) => *definition.as_i64_mut() = i,
Expand Down Expand Up @@ -222,7 +222,7 @@ impl Global {
return;
}

if let Some(gc_ref) = unsafe { (*store[self.0].definition).as_gc_ref() } {
if let Some(gc_ref) = unsafe { store[self.0].definition.as_ref().as_gc_ref() } {
let gc_ref = NonNull::from(gc_ref);
let gc_ref = SendSyncPtr::new(gc_ref);
unsafe {
Expand All @@ -240,9 +240,10 @@ impl Global {
.global
.wasm_ty
.canonicalize_for_runtime_usage(&mut |module_index| {
crate::runtime::vm::Instance::from_vmctx(wasmtime_export.vmctx, |instance| {
instance.engine_type_index(module_index)
})
crate::runtime::vm::Instance::from_vmctx(
wasmtime_export.vmctx.unwrap(),
|instance| instance.engine_type_index(module_index),
)
});

Global(store.store_data_mut().insert(wasmtime_export))
Expand All @@ -254,7 +255,7 @@ impl Global {

pub(crate) fn vmimport(&self, store: &StoreOpaque) -> crate::runtime::vm::VMGlobalImport {
crate::runtime::vm::VMGlobalImport {
from: store[self.0].definition,
from: store[self.0].definition.into(),
}
}

Expand All @@ -264,7 +265,7 @@ impl Global {
/// `StoreData` multiple times and becomes multiple `wasmtime::Global`s,
/// this hash key will be consistent across all of these globals.
pub(crate) fn hash_key(&self, store: &StoreOpaque) -> impl core::hash::Hash + Eq + use<> {
store[self.0].definition as usize
store[self.0].definition.as_ptr() as usize
}
}

Expand Down
12 changes: 6 additions & 6 deletions crates/wasmtime/src/runtime/externals/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl Table {
vmctx, definition, ..
} = store[self.0];
crate::runtime::vm::Instance::from_vmctx(vmctx, |handle| {
let idx = handle.table_index(&*definition);
let idx = handle.table_index(definition.as_ref());
handle.get_defined_table_with_lazy_init(idx, lazy_init_range)
})
}
Expand Down Expand Up @@ -229,7 +229,7 @@ impl Table {
pub(crate) fn internal_size(&self, store: &StoreOpaque) -> u64 {
// unwrap here should be ok because the runtime should always guarantee
// that we can fit the number of elements in a 64-bit integer.
unsafe { u64::try_from((*store[self.0].definition).current_elements).unwrap() }
unsafe { u64::try_from(store[self.0].definition.as_ref().current_elements).unwrap() }
}

/// Grows the size of this table by `delta` more elements, initialization
Expand Down Expand Up @@ -262,7 +262,7 @@ impl Table {
match (*table).grow(delta, init, store)? {
Some(size) => {
let vm = (*table).vmtable();
*store[self.0].definition = vm;
store[self.0].definition.write(vm);
// unwrap here should be ok because the runtime should always guarantee
// that we can fit the table size in a 64-bit integer.
Ok(u64::try_from(size).unwrap())
Expand Down Expand Up @@ -421,8 +421,8 @@ impl Table {
pub(crate) fn vmimport(&self, store: &StoreOpaque) -> crate::runtime::vm::VMTableImport {
let export = &store[self.0];
crate::runtime::vm::VMTableImport {
from: export.definition,
vmctx: export.vmctx,
from: export.definition.into(),
vmctx: export.vmctx.into(),
}
}

Expand All @@ -433,7 +433,7 @@ impl Table {
/// this hash key will be consistent across all of these tables.
#[allow(dead_code)] // Not used yet, but added for consistency.
pub(crate) fn hash_key(&self, store: &StoreOpaque) -> impl core::hash::Hash + Eq + use<'_> {
store[self.0].definition as usize
store[self.0].definition.as_ptr() as usize
}
}

Expand Down
Loading

0 comments on commit b86b968

Please sign in to comment.