Create a grow error and refactor grow impl to return result (#191)

This commit is contained in:
Mackenzie Clark
2019-02-22 22:18:59 -08:00
committed by GitHub
parent fa596d2d23
commit 82eef13f41
13 changed files with 250 additions and 112 deletions

View File

@ -1,3 +1,4 @@
use crate::sys::Memory;
use crate::types::{
FuncSig, GlobalDescriptor, MemoryDescriptor, MemoryIndex, TableDescriptor, TableIndex, Type,
};
@ -369,3 +370,105 @@ impl std::fmt::Display for Error {
}
impl std::error::Error for Error {}
#[derive(Debug)]
pub enum GrowError {
MemoryGrowError,
TableGrowError,
ExceededMaxPages(PageError),
ExceededMaxPagesForMemory(usize, usize),
CouldNotProtectMemory(MemoryProtectionError),
CouldNotCreateMemory(MemoryCreationError),
}
impl std::fmt::Display for GrowError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
GrowError::MemoryGrowError => write!(f, "Unable to grow memory"),
GrowError::TableGrowError => write!(f, "Unable to grow table"),
GrowError::ExceededMaxPages(e) => write!(f, "Grow Error: {}", e),
GrowError::ExceededMaxPagesForMemory(left, added) => write!(f, "Failed to add pages because would exceed maximum number of pages for the memory. Left: {}, Added: {}", left, added),
GrowError::CouldNotCreateMemory(e) => write!(f, "Grow Error: {}", e),
GrowError::CouldNotProtectMemory(e) => write!(f, "Grow Error: {}", e),
}
}
}
impl std::error::Error for GrowError {}
#[derive(Debug)]
pub enum PageError {
// left, right, added
ExceededMaxPages(usize, usize, usize),
}
impl std::fmt::Display for PageError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
PageError::ExceededMaxPages(left, right, added) => write!(f, "Failed to add pages because would exceed maximum number of pages. Left: {}, Right: {}, Pages added: {}", left, right, added),
}
}
}
impl std::error::Error for PageError {}
impl Into<GrowError> for PageError {
fn into(self) -> GrowError {
GrowError::ExceededMaxPages(self)
}
}
#[derive(Debug)]
pub enum MemoryCreationError {
VirtualMemoryAllocationFailed(usize, String),
CouldNotCreateMemoryFromFile(std::io::Error),
}
impl std::fmt::Display for MemoryCreationError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
MemoryCreationError::VirtualMemoryAllocationFailed(size, msg) => write!(
f,
"Allocation virtual memory with size {} failed. \nErrno message: {}",
size, msg
),
MemoryCreationError::CouldNotCreateMemoryFromFile(e) => write!(f, "IO Error: {}", e),
}
}
}
impl std::error::Error for MemoryCreationError {}
impl Into<GrowError> for MemoryCreationError {
fn into(self) -> GrowError {
GrowError::CouldNotCreateMemory(self)
}
}
impl From<std::io::Error> for MemoryCreationError {
fn from(io_error: std::io::Error) -> Self {
MemoryCreationError::CouldNotCreateMemoryFromFile(io_error)
}
}
#[derive(Debug)]
pub enum MemoryProtectionError {
ProtectionFailed(usize, usize, String),
}
impl std::fmt::Display for MemoryProtectionError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
MemoryProtectionError::ProtectionFailed(start, size, msg) => write!(
f,
"Allocation virtual memory starting at {} with size {} failed. \nErrno message: {}",
start, size, msg
),
}
}
}
impl std::error::Error for MemoryProtectionError {}
impl Into<GrowError> for MemoryProtectionError {
fn into(self) -> GrowError {
GrowError::CouldNotProtectMemory(self)
}
}

View File

