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

Make Wasmtime compatible with Stacked Borrows in MIRI #6338

Merged
merged 11 commits into from
May 9, 2023
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ jobs:
-p wasmtime-runtime \
-p wasmtime-environ
env:
MIRIFLAGS: -Zmiri-tree-borrows
MIRIFLAGS: -Zmiri-strict-provenance

# common logic to cancel the entire run if this job fails
- run: gh run cancel ${{ github.run_id }}
Expand Down
7 changes: 7 additions & 0 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions crates/c-api/include/wasmtime/func.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,14 @@ WASM_API_EXTERN wasmtime_context_t* wasmtime_caller_context(wasmtime_caller_t* c
*/
WASM_API_EXTERN void wasmtime_func_from_raw(
wasmtime_context_t* context,
size_t raw,
void *raw,
wasmtime_func_t *ret);

/**
* \brief Converts a `func` which belongs to `context` into a `usize`
* parameter that is suitable for insertion into a #wasmtime_val_raw_t.
*/
WASM_API_EXTERN size_t wasmtime_func_to_raw(
WASM_API_EXTERN void *wasmtime_func_to_raw(
wasmtime_context_t* context,
const wasmtime_func_t *func);

Expand Down
8 changes: 4 additions & 4 deletions crates/c-api/include/wasmtime/val.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ WASM_API_EXTERN void wasmtime_externref_delete(wasmtime_externref_t *ref);
* Note that the returned #wasmtime_externref_t is an owned value that must be
* deleted via #wasmtime_externref_delete by the caller if it is non-null.
*/
WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_from_raw(wasmtime_context_t *context, size_t raw);
WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_from_raw(wasmtime_context_t *context, void *raw);

/**
* \brief Converts a #wasmtime_externref_t to a raw value suitable for storing
Expand All @@ -82,7 +82,7 @@ WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_from_raw(wasmtime_conte
* context of the store. Do not perform a GC between calling this function and
* passing it to WebAssembly.
*/
WASM_API_EXTERN size_t wasmtime_externref_to_raw(
WASM_API_EXTERN void *wasmtime_externref_to_raw(
wasmtime_context_t *context,
const wasmtime_externref_t *ref);

Expand Down Expand Up @@ -180,15 +180,15 @@ typedef union wasmtime_val_raw {
/// passed to `wasmtime_func_from_raw` to determine the `wasmtime_func_t`.
///
/// Note that this field is always stored in a little-endian format.
size_t funcref;
void *funcref;
/// Field for when this val is a WebAssembly `externref` value.
///
/// If this is set to 0 then it's a null externref, otherwise this must be
/// passed to `wasmtime_externref_from_raw` to determine the
/// `wasmtime_externref_t`.
///
/// Note that this field is always stored in a little-endian format.
size_t externref;
void *externref;
} wasmtime_val_raw_t;

/**
Expand Down
7 changes: 5 additions & 2 deletions crates/c-api/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,16 @@ pub unsafe extern "C" fn wasmtime_caller_export_get(
#[no_mangle]
pub unsafe extern "C" fn wasmtime_func_from_raw(
store: CStoreContextMut<'_>,
raw: usize,
raw: *mut c_void,
func: &mut Func,
) {
*func = Func::from_raw(store, raw).unwrap();
}

#[no_mangle]
pub unsafe extern "C" fn wasmtime_func_to_raw(store: CStoreContextMut<'_>, func: &Func) -> usize {
pub unsafe extern "C" fn wasmtime_func_to_raw(
store: CStoreContextMut<'_>,
func: &Func,
) -> *mut c_void {
func.to_raw(store)
}
6 changes: 3 additions & 3 deletions crates/c-api/src/val.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,17 +296,17 @@ pub extern "C" fn wasmtime_externref_delete(_val: Option<ExternRef>) {}
pub unsafe extern "C" fn wasmtime_externref_to_raw(
cx: CStoreContextMut<'_>,
val: Option<ManuallyDrop<ExternRef>>,
) -> usize {
) -> *mut c_void {
match val {
Some(ptr) => ptr.to_raw(cx),
None => 0,
None => ptr::null_mut(),
}
}

#[no_mangle]
pub unsafe extern "C" fn wasmtime_externref_from_raw(
_cx: CStoreContextMut<'_>,
val: usize,
val: *mut c_void,
) -> Option<ExternRef> {
ExternRef::from_raw(val)
}
1 change: 1 addition & 0 deletions crates/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ anyhow = { workspace = true }
memfd = "0.6.2"
paste = "1.0.3"
encoding_rs = { version = "0.8.31", optional = true }
sptr = "0.3.2"

[target.'cfg(target_os = "macos")'.dependencies]
mach = "0.3.2"
Expand Down
61 changes: 19 additions & 42 deletions crates/runtime/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
//! cranelift-compiled adapters, will use this `VMComponentContext` as well.

use crate::{
Store, VMArrayCallFunction, VMFuncRef, VMGlobalDefinition, VMMemoryDefinition,
SendSyncPtr, Store, VMArrayCallFunction, VMFuncRef, VMGlobalDefinition, VMMemoryDefinition,
VMNativeCallFunction, VMOpaqueContext, VMSharedSignatureIndex, VMWasmCallFunction, ValRaw,
};
use memoffset::offset_of;
use sptr::Strict;
use std::alloc::{self, Layout};
use std::marker;
use std::mem;
Expand Down Expand Up @@ -40,37 +41,15 @@ pub struct ComponentInstance {
/// Size and offset information for the trailing `VMComponentContext`.
offsets: VMComponentOffsets<HostPtr>,

/// A self-referential pointer to the `ComponentInstance` itself.
///
/// The reason for this field's existence is a bit subtle because if you
/// have a pointer to `ComponentInstance` then why have this field which has
/// the same information? The subtlety here is due to the fact that this is
/// here to handle provenance and aliasing issues with optimizations in LLVM
/// and Rustc. Put another way, this is here to make MIRI happy. That's an
/// oversimplification, though, since this is actually required for
/// correctness to communicate the actual intent to LLVM's optimizations and
/// such.
///
/// This field is chiefly used during `ComponentInstance::vmctx`. If you
/// think of pointers as a pair of address and provenance, this type stores
/// the provenance of the entire allocation. That's technically all we need
/// here, just the provenance, but that can only be done with storage of a
/// pointer hence the storage here.
///
/// Finally note that there's a small wrapper around this to add `Send` and
/// `Sync` bounds, but serves no other purpose.
self_reference: RawComponentInstance,
/// For more information about this see the documentation on
/// `Instance::vmctx_self_reference`.
vmctx_self_reference: SendSyncPtr<VMComponentContext>,

/// A zero-sized field which represents the end of the struct for the actual
/// `VMComponentContext` to be allocated behind.
vmctx: VMComponentContext,
}

struct RawComponentInstance(NonNull<ComponentInstance>);

unsafe impl Send for RawComponentInstance {}
unsafe impl Sync for RawComponentInstance {}

/// Type signature for host-defined trampolines that are called from
/// WebAssembly.
///
Expand Down Expand Up @@ -174,7 +153,15 @@ impl ComponentInstance {
ptr.as_ptr(),
ComponentInstance {
offsets,
self_reference: RawComponentInstance(ptr),
vmctx_self_reference: SendSyncPtr::new(
NonNull::new(
ptr.as_ptr()
.cast::<u8>()
.add(mem::size_of::<ComponentInstance>())
.cast(),
)
.unwrap(),
),
vmctx: VMComponentContext {
_marker: marker::PhantomPinned,
},
Expand All @@ -185,13 +172,8 @@ impl ComponentInstance {
}

fn vmctx(&self) -> *mut VMComponentContext {
let self_ref = self.self_reference.0.as_ptr();
unsafe {
self_ref
.cast::<u8>()
.add(mem::size_of::<ComponentInstance>())
.cast()
}
let addr = std::ptr::addr_of!(self.vmctx);
Strict::with_addr(self.vmctx_self_reference.as_ptr(), Strict::addr(addr))
}

unsafe fn vmctx_plus_offset<T>(&self, offset: u32) -> *const T {
Expand Down Expand Up @@ -520,15 +502,9 @@ impl VMComponentContext {
/// This type can be dereferenced to `ComponentInstance` to access the
/// underlying methods.
pub struct OwnedComponentInstance {
ptr: ptr::NonNull<ComponentInstance>,
ptr: SendSyncPtr<ComponentInstance>,
}

// Using `NonNull` turns off auto-derivation of these traits but the owned usage
// here enables these trait impls so long as `ComponentInstance` itself
// implements these traits.
unsafe impl Send for OwnedComponentInstance where ComponentInstance: Send {}
unsafe impl Sync for OwnedComponentInstance where ComponentInstance: Sync {}

impl OwnedComponentInstance {
/// Allocates a new `ComponentInstance + VMComponentContext` pair on the
/// heap with `malloc` and configures it for the `component` specified.
Expand All @@ -545,10 +521,11 @@ impl OwnedComponentInstance {
// zeroed allocation is done here to try to contain
// use-before-initialized issues.
let ptr = alloc::alloc_zeroed(layout) as *mut ComponentInstance;
let ptr = ptr::NonNull::new(ptr).unwrap();
let ptr = NonNull::new(ptr).unwrap();

ComponentInstance::new_at(ptr, layout.size(), offsets, store);

let ptr = SendSyncPtr::new(ptr);
OwnedComponentInstance { ptr }
}
}
Expand Down
Loading