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

Change the return type of SharedMemory::data #5240

Merged
merged 1 commit into from
Nov 10, 2022
Merged
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
7 changes: 3 additions & 4 deletions crates/fuzzing/src/oracles/diff_wasmtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,11 @@ impl DiffInstance for WasmtimeInstance {

fn get_memory(&mut self, name: &str, shared: bool) -> Option<Vec<u8>> {
Some(if shared {
let data = self
let memory = self
.instance
.get_shared_memory(&mut self.store, name)
.unwrap()
.data();
unsafe { (*data).to_vec() }
.unwrap();
memory.data().iter().map(|i| unsafe { *i.get() }).collect()
} else {
self.instance
.get_memory(&mut self.store, name)
Expand Down
31 changes: 21 additions & 10 deletions crates/wasmtime/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::store::{StoreData, StoreOpaque, Stored};
use crate::trampoline::generate_memory_export;
use crate::{AsContext, AsContextMut, Engine, MemoryType, StoreContext, StoreContextMut};
use anyhow::{bail, Result};
use std::cell::UnsafeCell;
use std::convert::TryFrom;
use std::slice;
use wasmtime_environ::MemoryPlan;
Expand Down Expand Up @@ -699,6 +700,7 @@ pub unsafe trait MemoryCreator: Send + Sync {
/// ```
#[derive(Clone)]
pub struct SharedMemory(wasmtime_runtime::SharedMemory, Engine);

impl SharedMemory {
/// Construct a [`SharedMemory`] by providing both the `minimum` and
/// `maximum` number of 64K-sized pages. This call allocates the necessary
Expand Down Expand Up @@ -737,19 +739,28 @@ impl SharedMemory {

/// Return access to the available portion of the shared memory.
///
/// Because the memory is shared, it is possible that this memory is being
/// modified in other threads--in other words, the data can change at any
/// time. Users of this function must manage synchronization and locking to
/// this region of memory themselves.
/// The slice returned represents the region of accessible memory at the
/// time that this function was called. The contents of the returned slice
/// will reflect concurrent modifications happening on other threads.
///
/// # Safety
///
/// The returned slice is valid for the entire duration of the lifetime of
/// this instance of [`SharedMemory`]. The base pointer of a shared memory
/// does not change. This [`SharedMemory`] may grow further after this
/// function has been called, but the slice returned will not grow.
///
/// Not only can the data change, but the length of this region can change
/// as well. Other threads can call `memory.grow` operations that will
/// extend the region length but--importantly--this will not be reflected in
/// the size of region returned by this function.
pub fn data(&self) -> *mut [u8] {
/// Concurrent modifications may be happening to the data returned on other
/// threads. The `UnsafeCell<u8>` represents that safe access to the
/// contents of the slice is not possible through normal loads and stores.
///
/// The memory returned must be accessed safely through the `Atomic*` types
/// in the [`std::sync::atomic`] module. Casting to those types must
/// currently be done unsafely.
pub fn data(&self) -> &[UnsafeCell<u8>] {
unsafe {
let definition = &*self.0.vmmemory_ptr();
slice::from_raw_parts_mut(definition.base, definition.current_length())
slice::from_raw_parts_mut(definition.base.cast(), definition.current_length())
}
}

Expand Down
13 changes: 10 additions & 3 deletions tests/all/threads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,22 @@ fn test_sharing_of_shared_memory() -> Result<()> {
let shared_memory = SharedMemory::new(&engine, MemoryType::shared(1, 5))?;
let instance1 = Instance::new(&mut store, &module, &[shared_memory.clone().into()])?;
let instance2 = Instance::new(&mut store, &module, &[shared_memory.clone().into()])?;
let data = shared_memory.data();

// Modify the memory in one place.
unsafe {
(*(shared_memory.data()))[0] = 42;
*data[0].get() = 42;
}

// Verify that the memory is the same in all shared locations.
let shared_memory_first_word =
i32::from_le_bytes(unsafe { (*shared_memory.data())[0..4].try_into()? });
let shared_memory_first_word = i32::from_le_bytes(unsafe {
[
*data[0].get(),
*data[1].get(),
*data[2].get(),
*data[3].get(),
]
});
let instance1_first_word = instance1
.get_typed_func::<(), i32, _>(&mut store, "first_word")?
.call(&mut store, ())?;
Expand Down