@ -1,3 +1,4 @@
use crate::error::GrowError;
use crate::{
error::CreationError,
sys,
@ -14,9 +15,9 @@ pub const DYNAMIC_GUARD_SIZE: usize = 4096;
/// when first created. Over time, as it grows, it may reallocate to
/// a different location and size.
///
/// Dynamic memories are signifigantly faster to create than static
/// Dynamic memories are significantly faster to create than static
/// memories and use much less virtual memory, however, they require
/// the webassembly module to bounds-check memory accesses.
/// the WebAssembly module to bounds-check memory accesses.
///
/// While, a dynamic memory could use a vector of some sort as its
/// backing memory, we use mmap (or the platform-equivalent) to allow
@ -65,26 +66,29 @@ impl DynamicMemory {
self.current
}
pub fn grow(&mut self, delta: Pages, local: &mut vm::LocalMemory) -> Option<Pages> {
pub fn grow(&mut self, delta: Pages, local: &mut vm::LocalMemory) -> Result<Pages, GrowError> {
if delta == Pages(0) {
return Some(self.current);
return Ok(self.current);
}
let new_pages = self.current.checked_add(delta)?;
let new_pages = self.current.checked_add(delta).map_err(|e| e.into())?;
if let Some(max) = self.max {
if new_pages > max {
return None;
return Err(GrowError::ExceededMaxPagesForMemory(
new_pages.0 as usize,
max.0 as usize,
));
}
}
let mut new_memory =
sys::Memory::with_size(new_pages.bytes().0 + DYNAMIC_GUARD_SIZE).ok()?;
let mut new_memory = sys::Memory::with_size(new_pages.bytes().0 + DYNAMIC_GUARD_SIZE)
.map_err(|e| e.into())?;
unsafe {
new_memory
.protect(0..new_pages.bytes().0, sys::Protect::ReadWrite)
.ok()?;
.map_err(|e| e.into())?;
new_memory.as_slice_mut()[..self.current.bytes().0]
.copy_from_slice(&self.memory.as_slice()[..self.current.bytes().0]);
@ -97,7 +101,7 @@ impl DynamicMemory {
let old_pages = self.current;
self.current = new_pages;
Some(old_pages)
Ok(old_pages)
}
pub fn as_slice(&self) -> &[u8] {

View File

@ -1,5 +1,5 @@
use crate::{
error::CreationError,
error::{CreationError, GrowError},
export::Export,
import::IsExport,
memory::dynamic::DYNAMIC_GUARD_SIZE,
@ -89,8 +89,8 @@ impl Memory {
self.desc
}
/// Grow this memory by the specfied number of pages.
pub fn grow(&self, delta: Pages) -> Option<Pages> {
/// Grow this memory by the specified number of pages.
pub fn grow(&self, delta: Pages) -> Result<Pages, GrowError> {
match &self.variant {
MemoryVariant::Unshared(unshared_mem) => unshared_mem.grow(delta),
MemoryVariant::Shared(shared_mem) => shared_mem.grow(delta),
@ -244,7 +244,7 @@ impl UnsharedMemory {
})
}
pub fn grow(&self, delta: Pages) -> Option<Pages> {
pub fn grow(&self, delta: Pages) -> Result<Pages, GrowError> {
let mut storage = self.internal.storage.borrow_mut();
let mut local = self.internal.local.get();
@ -292,7 +292,7 @@ impl SharedMemory {
Ok(Self { desc })
}
pub fn grow(&self, _delta: Pages) -> Option<Pages> {
pub fn grow(&self, _delta: Pages) -> Result<Pages, GrowError> {
unimplemented!()
}

View File

@ -1,3 +1,4 @@
use crate::error::GrowError;
use crate::{
error::CreationError,
memory::static_::{SAFE_STATIC_GUARD_SIZE, SAFE_STATIC_HEAP_SIZE},
@ -61,27 +62,30 @@ impl StaticMemory {
self.current
}
pub fn grow(&mut self, delta: Pages, local: &mut vm::LocalMemory) -> Option<Pages> {
pub fn grow(&mut self, delta: Pages, local: &mut vm::LocalMemory) -> Result<Pages, GrowError> {
if delta == Pages(0) {
return Some(self.current);
return Ok(self.current);
}
let new_pages = self.current.checked_add(delta)?;
let new_pages = self.current.checked_add(delta).map_err(|e| e.into())?;
if let Some(max) = self.max {
if new_pages > max {
return None;
return Err(GrowError::ExceededMaxPagesForMemory(
new_pages.0 as usize,
max.0 as usize,
));
}
}
unsafe {
let _ = unsafe {
self.memory
.protect(
self.current.bytes().0..new_pages.bytes().0,
sys::Protect::ReadWrite,
)
.ok()?;
}
.map_err(|e| e.into())
}?;
local.bound = new_pages.bytes().0;
@ -89,7 +93,7 @@ impl StaticMemory {
self.current = new_pages;
Some(old_pages)
Ok(old_pages)
}
pub fn as_slice(&self) -> &[u8] {

View File

@ -1,3 +1,5 @@
use crate::error::MemoryCreationError;
use crate::error::MemoryProtectionError;
use errno;
use nix::libc;
use page_size;
@ -16,13 +18,13 @@ pub struct Memory {
}
impl Memory {
pub fn from_file_path<P>(path: P, protection: Protect) -> Result<Self, String>
pub fn from_file_path<P>(path: P, protection: Protect) -> Result<Self, MemoryCreationError>
where
P: AsRef<Path>,
{
let file = File::open(path).map_err(|e| e.to_string())?;
let file = File::open(path)?;
let file_len = file.metadata().map_err(|e| e.to_string())?.len();
let file_len = file.metadata()?.len();
let raw_fd = RawFd::from_file(file);
@ -38,7 +40,10 @@ impl Memory {
};
if ptr == -1 as _ {
Err(errno::errno().to_string())
Err(MemoryCreationError::VirtualMemoryAllocationFailed(
file_len as usize,
errno::errno().to_string(),
))
} else {
Ok(Self {
ptr: ptr as *mut u8,
@ -84,7 +89,7 @@ impl Memory {
}
}
pub fn with_size(size: usize) -> Result<Self, String> {
pub fn with_size(size: usize) -> Result<Self, MemoryCreationError> {
if size == 0 {
return Ok(Self {
ptr: ptr::null_mut(),
@ -108,7 +113,10 @@ impl Memory {
};
if ptr == -1 as _ {
Err(errno::errno().to_string())
Err(MemoryCreationError::VirtualMemoryAllocationFailed(
size,
errno::errno().to_string(),
))
} else {
Ok(Self {
ptr: ptr as *mut u8,
@ -123,7 +131,7 @@ impl Memory {
&mut self,
range: impl RangeBounds<usize>,
protection: Protect,
) -> Result<(), String> {
) -> Result<(), MemoryProtectionError> {
let protect = protection.to_protect_const();
let range_start = match range.start_bound() {
@ -147,7 +155,11 @@ impl Memory {
let success = libc::mprotect(start as _, size, protect as i32);
if success == -1 {
Err(errno::errno().to_string())
Err(MemoryProtectionError::ProtectionFailed(
start as usize,
size,
errno::errno().to_string(),
))
} else {
self.protection = protection;
Ok(())

View File

@ -1,3 +1,5 @@
use crate::error::MemoryCreationError;
use crate::error::MemoryProtectionError;
use page_size;
use std::ops::{Bound, RangeBounds};
use std::{ptr, slice};
@ -44,7 +46,7 @@ impl Memory {
}
}
pub fn with_size(size: usize) -> Result<Self, String> {
pub fn with_size(size: usize) -> Result<Self, MemoryCreationError> {
if size == 0 {
return Ok(Self {
ptr: ptr::null_mut(),
@ -58,7 +60,10 @@ impl Memory {
let ptr = unsafe { VirtualAlloc(ptr::null_mut(), size, MEM_RESERVE, PAGE_NOACCESS) };
if ptr.is_null() {
Err("unable to allocate memory".to_string())
Err(MemoryCreationError::VirtualMemoryAllocationFailed(
size,
"unable to allocate memory".to_string(),
))
} else {
Ok(Self {
ptr: ptr as *mut u8,
@ -72,7 +77,7 @@ impl Memory {
&mut self,
range: impl RangeBounds<usize>,
protect: Protect,
) -> Result<(), String> {
) -> Result<(), MemoryProtectionError> {
let protect_const = protect.to_protect_const();
let range_start = match range.start_bound() {
@ -98,7 +103,11 @@ impl Memory {
let ptr = VirtualAlloc(start as _, size, MEM_COMMIT, protect_const);
if ptr.is_null() {
Err("unable to protect memory".to_string())
Err(MemoryProtectionError::ProtectionFailed(
start as usize,
size,
"unable to protect memory".to_string(),
))
} else {
self.protection = protect;
Ok(())

View File

@ -11,6 +11,7 @@ mod anyfunc;
pub use self::anyfunc::Anyfunc;
use self::anyfunc::AnyfuncTable;
use crate::error::GrowError;
pub enum Element<'a> {
Anyfunc(Anyfunc<'a>),
@ -108,15 +109,15 @@ impl Table {
}
/// Grow this table by `delta`.
pub fn grow(&self, delta: u32) -> Option<u32> {
pub fn grow(&self, delta: u32) -> Result<u32, GrowError> {
if delta == 0 {
return Some(self.size());
return Ok(self.size());
}
match &mut *self.storage.borrow_mut() {
(TableStorage::Anyfunc(ref mut anyfunc_table), ref mut local) => {
anyfunc_table.grow(delta, local)
}
(TableStorage::Anyfunc(ref mut anyfunc_table), ref mut local) => anyfunc_table
.grow(delta, local)
.ok_or(GrowError::TableGrowError),
}
}

View File

@ -1,3 +1,4 @@
use crate::error::PageError;
use std::{
fmt,
ops::{Add, Sub},
@ -11,12 +12,16 @@ const WASM_MAX_PAGES: usize = 65_536;
pub struct Pages(pub u32);
impl Pages {
pub fn checked_add(self, rhs: Pages) -> Option<Pages> {
pub fn checked_add(self, rhs: Pages) -> Result<Pages, PageError> {
let added = (self.0 as usize) + (rhs.0 as usize);
if added <= WASM_MAX_PAGES {
Some(Pages(added as u32))
Ok(Pages(added as u32))
} else {
None
Err(PageError::ExceededMaxPages(
self.0 as usize,
rhs.0 as usize,
added,
))
}
}

View File

@ -20,10 +20,9 @@ pub unsafe extern "C" fn local_static_memory_grow(
let local_memory = *ctx.memories.add(memory_index.index());
let memory = (*local_memory).memory as *mut StaticMemory;
if let Some(old) = (*memory).grow(delta, &mut *local_memory) {
old.0 as i32
} else {
-1
match (*memory).grow(delta, &mut *local_memory) {
Ok(old) => old.0 as i32,
Err(_) => -1,
}
}
@ -45,10 +44,9 @@ pub unsafe extern "C" fn local_dynamic_memory_grow(
let local_memory = *ctx.memories.add(memory_index.index());
let memory = (*local_memory).memory as *mut DynamicMemory;
if let Some(old) = (*memory).grow(delta, &mut *local_memory) {
old.0 as i32
} else {
-1
match (*memory).grow(delta, &mut *local_memory) {
Ok(old) => old.0 as i32,
Err(_) => -1,
}
}
@ -74,10 +72,9 @@ pub unsafe extern "C" fn imported_static_memory_grow(
let local_memory = *ctx.imported_memories.add(import_memory_index.index());
let memory = (*local_memory).memory as *mut StaticMemory;
if let Some(old) = (*memory).grow(delta, &mut *local_memory) {
old.0 as i32
} else {
-1
match (*memory).grow(delta, &mut *local_memory) {
Ok(old) => old.0 as i32,
Err(_) => -1,
}
}
@ -99,10 +96,9 @@ pub unsafe extern "C" fn imported_dynamic_memory_grow(
let local_memory = *ctx.imported_memories.add(memory_index.index());
let memory = (*local_memory).memory as *mut DynamicMemory;
if let Some(old) = (*memory).grow(delta, &mut *local_memory) {
old.0 as i32
} else {
-1
match (*memory).grow(delta, &mut *local_memory) {
Ok(old) => old.0 as i32,
Err(_) => -1,
}
}