From 94b10306b2eda932890ce20248334ef7819a2770 Mon Sep 17 00:00:00 2001 From: Andrew Dirksen Date: Fri, 16 Nov 2018 19:46:57 -0800 Subject: [PATCH] typesafe api with tests for StackWithLimit --- src/common/stack.rs | 408 +++++++++++++++++++++++++---------------- src/lib.rs | 1 + src/validation/func.rs | 6 +- 3 files changed, 253 insertions(+), 162 deletions(-) diff --git a/src/common/stack.rs b/src/common/stack.rs index 692af3b..7a9d30d 100644 --- a/src/common/stack.rs +++ b/src/common/stack.rs @@ -1,135 +1,20 @@ -mod ol { - #[allow(unused_imports)] - use alloc::prelude::*; +use core::marker::PhantomData; +use core::usize; - use core::fmt; - #[cfg(feature = "std")] - use std::error; - - #[derive(Debug)] - pub struct Error(String); - - impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.0) - } - } - - #[cfg(feature = "std")] - impl error::Error for Error { - fn description(&self) -> &str { - &self.0 - } - } - - /// Stack with limit. - #[derive(Debug)] - pub struct StackWithLimit - where - T: Clone, - { - /// Stack values. - values: Vec, - /// Stack limit (maximal stack len). - limit: usize, - } - - impl StackWithLimit - where - T: Clone, - { - pub fn with_limit(limit: usize) -> Self { - StackWithLimit { - values: Vec::new(), - limit: limit, - } - } - - pub fn is_empty(&self) -> bool { - self.values.is_empty() - } - - pub fn len(&self) -> usize { - self.values.len() - } - - pub fn top(&self) -> Result<&T, Error> { - self.values - .last() - .ok_or_else(|| Error("non-empty stack expected".into())) - } - - pub fn top_mut(&mut self) -> Result<&mut T, Error> { - self.values - .last_mut() - .ok_or_else(|| Error("non-empty stack expected".into())) - } - - // Not the same as vector.get - pub fn get(&self, index: usize) -> Result<&T, Error> { - if index >= self.values.len() { - return Err(Error(format!( - "trying to get value at position {} on stack of size {}", - index, - self.values.len() - ))); - } - - Ok(self - .values - .get(self.values.len() - 1 - index) - .expect("checked couple of lines above")) - } - - pub fn push(&mut self, value: T) -> Result<(), Error> { - if self.values.len() >= self.limit { - return Err(Error(format!("exceeded stack limit {}", self.limit))); - } - - self.values.push(value); - Ok(()) - } - - pub fn pop(&mut self) -> Result { - self.values - .pop() - .ok_or_else(|| Error("non-empty stack expected".into())) - } - - pub fn resize(&mut self, new_size: usize, dummy: T) { - debug_assert!(new_size <= self.values.len()); - self.values.resize(new_size, dummy); - } - } -} +#[allow(unused_imports)] +use alloc::prelude::*; #[derive(Copy, Clone, Debug)] pub struct StackOverflow; -// impl From for TrapKind { -// fn from(_: StackOverflow) -> TrapKind { -// TrapKind::StackOverflow -// } -// } - -// impl From for Trap { -// fn from(_: StackOverflow) -> Trap { -// Trap::new(TrapKind::StackOverflow) -// } -// } - -// TODO: impl constructors -struct Limit(usize); -struct InitialSize(usize); - /// Pre-allocated, growable stack with upper bound on size. /// /// StackWithLimit is guaranteed never to grow larger than a set limit. /// When limit is reached attempting to push to the stack will return /// `Err(StackOverflow)`. /// -/// In addition to the limit. Initial stack size is configurable. -/// `StackWithLimit` will start out with initial size, but grow if necessary. +/// Both limit and initial stack size are configurable. +/// `StackWithLimit` will start out with initial size, but grow when necessary. #[derive(Debug)] pub struct StackWithLimit { stack: Vec, @@ -137,26 +22,54 @@ pub struct StackWithLimit { } impl StackWithLimit { - /// Create an new StackWithLimit with `limit` max size and `initial_size` elements pre-allocated - /// `initial_size` should not be larger than `limit` - pub fn new(initial_size: usize, limit: usize) -> StackWithLimit { + /// Create a StackWithLimit with `limit` max size and `initial_size` of pre-allocated + /// memory. + /// + /// ``` + /// # extern crate wasmi; + /// # use wasmi::{StackWithLimit, StackSize, FuncRef}; + /// StackWithLimit::::new( + /// StackSize::from_element_count(1024).into_initial(), + /// StackSize::from_element_count(2048).into_limit(), + /// ); + /// ``` + /// + /// Unlimited + /// + /// ``` + /// # extern crate wasmi; + /// # use wasmi::{StackWithLimit, StackSize, RuntimeValue}; + /// StackWithLimit::::new( + /// StackSize::from_element_count(1024).into_initial(), + /// StackSize::unlimited(), + /// ); + /// ``` + /// + /// # Errors + /// + /// In debug mode, panics if `initial_size` is larger than `limit`. + pub fn new(initial_size: StackSizeInitial, limit: StackSizeLimit) -> StackWithLimit { + let initial_size_elements = initial_size.0.element_count(); + let limit_elements = limit.0.element_count(); debug_assert!( - limit >= initial_size, + limit_elements >= initial_size_elements, "initial_size should not be larger than StackWithLimit limit" ); - use std::cmp::min; StackWithLimit { - stack: Vec::with_capacity(initial_size), - limit: min(initial_size, limit), + stack: Vec::with_capacity(initial_size_elements.min(limit_elements)), + limit: limit_elements, } } /// Create an new StackWithLimit with `limit` max size and `limit` elements pre-allocated - pub fn with_limit(limit: usize) -> StackWithLimit { - StackWithLimit { - stack: Vec::with_capacity(limit), - limit: limit, - } + /// + /// ``` + /// # extern crate wasmi; + /// # use wasmi::{StackWithLimit, StackSize}; + /// let bstack = StackWithLimit::::with_size(StackSize::from_element_count(2)); + /// ``` + pub fn with_size(size: StackSize) -> StackWithLimit { + StackWithLimit::new(StackSizeInitial(size), StackSizeLimit(size)) } /// Attempt to push value onto stack. @@ -165,16 +78,18 @@ impl StackWithLimit { /// /// Returns Err(StackOverflow) if stack is already full. pub(crate) fn push(&mut self, value: T) -> Result<(), StackOverflow> { - debug_assert!( - self.stack.len() <= self.limit, - "Stack length should never be larger than stack limit." - ); - if self.stack.len() < self.limit { + println!("limit {}", self.limit); + let ret = if self.stack.len() < self.limit { self.stack.push(value); Ok(()) } else { Err(StackOverflow) - } + }; + debug_assert!( + self.stack.len() <= self.limit, + "Stack length should never be larger than stack limit." + ); + ret } pub(crate) fn pop(&mut self) -> Option { @@ -190,18 +105,15 @@ impl StackWithLimit { /// `bstack.get_relative_to_top(0)` gets the top of the stack /// /// `bstack.get_relative_to_top(1)` gets the item just below the stack - /// - pub(crate) fn get_relative_to_top(&self, depth: usize) -> Option<&T> { - let offset = depth + 1; - if self.stack.len() < offset { - None - } else { - // We should be cognizant of integer underflow here. - // If offset > len(), (len() - offset) will underflow. - // In debug builds, underflow panics, but in release mode, underflow is not checked. - self.stack.get(self.stack.len() - offset) - } + // Be cognizant of integer underflow and overflow here. Both are possible in this situation. + // len() is unsigned, so if len() == 0, subtraction is a problem + // depth can legally be 2^32. On a 32 bit system, adding may overflow + // overflow isn't an issue unless T is zero size + // In debug builds, underflow panics, but in release mode, underflow is not checked. + + let index = self.stack.len().checked_sub(1)?.checked_sub(depth)?; + self.stack.get(index) } pub(crate) fn top(&self) -> Option<&T> { @@ -212,6 +124,7 @@ impl StackWithLimit { self.stack.last_mut() } + // Same as Vec::[truncate](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.truncate) pub(crate) fn truncate(&mut self, new_size: usize) { self.stack.truncate(new_size) } @@ -223,26 +136,203 @@ impl StackWithLimit { pub(crate) fn is_empty(&self) -> bool { self.stack.is_empty() } - - // /// return a new empty StackWithLimit with limit equal to the amount of room - // /// this stack has available - // pub fn spare(&self) -> StackWithLimit { - // // This will be used to allocate new stacks when calling into other wasm modules - // StackWithLimit::new(0, self.limit - self.len()) - // } } +// Why introduce the extra complexity of StackSizeLimit, StackSizeInitial, and StackSize? +// We want to make the user to do the correct thing using the type checker. +// Check out the excellent Rust API Guidelines (linked below) for suggestions +// about type safety in rust. +// +// By introducing the new typed arguments, we turn: +// +// ``` +// pub fn new(initial_size: usize, limit: usize) -> StackWithLimit { ... } +// ``` +// +// into: +// +// ``` +// pub fn new(initial_size: StackSizeInitial, limit: StackSizeLimit) -> StackWithLimit { ... } +// ``` +// +// https://rust-lang-nursery.github.io/api-guidelines/type-safety.html#c-custom-type + +/// Type for communicating the size of some contigous container. +/// Used for constructing both [`StackSizeLimit`] and [`StackSizeInitial`]. +#[derive(Eq, PartialEq, Hash, Debug)] +pub struct StackSize { + num_elements: usize, + // PhantomData is a zero-sized type for keeping track of T without rustc complaining. + phantom: PhantomData<*const T>, +} + +impl Clone for StackSize { + fn clone(&self) -> Self { + StackSize { + num_elements: self.num_elements, + phantom: PhantomData, + } + } +} + +impl Copy for StackSize {} + +impl StackSize { + /// Create StackSize based on number of elements. + /// + /// ``` + /// # extern crate wasmi; + /// # use wasmi::StackSize; + /// let ss = StackSize::<(u8, u8)>::from_element_count(10); + /// assert_eq!(ss.element_count(), 10); + /// ``` + pub fn from_element_count(num_elements: usize) -> StackSize { + StackSize { + num_elements, + phantom: PhantomData, + } + } + + /// Compute StackSize based on allowable memory. + /// + /// ``` + /// # extern crate wasmi; + /// # use wasmi::StackSize; + /// let ss = StackSize::<(u8, u8)>::from_byte_count(10); + /// assert_eq!(ss.element_count(), 10 / 2); + /// ``` + /// + /// # Errors + /// + /// In debug mode, panics if size of `T` is 0. + pub fn from_byte_count(num_bytes: usize) -> StackSize { + // This debug_assert should catch logical errors. + debug_assert!(::core::mem::size_of::() != 0, "That doesn't make sense."); + + // In case a zero sized T still makes it into prod. We assume unlimited stack + // size instead of panicking. + let element_count = if ::core::mem::size_of::() != 0 { + num_bytes / ::core::mem::size_of::() + } else { + usize::MAX // Semi-relevant fun fact: Vec::<()>::new().capacity() == usize::MAX + }; + + StackSize::from_element_count(element_count) + } + + /// Return number the of elements this StackSize indicates. + /// + /// ``` + /// # use wasmi::StackSize; + /// let ss = StackSize::<(u8, u8)>::from_element_count(10); + /// assert_eq!(ss.element_count(), 10); + /// ``` + /// + pub fn element_count(&self) -> usize { + self.num_elements + } + + /// Create StackSizeLimit out of self + /// + /// ``` + /// # use wasmi::{StackSize, StackSizeLimit, RuntimeValue}; + /// let values_limit: StackSizeLimit = StackSize::from_element_count(1024).into_limit(); + /// ``` + pub fn into_limit(self) -> StackSizeLimit { + StackSizeLimit(self) + } + + /// Create StackSizeLimit out of self + pub fn into_initial(self) -> StackSizeInitial { + StackSizeInitial(self) + } + + /// Create StackSizeLimit with no upper bound. + pub fn unlimited() -> StackSizeLimit { + StackSize::from_element_count(usize::MAX).into_limit() + } +} + +/// Max size a stack may become. +/// +/// Constructed by [`StackSize::into_limit`](into_limit) or [`StackSize::unlimited`](unlimited) +/// +/// [into_limit]: type.StackSize.into_limit.html +/// [unlimited]: type.StackSize.unlimited.html +pub struct StackSizeLimit(StackSize); + +/// Number of pre-allocated elements. +/// +/// Constructed by [`StackSize::into_initial`] +/// +/// [into_initial]: type.StackSize.into_initial.html +pub struct StackSizeInitial(StackSize); + #[cfg(test)] mod test { - use super::StackWithLimit; + use super::{StackSize, StackSizeInitial, StackSizeLimit, StackWithLimit}; + use core::usize; + #[test] fn get_relative_to_top() { - let mut bstack = StackWithLimit::::with_limit(2); + let mut bstack = StackWithLimit::::with_size(StackSize::from_element_count(2)); + assert_eq!(bstack.get_relative_to_top(0), None); bstack.push(1).unwrap(); bstack.push(2).unwrap(); bstack.push(3).unwrap_err(); assert_eq!(bstack.get_relative_to_top(0), Some(&2)); assert_eq!(bstack.get_relative_to_top(1), Some(&1)); assert_eq!(bstack.get_relative_to_top(2), None); + assert_eq!(bstack.get_relative_to_top(3), None); + } + + fn exersize(mut bstack: StackWithLimit) { + assert!(bstack.is_empty()); + assert_eq!(bstack.len(), 0); + assert_eq!(bstack.top(), None); + assert_eq!(bstack.top_mut(), None); + assert_eq!(bstack.pop(), None); + bstack.push(0).unwrap(); + assert!(!bstack.is_empty()); + assert_eq!(bstack.len(), 1); + assert_eq!(bstack.top(), Some(&0)); + assert_eq!(bstack.top_mut(), Some(&mut 0)); + assert_eq!(bstack.pop(), Some(0)); + + bstack.push(0).unwrap(); + bstack.push(0).unwrap(); + bstack.push(0).unwrap(); + bstack.push(0).unwrap(); + assert_eq!(bstack.len(), 4); + bstack.truncate(8); + assert_eq!(bstack.len(), 4); + bstack.truncate(4); + assert_eq!(bstack.len(), 4); + bstack.truncate(2); + assert_eq!(bstack.len(), 2); + bstack.truncate(0); + assert_eq!(bstack.len(), 0); + } + + #[test] + fn stack_with_limit() { + let bstack = StackWithLimit::::with_size(StackSize::from_element_count(20)); + exersize(bstack); + } + + // Check for integer overflow bugs + #[test] + fn practically_unlimited_stack() { + let bstack = StackWithLimit::::new( + StackSizeInitial(StackSize::from_element_count(0)), + StackSizeLimit(StackSize::from_element_count(usize::MAX)), + ); + println!( + "{}", + StackSizeLimit(StackSize::::from_element_count(usize::MAX)) + .0 + .element_count() + ); + exersize(bstack); } } diff --git a/src/lib.rs b/src/lib.rs index eeb72e7..cad8752 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -404,6 +404,7 @@ pub use self::module::{ModuleInstance, ModuleRef, ExternVal, NotStartedModuleRef pub use self::global::{GlobalInstance, GlobalRef}; pub use self::func::{FuncInstance, FuncRef, FuncInvocation, ResumableError}; pub use self::types::{Signature, ValueType, GlobalDescriptor, TableDescriptor, MemoryDescriptor}; +pub use self::common::stack::{StackWithLimit, StackSize, StackSizeLimit, StackSizeInitial}; /// WebAssembly-specific sizes and units. pub mod memory_units { diff --git a/src/validation/func.rs b/src/validation/func.rs index f53bdca..3fdd513 100644 --- a/src/validation/func.rs +++ b/src/validation/func.rs @@ -8,7 +8,7 @@ use validation::context::ModuleContext; use validation::util::Locals; use validation::Error; -use common::stack::{StackOverflow, StackWithLimit}; +use common::stack::{StackSize, StackWithLimit}; use isa; /// Maximum number of entries in value stack per function. @@ -1348,8 +1348,8 @@ impl<'a> FunctionValidationContext<'a> { module: module, position: 0, locals: locals, - value_stack: StackWithLimit::with_limit(value_stack_limit), - frame_stack: StackWithLimit::with_limit(frame_stack_limit), + value_stack: StackWithLimit::with_size(StackSize::from_element_count(value_stack_limit)), + frame_stack: StackWithLimit::with_size(StackSize::from_element_count(frame_stack_limit)), return_type: return_type, sink: Sink::with_instruction_capacity(size_estimate), }