From ec456e9e509d72ffb285656d8657a7d34b9448e4 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 27 Jan 2020 18:20:47 -0800 Subject: [PATCH] new memory model. not quite complete --- crates/generate/src/funcs.rs | 6 +- crates/generate/src/lib.rs | 2 +- crates/generate/src/types.rs | 32 ++-- crates/memory/src/error.rs | 6 +- crates/memory/src/guest_type.rs | 62 +++---- crates/memory/src/lib.rs | 4 +- crates/memory/src/memory.rs | 317 ++++++++++++++++++++++---------- src/lib.rs | 37 ++-- 8 files changed, 287 insertions(+), 179 deletions(-) diff --git a/crates/generate/src/funcs.rs b/crates/generate/src/funcs.rs index e886308f86..6f4eebe592 100644 --- a/crates/generate/src/funcs.rs +++ b/crates/generate/src/funcs.rs @@ -74,7 +74,8 @@ pub fn define_func(names: &Names, func: &witx::InterfaceFunc) -> TokenStream { let marshal_args = func .params .iter() - .map(|p| marshal_arg(names, p, error_handling.clone())); + //.map(|p| marshal_arg(names, p, error_handling.clone())); + .map(|_p| quote!(unimplemented!(); )); // FIXME let trait_args = func .params .iter() @@ -98,7 +99,8 @@ pub fn define_func(names: &Names, func: &witx::InterfaceFunc) -> TokenStream { .results .iter() .skip(1) - .map(|result| marshal_result(names, result, error_handling.clone())); + //.map(|result| marshal_result(names, result, error_handling.clone())); + .map(|_result| (quote!(unimplemented!();), quote!(unimplemented!();))); // FIXME let marshal_rets_pre = marshal_rets.clone().map(|(pre, _post)| pre); let marshal_rets_post = marshal_rets.map(|(_pre, post)| post); diff --git a/crates/generate/src/lib.rs b/crates/generate/src/lib.rs index 49d9289d58..15d07cba0c 100644 --- a/crates/generate/src/lib.rs +++ b/crates/generate/src/lib.rs @@ -34,7 +34,7 @@ pub fn from_witx(args: TokenStream) -> TokenStream { mod #modname { use super::WasiCtx; use super::types::*; - #(#fs)* + // #(#fs)* #modtrait } diff --git a/crates/generate/src/types.rs b/crates/generate/src/types.rs index d385ae3715..4ee337bbc0 100644 --- a/crates/generate/src/types.rs +++ b/crates/generate/src/types.rs @@ -89,24 +89,30 @@ fn define_enum(names: &Names, name: &witx::Id, e: &witx::EnumDatatype) -> TokenS fn size() -> u32 { ::std::mem::size_of::<#repr>() as u32 } - fn name() -> &'static str { - stringify!(#ident) + fn align() -> u32 { + ::std::mem::align_of::<#repr>() as u32 + } + fn name() -> String { + stringify!(#ident).to_owned() + } + fn validate<'a>(location: &::memory::GuestPtr<'a, #ident>) -> Result<(), ::memory::GuestError> { + use ::std::convert::TryFrom; + let raw: #repr = unsafe { (location.as_raw() as *const #repr).read() }; + let _ = #ident::try_from(raw)?; + Ok(()) } } - impl ::memory::GuestTypeCopy for #ident { - fn read_val<'a, P: ::memory::GuestPtrRead<'a, #ident>>(src: &P) -> Result<#ident, ::memory::GuestError> { - use ::std::convert::TryInto; - let val = unsafe { ::std::ptr::read_unaligned(src.ptr() as *const #repr) }; - val.try_into() - } - fn write_val(val: #ident, dest: &::memory::GuestPtrMut<#ident>) { - let val: #repr = val.into(); - unsafe { - ::std::ptr::write_unaligned(dest.ptr_mut() as *mut #repr, val) - }; + impl ::memory::GuestTypeCopy for #ident {} + impl ::memory::GuestTypeClone for #ident { + fn from_guest(location: &::memory::GuestPtr<#ident>) -> Result<#ident, ::memory::GuestError> { + use ::std::convert::TryFrom; + let raw: #repr = unsafe { (location.as_raw() as *const #repr).read() }; + let val = #ident::try_from(raw)?; + Ok(val) } } + } } diff --git a/crates/memory/src/error.rs b/crates/memory/src/error.rs index 7c5632ca05..b1ed3b49b0 100644 --- a/crates/memory/src/error.rs +++ b/crates/memory/src/error.rs @@ -5,8 +5,10 @@ use thiserror::Error; pub enum GuestError { #[error("Invalid enum value {0}")] InvalidEnumValue(&'static str), - #[error("Out of bounds: {0:?}")] + #[error("Pointer out of bounds: {0:?}")] PtrOutOfBounds(Region), - #[error("Borrowed: {0:?}")] + #[error("Pointer not aligned to {1}: {0:?}")] + PtrNotAligned(Region, u32), + #[error("Pointer already borrowed: {0:?}")] PtrBorrowed(Region), } diff --git a/crates/memory/src/guest_type.rs b/crates/memory/src/guest_type.rs index 6233888540..2b4b3f387a 100644 --- a/crates/memory/src/guest_type.rs +++ b/crates/memory/src/guest_type.rs @@ -1,56 +1,39 @@ -use crate::{GuestError, GuestPtrMut, GuestPtrRead}; +use crate::{GuestError, GuestPtr}; pub trait GuestType: Sized { + // These are morally the same as Rust ::std::mem::size_of / align_of, but they return + // a u32 because the wasm memory space is 32 bits. They have a different names so they + // don't collide with the std::mem methods. fn size() -> u32; - fn name() -> &'static str; -} - -pub trait GuestTypeCopy: GuestType + Copy { - fn read_val<'a, P: GuestPtrRead<'a, Self>>(src: &P) -> Result; - fn write_val(val: Self, dest: &GuestPtrMut); + fn align() -> u32; + fn name() -> String; + fn validate<'a>(location: &GuestPtr<'a, Self>) -> Result<(), GuestError>; } +pub trait GuestTypeCopy: GuestType + Copy {} pub trait GuestTypeClone: GuestType + Clone { - fn read_ref<'a, P: GuestPtrRead<'a, Self>>(src: &P, dest: &mut Self) -> Result<(), GuestError>; - fn write_ref(val: &Self, dest: &GuestPtrMut); + fn from_guest<'a>(location: &GuestPtr<'a, Self>) -> Result; +} +pub trait GuestTypeRef<'a>: GuestType { + type Ref; + fn from_guest(location: &GuestPtr<'a, Self>) -> Result; } -impl GuestTypeClone for T -where - T: GuestTypeCopy, -{ - fn read_ref<'a, P: GuestPtrRead<'a, Self>>(src: &P, dest: &mut T) -> Result<(), GuestError> { - let val = GuestTypeCopy::read_val(src)?; - *dest = val; - Ok(()) - } - fn write_ref(val: &T, dest: &GuestPtrMut) { - GuestTypeCopy::write_val(*val, dest) - } -} - -macro_rules! builtin_copy { +macro_rules! builtin_type { ( $( $t:ident ), * ) => { $( impl GuestType for $t { fn size() -> u32 { ::std::mem::size_of::<$t>() as u32 } - fn name() -> &'static str { - ::std::stringify!($t) + fn align() -> u32 { + ::std::mem::align_of::<$t>() as u32 } - } - - impl GuestTypeCopy for $t { - fn read_val<'a, P: GuestPtrRead<'a, $t>>(src: &P) -> Result<$t, GuestError> { - Ok(unsafe { - ::std::ptr::read_unaligned(src.ptr() as *const $t) - }) + fn name() -> String { + ::std::stringify!($t).to_owned() } - fn write_val(val: $t, dest: &GuestPtrMut<$t>) { - unsafe { - ::std::ptr::write_unaligned(dest.ptr_mut() as *mut $t, val) - } + fn validate(_ptr: &GuestPtr<$t>) -> Result<(), GuestError> { + Ok(()) } } )* @@ -58,7 +41,10 @@ macro_rules! builtin_copy { } // These definitions correspond to all the witx BuiltinType variants that are Copy: -builtin_copy!(u8, i8, u16, i16, u32, i32, u64, i64, f32, f64, usize, char); +builtin_type!(u8, i8, u16, i16, u32, i32, u64, i64, f32, f64, usize); + +// FIXME implement GuestType for char. needs to validate that its a code point. what is the sizeof a char? +// FIXME implement GuestType for String. how does validate work for array types? pub trait GuestErrorType { type Context; diff --git a/crates/memory/src/lib.rs b/crates/memory/src/lib.rs index f8705d6c80..5aceb413b3 100644 --- a/crates/memory/src/lib.rs +++ b/crates/memory/src/lib.rs @@ -5,6 +5,6 @@ mod memory; mod region; pub use error::GuestError; -pub use guest_type::{GuestErrorType, GuestType, GuestTypeClone, GuestTypeCopy}; -pub use memory::{GuestMemory, GuestPtr, GuestPtrMut, GuestPtrRead}; +pub use guest_type::{GuestErrorType, GuestType, GuestTypeClone, GuestTypeCopy, GuestTypeRef}; +pub use memory::{GuestMemory, GuestPtr, GuestPtrMut, GuestRef, GuestRefMut}; pub use region::Region; diff --git a/crates/memory/src/memory.rs b/crates/memory/src/memory.rs index 3af1a0413d..3b6897b8cd 100644 --- a/crates/memory/src/memory.rs +++ b/crates/memory/src/memory.rs @@ -3,7 +3,7 @@ use std::marker::PhantomData; use std::rc::Rc; use crate::borrow::{BorrowHandle, GuestBorrows}; -use crate::{GuestError, GuestType, Region}; +use crate::{GuestError, GuestType, GuestTypeCopy, GuestTypeRef, Region}; pub struct GuestMemory<'a> { ptr: *mut u8, @@ -14,6 +14,7 @@ pub struct GuestMemory<'a> { impl<'a> GuestMemory<'a> { pub fn new(ptr: *mut u8, len: u32) -> GuestMemory<'a> { + assert_eq!(ptr as usize % 4096, 0, "GuestMemory must be page-aligned"); GuestMemory { ptr, len, @@ -36,145 +37,259 @@ impl<'a> GuestMemory<'a> { if !self.contains(region) { Err(GuestError::PtrOutOfBounds(region))?; } - let mut borrows = self.borrows.borrow_mut(); - if let Some(handle) = borrows.borrow_immut(region) { - Ok(GuestPtr { - mem: &self, - region, - handle, - type_: PhantomData, - }) - } else { - Err(GuestError::PtrBorrowed(region)) + if at % T::align() != 0 { + Err(GuestError::PtrNotAligned(region, T::align()))?; } + Ok(GuestPtr { + mem: &self, + region, + type_: PhantomData, + }) } - pub fn ptr_mut(&'a self, at: u32) -> Result, GuestError> { - let region = Region { - start: at, - len: T::size(), - }; - if !self.contains(region) { - Err(GuestError::PtrOutOfBounds(region))?; - } - let mut borrows = self.borrows.borrow_mut(); - if let Some(handle) = borrows.borrow_mut(region) { - Ok(GuestPtrMut { - mem: &self, - region, - handle, - type_: PhantomData, - }) - } else { - Err(GuestError::PtrBorrowed(region)) - } - } -} - -/// These methods should not be used by the end user - just by implementations of the -/// GuestValueClone and GuestValueCopy traits! -pub trait GuestPtrRead<'a, T> { - fn mem(&self) -> &'a GuestMemory<'a>; - fn region(&self) -> &Region; - fn ptr(&self) -> *const u8 { - (self.mem().ptr as usize + self.region().start as usize) as *const u8 + let ptr = self.ptr(at)?; + Ok(GuestPtrMut { + mem: ptr.mem, + region: ptr.region, + type_: ptr.type_, + }) } } +#[derive(Clone)] pub struct GuestPtr<'a, T> { + mem: &'a GuestMemory<'a>, + region: Region, + type_: PhantomData, +} + +impl<'a, T: GuestType> GuestPtr<'a, T> { + pub fn as_raw(&self) -> *const u8 { + (self.mem.ptr as usize + self.region.start as usize) as *const u8 + } + pub fn as_ref(&self) -> Result, GuestError> { + T::validate(&self)?; + let handle = { + let mut borrows = self.mem.borrows.borrow_mut(); + borrows + .borrow_immut(self.region) + .ok_or_else(|| GuestError::PtrBorrowed(self.region))? + }; + Ok(GuestRef { + mem: self.mem, + region: self.region, + handle, + type_: self.type_, + }) + } +} + +impl<'a, T> GuestType for GuestPtr<'a, T> +where + T: GuestType, +{ + fn size() -> u32 { + 4 + } + fn align() -> u32 { + 4 + } + fn name() -> String { + format!("GuestPtr<{}>", T::name()) + } + fn validate<'b>(location: &GuestPtr<'b, GuestPtr<'b, T>>) -> Result<(), GuestError> { + // location is guaranteed to be in GuestMemory and aligned to 4 + let raw_ptr: u32 = unsafe { *(location.as_raw() as *const u32) }; + // GuestMemory can validate that the raw pointer contents are legal for T: + let _guest_ptr: GuestPtr = location.mem.ptr(raw_ptr)?; + Ok(()) + } +} + +impl<'a, T> GuestTypeRef<'a> for GuestPtr<'a, T> +where + T: GuestType, +{ + type Ref = GuestRef<'a, T>; + fn from_guest(location: &GuestPtr<'a, Self>) -> Result { + // location is guaranteed to be in GuestMemory and aligned to 4 + let raw_ptr: u32 = unsafe { *(location.as_raw() as *const u32) }; + // GuestMemory can validate that the raw pointer contents are legal for T: + let guest_ptr: GuestPtr<'a, T> = location.mem.ptr(raw_ptr)?; + Ok(guest_ptr.as_ref()?) + } +} + +#[derive(Clone)] +pub struct GuestPtrMut<'a, T> { + mem: &'a GuestMemory<'a>, + region: Region, + type_: PhantomData, +} + +impl<'a, T: GuestType> GuestPtrMut<'a, T> { + pub fn as_immut(&self) -> GuestPtr<'a, T> { + GuestPtr { + mem: self.mem, + region: self.region, + type_: self.type_, + } + } + + pub fn as_raw(&self) -> *const u8 { + self.as_immut().as_raw() + } + + pub fn as_ref(&self) -> Result, GuestError> { + self.as_immut().as_ref() + } + + pub fn as_ref_mut(&self) -> Result, GuestError> { + T::validate(&self.as_immut())?; + let handle = { + let mut borrows = self.mem.borrows.borrow_mut(); + borrows + .borrow_mut(self.region) + .ok_or_else(|| GuestError::PtrBorrowed(self.region))? + }; + Ok(GuestRefMut { + mem: self.mem, + region: self.region, + handle, + type_: self.type_, + }) + } +} + +impl<'a, T> GuestType for GuestPtrMut<'a, T> +where + T: GuestType, +{ + fn size() -> u32 { + 4 + } + fn align() -> u32 { + 4 + } + fn name() -> String { + format!("GuestPtrMut<{}>", T::name()) + } + fn validate<'b>(location: &GuestPtr<'b, GuestPtrMut<'b, T>>) -> Result<(), GuestError> { + // location is guaranteed to be in GuestMemory and aligned to 4 + let raw_ptr: u32 = unsafe { *(location.as_raw() as *const u32) }; + // GuestMemory can validate that the raw pointer contents are legal for T: + let _guest_ptr: GuestPtr = location.mem.ptr(raw_ptr)?; + Ok(()) + } +} + +impl<'a, T> GuestTypeRef<'a> for GuestPtrMut<'a, T> +where + T: GuestType, +{ + type Ref = GuestRefMut<'a, T>; + fn from_guest(location: &GuestPtr<'a, Self>) -> Result { + // location is guaranteed to be in GuestMemory and aligned to 4 + let raw_ptr: u32 = unsafe { *(location.as_raw() as *const u32) }; + // GuestMemory can validate that the raw pointer contents are legal for T: + let guest_ptr_mut: GuestPtrMut<'a, T> = location.mem.ptr_mut(raw_ptr)?; + Ok(guest_ptr_mut.as_ref_mut()?) + } +} + +pub struct GuestRef<'a, T> { mem: &'a GuestMemory<'a>, region: Region, handle: BorrowHandle, type_: PhantomData, } -impl<'a, T> GuestPtrRead<'a, T> for GuestPtr<'a, T> { - fn mem(&self) -> &'a GuestMemory<'a> { - self.mem - } - fn region(&self) -> &Region { - &self.region +impl<'a, T> ::std::ops::Deref for GuestRef<'a, T> +where + T: GuestTypeCopy, +{ + type Target = T; + fn deref(&self) -> &T { + unsafe { + ((self.mem.ptr as usize + self.region.start as usize) as *const T) + .as_ref() + .expect("GuestRef implies non-null") + } } } -impl<'a, T> GuestType for GuestPtr<'a, T> { - fn size() -> u32 { - 4 - } - fn name() -> &'static str { - "GuestPtr<...>" +impl<'a, T> GuestRef<'a, T> +where + T: GuestTypeRef<'a>, +{ + pub fn from_guest(&self) -> Result { + let ptr = GuestPtr { + mem: self.mem, + region: self.region, + type_: self.type_, + }; + GuestTypeRef::from_guest(&ptr) } } -impl<'a, T: GuestType> GuestPtr<'a, T> { - pub fn read_ptr>(src: &P) -> Result { - let raw_ptr = unsafe { ::std::ptr::read_unaligned(src.ptr() as *const u32) }; - src.mem().ptr(raw_ptr) - } - pub fn write_ptr(ptr: &Self, dest: &GuestPtrMut) { - unsafe { ::std::ptr::write_unaligned(dest.ptr_mut() as *mut u32, ptr.region.start) } - } -} - -impl<'a, T> Drop for GuestPtr<'a, T> { +impl<'a, T> Drop for GuestRef<'a, T> { fn drop(&mut self) { let mut borrows = self.mem.borrows.borrow_mut(); borrows.unborrow_immut(self.handle); } } -pub struct GuestPtrMut<'a, T> { +pub struct GuestRefMut<'a, T> { mem: &'a GuestMemory<'a>, region: Region, handle: BorrowHandle, type_: PhantomData, } -impl<'a, T> GuestPtrRead<'a, T> for GuestPtrMut<'a, T> { - fn mem(&self) -> &'a GuestMemory<'a> { - self.mem - } - fn region(&self) -> &Region { - &self.region +impl<'a, T> ::std::ops::Deref for GuestRefMut<'a, T> +where + T: GuestTypeCopy, +{ + type Target = T; + fn deref(&self) -> &T { + unsafe { + ((self.mem.ptr as usize + self.region.start as usize) as *const T) + .as_ref() + .expect("GuestRef implies non-null") + } } } -impl<'a, T> GuestPtrMut<'a, T> { - pub fn ptr_mut(&self) -> *mut u8 { - (self.mem.ptr as usize + self.region.start as usize) as *mut u8 +impl<'a, T> ::std::ops::DerefMut for GuestRefMut<'a, T> +where + T: GuestTypeCopy, +{ + fn deref_mut(&mut self) -> &mut T { + unsafe { + ((self.mem.ptr as usize + self.region.start as usize) as *mut T) + .as_mut() + .expect("GuestRef implies non-null") + } } } -impl<'a, T> Drop for GuestPtrMut<'a, T> { +impl<'a, T> GuestRefMut<'a, T> +where + T: GuestTypeRef<'a>, +{ + pub fn from_guest(&self) -> Result { + let ptr = GuestPtr { + mem: self.mem, + region: self.region, + type_: self.type_, + }; + GuestTypeRef::from_guest(&ptr) + } +} + +impl<'a, T> Drop for GuestRefMut<'a, T> { fn drop(&mut self) { let mut borrows = self.mem.borrows.borrow_mut(); borrows.unborrow_mut(self.handle); } } - -impl<'a, T> GuestType for GuestPtrMut<'a, T> { - fn size() -> u32 { - 4 - } - fn name() -> &'static str { - "GuestPtrMut<...>" - } -} - -impl<'a, T: GuestType> GuestPtrMut<'a, T> { - pub fn read_ptr>(src: &P) -> Result { - let raw_ptr = unsafe { ::std::ptr::read_unaligned(src.ptr() as *const u32) }; - src.mem().ptr_mut(raw_ptr) - } - pub fn write_ptr(ptr: &Self, dest: &GuestPtrMut) { - unsafe { ::std::ptr::write_unaligned(dest.ptr_mut() as *mut u32, ptr.region.start) } - } - - pub fn as_immut(self) -> GuestPtr<'a, T> { - let mem = self.mem; - let start = self.region.start; - drop(self); - mem.ptr(start) - .expect("can borrow just-dropped mutable region as immut") - } -} diff --git a/src/lib.rs b/src/lib.rs index cf36236e83..6fa441546d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,43 +18,40 @@ pub mod test { a_lamer_excuse_by_reference: ::memory::GuestPtr, two_layers_of_excuses: ::memory::GuestPtrMut<::memory::GuestPtr>, ) -> Result<(), types::Errno> { - use memory::GuestTypeCopy; - // Read enum value from mutable: - let a_better_excuse = - types::Excuse::read_val(&a_better_excuse_by_reference).map_err(|e| { + let mut a_better_excuse_ref: ::memory::GuestRefMut = + a_better_excuse_by_reference.as_ref_mut().map_err(|e| { eprintln!("a_better_excuse_by_reference error: {}", e); types::Errno::InvalidArg })?; + let a_better_excuse: types::Excuse = *a_better_excuse_ref; // Read enum value from immutable ptr: - let a_lamer_excuse = - types::Excuse::read_val(&a_lamer_excuse_by_reference).map_err(|e| { - eprintln!("a_lamer_excuse_by_reference error: {}", e); - types::Errno::InvalidArg - })?; + let a_lamer_excuse = *a_lamer_excuse_by_reference.as_ref().map_err(|e| { + eprintln!("a_lamer_excuse_by_reference error: {}", e); + types::Errno::InvalidArg + })?; // Write enum to mutable ptr: - types::Excuse::write_val(a_lamer_excuse, &a_better_excuse_by_reference); + *a_better_excuse_ref = a_lamer_excuse; // Read ptr value from mutable ptr: - let one_layer_down = - ::memory::GuestPtr::read_ptr(&two_layers_of_excuses).map_err(|e| { + let mut one_layer_down: ::memory::GuestRefMut<::memory::GuestPtr> = + two_layers_of_excuses.as_ref_mut().map_err(|e| { eprintln!("one_layer_down error: {}", e); types::Errno::InvalidArg })?; // Read enum value from that ptr: - let two_layers_down = types::Excuse::read_val(&one_layer_down).map_err(|e| { - eprintln!("two_layers_down error: {}", e); - types::Errno::InvalidArg - })?; + let two_layers_down: ::memory::GuestRef = + one_layer_down.from_guest().map_err(|e| { + eprintln!("two_layers_down error: {}", e); + types::Errno::InvalidArg + })?; // Write ptr value to mutable ptr: - ::memory::GuestPtr::write_ptr( - &a_better_excuse_by_reference.as_immut(), - &two_layers_of_excuses, - ); + // FIXME this is still impossible... + // two_layers_of_excuses.write_guest(&a_better_excuse_by_reference) println!( "BAZ: excuse: {:?}, better excuse: {:?}, lamer excuse: {:?}, two layers down: {:?}",