Skip to content

Commit

Permalink
Merge with upstream (#107)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhil authored Feb 16, 2024
2 parents 75fe454 + 4254dbe commit 4df0bc5
Show file tree
Hide file tree
Showing 16 changed files with 343 additions and 605 deletions.
17 changes: 16 additions & 1 deletion cranelift/jit/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,11 @@ impl Module for JITModule {
} = data;

let size = init.size();
let ptr = if decl.writable {
let ptr = if size == 0 {
// Return a correctly aligned non-null pointer to avoid UB in write_bytes and
// copy_nonoverlapping.
usize::try_from(align.unwrap_or(WRITABLE_DATA_ALIGNMENT)).unwrap() as *mut u8
} else if decl.writable {
self.memory
.writable
.allocate(size, align.unwrap_or(WRITABLE_DATA_ALIGNMENT))
Expand All @@ -869,6 +873,17 @@ impl Module for JITModule {
})?
};

if ptr.is_null() {
// FIXME pass a Layout to allocate and only compute the layout once.
std::alloc::handle_alloc_error(
std::alloc::Layout::from_size_align(
size,
align.unwrap_or(READONLY_DATA_ALIGNMENT).try_into().unwrap(),
)
.unwrap(),
);
}

match *init {
Init::Uninitialized => {
panic!("data is not initialized yet");
Expand Down
24 changes: 24 additions & 0 deletions cranelift/jit/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,27 @@ fn libcall_function() {

module.finalize_definitions().unwrap();
}

// This used to cause UB. See https://github.com/bytecodealliance/wasmtime/issues/7918.
#[test]
fn empty_data_object() {
let mut flag_builder = settings::builder();
flag_builder.set("use_colocated_libcalls", "false").unwrap();
// FIXME set back to true once the x64 backend supports it.
flag_builder.set("is_pic", "false").unwrap();
let isa_builder = cranelift_native::builder().unwrap_or_else(|msg| {
panic!("host machine is not supported: {}", msg);
});
let isa = isa_builder
.finish(settings::Flags::new(flag_builder))
.unwrap();
let mut module = JITModule::new(JITBuilder::with_isa(isa, default_libcall_names()));

let data_id = module
.declare_data("empty", Linkage::Export, false, false)
.unwrap();

let mut data = DataDescription::new();
data.define(Box::new([]));
module.define_data(data_id, &data).unwrap();
}
13 changes: 1 addition & 12 deletions crates/continuations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,14 @@ pub type ContinuationFiber = Fiber<'static, (), u32, ()>;
/// This type is used to save (and subsequently restore) a subset of the data in
/// `VMRuntimeLimits`. See documentation of `StackChain` for the exact uses.
#[repr(C)]
#[derive(Debug, Clone)]
#[derive(Debug, Default, Clone)]
pub struct StackLimits {
pub stack_limit: usize,
pub last_wasm_exit_fp: usize,
pub last_wasm_exit_pc: usize,
pub last_wasm_entry_sp: usize,
}

impl Default for StackLimits {
fn default() -> Self {
Self {
stack_limit: 0,
last_wasm_exit_fp: 0,
last_wasm_exit_pc: 0,
last_wasm_entry_sp: 0,
}
}
}

impl StackLimits {
pub fn with_stack_limit(stack_limit: usize) -> Self {
Self {
Expand Down
4 changes: 4 additions & 0 deletions crates/runtime/src/mpk/disabled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ pub fn keys(_: usize) -> &'static [ProtectionKey] {
}
pub fn allow(_: ProtectionMask) {}

pub fn current_mask() -> ProtectionMask {
ProtectionMask
}

#[derive(Clone, Copy, Debug)]
pub enum ProtectionKey {}
impl ProtectionKey {
Expand Down
5 changes: 5 additions & 0 deletions crates/runtime/src/mpk/enabled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ pub fn allow(mask: ProtectionMask) {
log::trace!("PKRU change: {:#034b} => {:#034b}", previous, pkru::read());
}

/// Retrieve the current protection mask.
pub fn current_mask() -> ProtectionMask {
ProtectionMask(pkru::read())
}

/// An MPK protection key.
///
/// The expected usage is:
Expand Down
4 changes: 2 additions & 2 deletions crates/runtime/src/mpk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ cfg_if::cfg_if! {
mod enabled;
mod pkru;
mod sys;
pub use enabled::{allow, is_supported, keys, ProtectionKey, ProtectionMask};
pub use enabled::{allow, current_mask, is_supported, keys, ProtectionKey, ProtectionMask};
} else {
mod disabled;
pub use disabled::{allow, is_supported, keys, ProtectionKey, ProtectionMask};
pub use disabled::{allow, current_mask, is_supported, keys, ProtectionKey, ProtectionMask};
}
}

Expand Down
12 changes: 12 additions & 0 deletions crates/test-programs/src/bin/preview2_pollable_correct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use test_programs::wasi::cli::stdin;

fn main() {
let stdin = stdin::get_stdin();
let p1 = stdin.subscribe();
let p2 = stdin.subscribe();

// Should work:
// - Exactly the same pollable passed in multiple times.
// - Distinct pollables for the same backing resource (stdin in this case).
test_programs::wasi::io::poll::poll(&[&p1, &p2, &p1, &p2]);
}
4 changes: 4 additions & 0 deletions crates/test-programs/src/bin/preview2_pollable_traps.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {
// Polling an empty list should trap:
test_programs::wasi::io::poll::poll(&[]);
}
5 changes: 3 additions & 2 deletions crates/wasi-http/wit/deps/io/poll.wit
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ interface poll {
/// The result `list<u32>` contains one or more indices of handles in the
/// argument list that is ready for I/O.
///
/// If the list contains more elements than can be indexed with a `u32`
/// value, this function traps.
/// This function traps if either:
/// - the list is empty, or:
/// - the list contains more elements than can be indexed with a `u32` value.
///
/// A timeout can be implemented by adding a pollable from the
/// wasi-clocks API to the list.
Expand Down
6 changes: 5 additions & 1 deletion crates/wasi/src/preview2/poll.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::preview2::{bindings::io::poll, WasiView};
use anyhow::Result;
use anyhow::{anyhow, Result};
use std::any::Any;
use std::collections::HashMap;
use std::future::Future;
Expand Down Expand Up @@ -68,6 +68,10 @@ impl<T: WasiView> poll::Host for T {
async fn poll(&mut self, pollables: Vec<Resource<Pollable>>) -> Result<Vec<u32>> {
type ReadylistIndex = u32;

if pollables.is_empty() {
return Err(anyhow!("empty poll list"));
}

let table = self.table();

let mut table_futures: HashMap<u32, (MakeFuture, Vec<ReadylistIndex>)> = HashMap::new();
Expand Down
16 changes: 16 additions & 0 deletions crates/wasi/tests/all/async_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,22 @@ async fn preview2_stream_pollable_traps() {
)
}
#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn preview2_pollable_correct() {
run(PREVIEW2_POLLABLE_CORRECT_COMPONENT, false)
.await
.unwrap()
}
#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn preview2_pollable_traps() {
let e = run(PREVIEW2_POLLABLE_TRAPS_COMPONENT, false)
.await
.unwrap_err();
assert_eq!(
format!("{}", e.source().expect("trap source")),
"empty poll list"
)
}
#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn preview2_adapter_badfd() {
run(PREVIEW2_ADAPTER_BADFD_COMPONENT, false).await.unwrap()
}
12 changes: 12 additions & 0 deletions crates/wasi/tests/all/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,18 @@ fn preview2_stream_pollable_traps() {
)
}
#[test_log::test]
fn preview2_pollable_correct() {
run(PREVIEW2_POLLABLE_CORRECT_COMPONENT, false).unwrap()
}
#[test_log::test]
fn preview2_pollable_traps() {
let e = run(PREVIEW2_POLLABLE_TRAPS_COMPONENT, false).unwrap_err();
assert_eq!(
format!("{}", e.source().expect("trap source")),
"empty poll list"
)
}
#[test_log::test]
fn preview2_adapter_badfd() {
run(PREVIEW2_ADAPTER_BADFD_COMPONENT, false).unwrap()
}
5 changes: 3 additions & 2 deletions crates/wasi/wit/deps/io/poll.wit
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ interface poll {
/// The result `list<u32>` contains one or more indices of handles in the
/// argument list that is ready for I/O.
///
/// If the list contains more elements than can be indexed with a `u32`
/// value, this function traps.
/// This function traps if either:
/// - the list is empty, or:
/// - the list contains more elements than can be indexed with a `u32` value.
///
/// A timeout can be implemented by adding a pollable from the
/// wasi-clocks API to the list.
Expand Down
23 changes: 20 additions & 3 deletions crates/wasmtime/src/runtime/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,11 @@ use std::ptr;
use std::sync::atomic::AtomicU64;
use std::sync::Arc;
use std::task::{Context, Poll};
use wasmtime_runtime::mpk::{self, ProtectionKey, ProtectionMask};
use wasmtime_runtime::{
mpk::ProtectionKey, ExportGlobal, InstanceAllocationRequest, InstanceAllocator, InstanceHandle,
ModuleInfo, OnDemandInstanceAllocator, SignalHandler, StoreBox, StorePtr, VMContext,
VMExternRef, VMExternRefActivationsTable, VMFuncRef, VMRuntimeLimits, WasmFault,
ExportGlobal, InstanceAllocationRequest, InstanceAllocator, InstanceHandle, ModuleInfo,
OnDemandInstanceAllocator, SignalHandler, StoreBox, StorePtr, VMContext, VMExternRef,
VMExternRefActivationsTable, VMFuncRef, VMRuntimeLimits, WasmFault,
};

mod context;
Expand Down Expand Up @@ -1401,6 +1402,7 @@ impl StoreOpaque {
Some(AsyncCx {
current_suspend: self.async_state.current_suspend.get(),
current_poll_cx: poll_cx_box_ptr,
track_pkey_context_switch: mpk::is_supported() && self.pkey.is_some(),
})
}

Expand Down Expand Up @@ -1938,6 +1940,7 @@ impl<T> StoreContextMut<'_, T> {
pub struct AsyncCx {
current_suspend: *mut *const wasmtime_fiber::Suspend<Result<()>, (), Result<()>>,
current_poll_cx: *mut *mut Context<'static>,
track_pkey_context_switch: bool,
}

#[cfg(feature = "async")]
Expand Down Expand Up @@ -1998,7 +2001,21 @@ impl AsyncCx {
Poll::Pending => {}
}

// In order to prevent this fiber's MPK state from being munged by
// other fibers while it is suspended, we save and restore it once
// once execution resumes. Note that when MPK is not supported,
// these are noops.
let previous_mask = if self.track_pkey_context_switch {
let previous_mask = mpk::current_mask();
mpk::allow(ProtectionMask::all());
previous_mask
} else {
ProtectionMask::all()
};
(*suspend).suspend(())?;
if self.track_pkey_context_switch {
mpk::allow(previous_mask);
}
}
}
}
Expand Down
Loading

0 comments on commit 4df0bc5

Please sign in to comment.