make StackWithLimit pre-allocate

src/common/stack.rs

@@ -1,40 +1,47 @@
-#[allow(unused_imports)]
+mod ol {
-use alloc::prelude::*;
+	#[allow(unused_imports)]
+	use alloc::prelude::*;
 
-#[cfg(feature = "std")]
+	use core::fmt;
-use std::error;
+	#[cfg(feature = "std")]
-use core::fmt;
+	use std::error;
 
-#[derive(Debug)]
+	#[derive(Debug)]
-pub struct Error(String);
+	pub struct Error(String);
 
-impl fmt::Display for Error {
+	impl fmt::Display for Error {
 		fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 			write!(f, "{}", self.0)
 		}
-}
+	}
 
-#[cfg(feature = "std")]
+	#[cfg(feature = "std")]
-impl error::Error for Error {
+	impl error::Error for Error {
 		fn description(&self) -> &str {
 			&self.0
 		}
-}
+	}
 
-/// Stack with limit.
+	/// Stack with limit.
-#[derive(Debug)]
+	#[derive(Debug)]
-pub struct StackWithLimit<T> where T: Clone {
+	pub struct StackWithLimit<T>
+	where
+		T: Clone,
+	{
 		/// Stack values.
 		values: Vec<T>,
 		/// Stack limit (maximal stack len).
 		limit: usize,
-}
+	}
 
-impl<T> StackWithLimit<T> where T: Clone {
+	impl<T> StackWithLimit<T>
+	where
+		T: Clone,
+	{
 		pub fn with_limit(limit: usize) -> Self {
 			StackWithLimit {
 				values: Vec::new(),
-			limit: limit
+				limit: limit,
 			}
 		}
 
@@ -58,12 +65,20 @@ impl<T> StackWithLimit<T> where T: Clone {
 				.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())));
+				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"))
+			Ok(self
+				.values
+				.get(self.values.len() - 1 - index)
+				.expect("checked couple of lines above"))
 		}
 
 		pub fn push(&mut self, value: T) -> Result<(), Error> {
@@ -85,4 +100,149 @@ impl<T> StackWithLimit<T> where T: Clone {
 			debug_assert!(new_size <= self.values.len());
 			self.values.resize(new_size, dummy);
 		}
+	}
+}
+
+#[derive(Copy, Clone, Debug)]
+pub struct StackOverflow;
+
+// impl From<StackOverflow> for TrapKind {
+// 	fn from(_: StackOverflow) -> TrapKind {
+// 		TrapKind::StackOverflow
+// 	}
+// }
+
+// impl From<StackOverflow> 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.
+#[derive(Debug)]
+pub struct StackWithLimit<T> {
+	stack: Vec<T>,
+	limit: usize,
+}
+
+impl<T> StackWithLimit<T> {
+	/// 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<T> {
+		debug_assert!(
+			limit >= initial_size,
+			"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),
+		}
+	}
+
+	/// Create an new StackWithLimit with `limit` max size and `limit` elements pre-allocated
+	pub fn with_limit(limit: usize) -> StackWithLimit<T> {
+		StackWithLimit {
+			stack: Vec::with_capacity(limit),
+			limit: limit,
+		}
+	}
+
+	/// Attempt to push value onto stack.
+	///
+	/// # Errors
+	///
+	/// 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 {
+			self.stack.push(value);
+			Ok(())
+		} else {
+			Err(StackOverflow)
+		}
+	}
+
+	pub(crate) fn pop(&mut self) -> Option<T> {
+		debug_assert!(
+			self.stack.len() <= self.limit,
+			"Stack length should never be larger than stack limit."
+		);
+		self.stack.pop()
+	}
+
+	/// Return optional reference to item in stack
+	///
+	/// `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)
+		}
+	}
+
+	pub(crate) fn top(&self) -> Option<&T> {
+		self.stack.last()
+	}
+
+	pub(crate) fn top_mut(&mut self) -> Option<&mut T> {
+		self.stack.last_mut()
+	}
+
+	pub(crate) fn truncate(&mut self, new_size: usize) {
+		self.stack.truncate(new_size)
+	}
+
+	pub(crate) fn len(&self) -> usize {
+		self.stack.len()
+	}
+
+	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<T> {
+	//     // This will be used to allocate new stacks when calling into other wasm modules
+	//     StackWithLimit::new(0, self.limit - self.len())
+	// }
+}
+
+#[cfg(test)]
+mod test {
+	use super::StackWithLimit;
+
+	fn get_relative_to_top() {
+		let mut bstack = StackWithLimit::<i32>::with_limit(2);
+		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);
+	}
 }

src/validation/func.rs

@@ -1,14 +1,14 @@
 #[allow(unused_imports)]
 use alloc::prelude::*;
-use core::u32;
-use parity_wasm::elements::{Instruction, BlockType, ValueType, TableElementType, Func, FuncBody};
 use common::{DEFAULT_MEMORY_INDEX, DEFAULT_TABLE_INDEX};
+use core::u32;
+use parity_wasm::elements::{BlockType, Func, FuncBody, Instruction, TableElementType, ValueType};
 use validation::context::ModuleContext;
 
-use validation::Error;
 use validation::util::Locals;
+use validation::Error;
 
-use common::stack::StackWithLimit;
+use common::stack::{StackOverflow, StackWithLimit};
 use isa;
 
 /// Maximum number of entries in value stack per function.
@@ -39,13 +39,9 @@ enum BlockFrameType {
 	/// Usual block frame.
 	///
 	/// Can be used for an implicit function block.
-	Block {
+	Block { end_label: LabelId },
-		end_label: LabelId,
-	},
 	/// Loop frame (branching to the beginning of block).
-	Loop {
+	Loop { header: LabelId },
-		header: LabelId,
-	},
 	/// True-subblock of if expression.
 	IfTrue {
 		/// If jump happens inside the if-true block then control will
@@ -58,9 +54,7 @@ enum BlockFrameType {
 		if_not: LabelId,
 	},
 	/// False-subblock of if expression.
-	IfFalse {
+	IfFalse { end_label: LabelId },
-		end_label: LabelId,
-	}
 }
 
 impl BlockFrameType {
@@ -164,11 +158,7 @@ enum Outcome {
 pub struct FunctionReader;
 
 impl FunctionReader {
-	pub fn read_function(
+	pub fn read_function(module: &ModuleContext, func: &Func, body: &FuncBody) -> Result<isa::Instructions, Error> {
-		module: &ModuleContext,
-		func: &Func,
-		body: &FuncBody,
-	) -> Result<isa::Instructions, Error> {
 		let (params, result_ty) = module.require_function_type(func.type_ref())?;
 
 		let ins_size_estimate = body.code().elements().len();
@@ -183,9 +173,7 @@ impl FunctionReader {
 
 		let end_label = context.sink.new_label();
 		push_label(
-			BlockFrameType::Block {
+			BlockFrameType::Block { end_label },
-				end_label,
-			},
 			result_ty,
 			context.position,
 			&context.value_stack,
@@ -207,15 +195,16 @@ impl FunctionReader {
 		loop {
 			let instruction = &body[context.position];
 
-			let outcome = FunctionReader::read_instruction(context, instruction)
+			let outcome = FunctionReader::read_instruction(context, instruction).map_err(|err| {
-				.map_err(|err| Error(format!("At instruction {:?}(@{}): {}", instruction, context.position, err)))?;
+				Error(format!(
+					"At instruction {:?}(@{}): {}",
+					instruction, context.position, err
+				))
+			})?;
 
 			match outcome {
 				Outcome::NextInstruction => (),
-				Outcome::Unreachable => make_top_frame_polymorphic(
+				Outcome::Unreachable => make_top_frame_polymorphic(&mut context.value_stack, &mut context.frame_stack),
-					&mut context.value_stack,
-					&mut context.frame_stack
-				),
 			}
 
 			context.position += 1;
@@ -229,7 +218,7 @@ impl FunctionReader {
 		use self::Instruction::*;
 		match *instruction {
 			// Nop instruction doesn't do anything. It is safe to just skip it.
-			Nop => {},
+			Nop => {}
 
 			Unreachable => {
 				context.sink.emit(isa::Instruction::Unreachable);
@@ -239,9 +228,7 @@ impl FunctionReader {
 			Block(block_type) => {
 				let end_label = context.sink.new_label();
 				push_label(
-					BlockFrameType::Block {
+					BlockFrameType::Block { end_label },
-						end_label
-					},
 					block_type,
 					context.position,
 					&context.value_stack,
@@ -254,9 +241,7 @@ impl FunctionReader {
 				context.sink.resolve_label(header);
 
 				push_label(
-					BlockFrameType::Loop {
+					BlockFrameType::Loop { header },
-						header,
-					},
 					block_type,
 					context.position,
 					&context.value_stack,
@@ -271,10 +256,7 @@ impl FunctionReader {
 
 				pop_value(&mut context.value_stack, &context.frame_stack, ValueType::I32.into())?;
 				push_label(
-					BlockFrameType::IfTrue {
+					BlockFrameType::IfTrue { if_not, end_label },
-						if_not,
-						end_label,
-					},
 					block_type,
 					context.position,
 					&context.value_stack,
@@ -283,14 +265,15 @@ impl FunctionReader {
 
 				context.sink.emit_br_eqz(Target {
 					label: if_not,
-					drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, },
+					drop_keep: isa::DropKeep {
+						drop: 0,
+						keep: isa::Keep::None,
+					},
 				});
 			}
 			Else => {
 				let (block_type, if_not, end_label) = {
-					let top_frame = top_label(
+					let top_frame = top_label(&context.frame_stack);
-						&context.frame_stack,
-					);
 
 					let (if_not, end_label) = match top_frame.frame_type {
 						BlockFrameType::IfTrue { if_not, end_label } => (if_not, end_label),
@@ -303,7 +286,10 @@ impl FunctionReader {
 				// to the "end_label" (it will be resolved at End).
 				context.sink.emit_br(Target {
 					label: end_label,
-					drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, },
+					drop_keep: isa::DropKeep {
+						drop: 0,
+						keep: isa::Keep::None,
+					},
 				});
 
 				// Resolve `if_not` to here so when if condition is unsatisfied control flow
@@ -312,14 +298,9 @@ impl FunctionReader {
 
 				// Then, we pop the current label. It discards all values that pushed in the current
 				// frame.
-				pop_label(
+				pop_label(&mut context.value_stack, &mut context.frame_stack)?;
-					&mut context.value_stack,
-					&mut context.frame_stack
-				)?;
 				push_label(
-					BlockFrameType::IfFalse {
+					BlockFrameType::IfFalse { end_label },
-						end_label,
-					},
 					block_type,
 					context.position,
 					&context.value_stack,
@@ -335,14 +316,10 @@ impl FunctionReader {
 				if let BlockFrameType::IfTrue { if_not, .. } = frame_type {
 					// A `if` without an `else` can't return a result.
 					if block_type != BlockType::NoResult {
-						return Err(
+						return Err(Error(format!(
-							Error(
-								format!(
 							"If block without else required to have NoResult block type. But it has {:?} type",
 							block_type
-								)
+						)));
-							)
-						);
 					}
 
 					// Resolve `if_not` label. If the `if's` condition doesn't hold the control will jump
@@ -362,19 +339,11 @@ impl FunctionReader {
 
 					// Check the return type.
 					if let BlockType::Value(value_type) = context.return_type()? {
-						tee_value(
+						tee_value(&mut context.value_stack, &context.frame_stack, value_type.into())?;
-							&mut context.value_stack,
-							&context.frame_stack,
-							value_type.into()
-						)?;
 					}
 
 					// Emit the return instruction.
-					let drop_keep = drop_keep_return(
+					let drop_keep = drop_keep_return(&context.locals, &context.value_stack, &context.frame_stack);
-						&context.locals,
-						&context.value_stack,
-						&context.frame_stack,
-					);
 					context.sink.emit(isa::Instruction::Return(drop_keep));
 				}
 
@@ -388,11 +357,7 @@ impl FunctionReader {
 			Br(depth) => {
 				Validator::validate_br(context, depth)?;
 
-				let target = require_target(
+				let target = require_target(depth, &context.value_stack, &context.frame_stack);
-					depth,
-					&context.value_stack,
-					&context.frame_stack,
-				);
 				context.sink.emit_br(target);
 
 				return Ok(Outcome::Unreachable);
@@ -400,11 +365,7 @@ impl FunctionReader {
 			BrIf(depth) => {
 				Validator::validate_br_if(context, depth)?;
 
-				let target = require_target(
+				let target = require_target(depth, &context.value_stack, &context.frame_stack);
-					depth,
-					&context.value_stack,
-					&context.frame_stack,
-				);
 				context.sink.emit_br_nez(target);
 			}
 			BrTable(ref table, default) => {
@@ -412,18 +373,10 @@ impl FunctionReader {
 
 				let mut targets = Vec::new();
 				for depth in table.iter() {
-					let target = require_target(
+					let target = require_target(*depth, &context.value_stack, &context.frame_stack);
-						*depth,
-						&context.value_stack,
-						&context.frame_stack,
-					);
 					targets.push(target);
 				}
-				let default_target = require_target(
+				let default_target = require_target(default, &context.value_stack, &context.frame_stack);
-					default,
-					&context.value_stack,
-					&context.frame_stack,
-				);
 				context.sink.emit_br_table(&targets, default_target);
 
 				return Ok(Outcome::Unreachable);
@@ -433,11 +386,7 @@ impl FunctionReader {
 					tee_value(&mut context.value_stack, &context.frame_stack, value_type.into())?;
 				}
 
-				let drop_keep = drop_keep_return(
+				let drop_keep = drop_keep_return(&context.locals, &context.value_stack, &context.frame_stack);
-					&context.locals,
-					&context.value_stack,
-					&context.frame_stack,
-				);
 				context.sink.emit(isa::Instruction::Return(drop_keep));
 
 				return Ok(Outcome::Unreachable);
@@ -464,37 +413,19 @@ impl FunctionReader {
 			GetLocal(index) => {
 				// We need to calculate relative depth before validation since
 				// it will change the value stack size.
-				let depth = relative_local_depth(
+				let depth = relative_local_depth(index, &context.locals, &context.value_stack)?;
-					index,
-					&context.locals,
-					&context.value_stack,
-				)?;
 				Validator::validate_get_local(context, index)?;
-				context.sink.emit(
+				context.sink.emit(isa::Instruction::GetLocal(depth));
-					isa::Instruction::GetLocal(depth),
-				);
 			}
 			SetLocal(index) => {
 				Validator::validate_set_local(context, index)?;
-				let depth = relative_local_depth(
+				let depth = relative_local_depth(index, &context.locals, &context.value_stack)?;
-					index,
+				context.sink.emit(isa::Instruction::SetLocal(depth));
-					&context.locals,
-					&context.value_stack,
-				)?;
-				context.sink.emit(
-					isa::Instruction::SetLocal(depth),
-				);
 			}
 			TeeLocal(index) => {
 				Validator::validate_tee_local(context, index)?;
-				let depth = relative_local_depth(
+				let depth = relative_local_depth(index, &context.locals, &context.value_stack)?;
-					index,
+				context.sink.emit(isa::Instruction::TeeLocal(depth));
-					&context.locals,
-					&context.value_stack,
-				)?;
-				context.sink.emit(
-					isa::Instruction::TeeLocal(depth),
-				);
 			}
 			GetGlobal(index) => {
 				Validator::validate_get_global(context, index)?;
@@ -1167,7 +1098,11 @@ impl Validator {
 		Ok(())
 	}
 
-	fn validate_cvtop(context: &mut FunctionValidationContext, value_type1: ValueType, value_type2: ValueType) -> Result<(), Error> {
+	fn validate_cvtop(
+		context: &mut FunctionValidationContext,
+		value_type1: ValueType,
+		value_type2: ValueType,
+	) -> Result<(), Error> {
 		pop_value(&mut context.value_stack, &context.frame_stack, value_type1.into())?;
 		push_value(&mut context.value_stack, value_type2.into())?;
 		Ok(())
@@ -1196,7 +1131,10 @@ impl Validator {
 		let local_type = require_local(&context.locals, index)?;
 		let value_type = pop_value(&mut context.value_stack, &context.frame_stack, StackValueType::Any)?;
 		if StackValueType::from(local_type) != value_type {
-			return Err(Error(format!("Trying to update local {} of type {:?} with value of type {:?}", index, local_type, value_type)));
+			return Err(Error(format!(
+				"Trying to update local {} of type {:?} with value of type {:?}",
+				index, local_type, value_type
+			)));
 		}
 		Ok(())
 	}
@@ -1223,14 +1161,25 @@ impl Validator {
 		};
 		let value_type = pop_value(&mut context.value_stack, &context.frame_stack, StackValueType::Any)?;
 		if global_type != value_type {
-			return Err(Error(format!("Trying to update global {} of type {:?} with value of type {:?}", index, global_type, value_type)));
+			return Err(Error(format!(
+				"Trying to update global {} of type {:?} with value of type {:?}",
+				index, global_type, value_type
+			)));
 		}
 		Ok(())
 	}
 
-	fn validate_load(context: &mut FunctionValidationContext, align: u32, max_align: u32, value_type: ValueType) -> Result<(), Error> {
+	fn validate_load(
+		context: &mut FunctionValidationContext,
+		align: u32,
+		max_align: u32,
+		value_type: ValueType,
+	) -> Result<(), Error> {
 		if 1u32.checked_shl(align).unwrap_or(u32::MAX) > max_align {
-			return Err(Error(format!("Too large memory alignment 2^{} (expected at most {})", align, max_align)));
+			return Err(Error(format!(
+				"Too large memory alignment 2^{} (expected at most {})",
+				align, max_align
+			)));
 		}
 
 		pop_value(&mut context.value_stack, &context.frame_stack, ValueType::I32.into())?;
@@ -1239,9 +1188,17 @@ impl Validator {
 		Ok(())
 	}
 
-	fn validate_store(context: &mut FunctionValidationContext, align: u32, max_align: u32, value_type: ValueType) -> Result<(), Error> {
+	fn validate_store(
+		context: &mut FunctionValidationContext,
+		align: u32,
+		max_align: u32,
+		value_type: ValueType,
+	) -> Result<(), Error> {
 		if 1u32.checked_shl(align).unwrap_or(u32::MAX) > max_align {
-			return Err(Error(format!("Too large memory alignment 2^{} (expected at most {})", align, max_align)));
+			return Err(Error(format!(
+				"Too large memory alignment 2^{} (expected at most {})",
+				align, max_align
+			)));
 		}
 
 		context.module.require_memory(DEFAULT_MEMORY_INDEX)?;
@@ -1295,15 +1252,10 @@ impl Validator {
 					BlockType::NoResult
 				};
 				if required_block_type != label_block_type {
-					return Err(
+					return Err(Error(format!(
-						Error(
-							format!(
 						"Labels in br_table points to block of different types: {:?} and {:?}",
-								required_block_type,
+						required_block_type, label_block.block_type
-								label_block.block_type
+					)));
-							)
-						)
-					);
 				}
 			}
 			required_block_type
@@ -1416,15 +1368,22 @@ fn make_top_frame_polymorphic(
 	value_stack: &mut StackWithLimit<StackValueType>,
 	frame_stack: &mut StackWithLimit<BlockFrame>,
 ) {
-	let frame = frame_stack.top_mut().expect("make_top_frame_polymorphic is called with empty frame stack");
+	let frame = frame_stack
-	value_stack.resize(frame.value_stack_len, StackValueType::Any);
+		.top_mut()
+		.expect("make_top_frame_polymorphic is called with empty frame stack");
+
+	// shrink value stack to match top frame
+	debug_assert!(frame.value_stack_len <= value_stack.len());
+	while value_stack.len() > frame.value_stack_len {
+		value_stack.pop().expect(
+			"frame.value_stack_len >= 0, value_stack.len() > frame.value_stack_len :. value_stack.len() > 0; qed",
+		);
+	}
+
 	frame.polymorphic_stack = true;
 }
 
-fn push_value(
+fn push_value(value_stack: &mut StackWithLimit<StackValueType>, value_type: StackValueType) -> Result<(), Error> {
-	value_stack: &mut StackWithLimit<StackValueType>,
-	value_type: StackValueType,
-) -> Result<(), Error> {
 	Ok(value_stack.push(value_type.into())?)
 }
 
@@ -1442,19 +1401,16 @@ fn pop_value(
 	let actual_value = if stack_is_empty && is_stack_polymorphic {
 		StackValueType::Any
 	} else {
-		let value_stack_min = frame_stack
+		let value_stack_min = frame_stack.top().expect("at least 1 topmost block").value_stack_len;
-			.top()
-			.expect("at least 1 topmost block")
-			.value_stack_len;
 		if value_stack.len() <= value_stack_min {
 			return Err(Error("Trying to access parent frame stack values.".into()));
 		}
-		value_stack.pop()?
+		value_stack
+			.pop()
+			.ok_or_else(|| Error("non-empty stack expected".into()))?
 	};
 	match actual_value {
-		StackValueType::Specific(stack_value_type) if stack_value_type == value_type => {
+		StackValueType::Specific(stack_value_type) if stack_value_type == value_type => Ok(actual_value),
-			Ok(actual_value)
-		}
 		StackValueType::Any => Ok(actual_value),
 		stack_value_type @ _ => Err(Error(format!(
 			"Expected value of type {:?} on top of stack. Got {:?}",
@@ -1497,7 +1453,10 @@ fn pop_label(
 	// Don't pop frame yet. This is essential since we still might pop values from the value stack
 	// and this in turn requires current frame to check whether or not we've reached
 	// unreachable.
-	let block_type = frame_stack.top()?.block_type;
+	let block_type = frame_stack
+		.top()
+		.ok_or_else(|| Error("non-empty stack expected".into()))?
+		.block_type;
 	match block_type {
 		BlockType::NoResult => (),
 		BlockType::Value(required_value_type) => {
@@ -1505,7 +1464,9 @@ fn pop_label(
 		}
 	}
 
-	let frame = frame_stack.pop()?;
+	let frame = frame_stack
+		.pop()
+		.ok_or_else(|| Error("non-empty stack expected".into()))?;
 	if value_stack.len() != frame.value_stack_len {
 		return Err(Error(format!(
 			"Unexpected stack height {}, expected {}",
@@ -1518,15 +1479,15 @@ fn pop_label(
 }
 
 fn top_label(frame_stack: &StackWithLimit<BlockFrame>) -> &BlockFrame {
-	frame_stack.top()
+	frame_stack
+		.top()
 		.expect("this function can't be called with empty frame stack")
 }
 
-fn require_label(
+fn require_label(depth: u32, frame_stack: &StackWithLimit<BlockFrame>) -> Result<&BlockFrame, Error> {
-	depth: u32,
+	Ok(frame_stack
-	frame_stack: &StackWithLimit<BlockFrame>,
+		.get_relative_to_top(depth as usize)
-) -> Result<&BlockFrame, Error> {
+		.ok_or_else(|| Error("non-empty stack expected".into()))?)
-	Ok(frame_stack.get(depth as usize)?)
 }
 
 fn require_target(
@@ -1534,10 +1495,8 @@ fn require_target(
 	value_stack: &StackWithLimit<StackValueType>,
 	frame_stack: &StackWithLimit<BlockFrame>,
 ) -> Target {
-	let is_stack_polymorphic = top_label(frame_stack)
+	let is_stack_polymorphic = top_label(frame_stack).polymorphic_stack;
-		.polymorphic_stack;
+	let frame = require_label(depth, frame_stack).expect("require_target called with a bogus depth");
-	let frame =
-		require_label(depth, frame_stack).expect("require_target called with a bogus depth");
 
 	// Find out how many values we need to keep (copy to the new stack location after the drop).
 	let keep: isa::Keep = match (frame.frame_type, frame.block_type) {
@@ -1601,20 +1560,14 @@ fn require_local(locals: &Locals, idx: u32) -> Result<ValueType, Error> {
 }
 
 /// See stack layout definition in mod isa.
-fn relative_local_depth(
+fn relative_local_depth(idx: u32, locals: &Locals, value_stack: &StackWithLimit<StackValueType>) -> Result<u32, Error> {
-	idx: u32,
-	locals: &Locals,
-	value_stack: &StackWithLimit<StackValueType>
-) -> Result<u32, Error> {
 	let value_stack_height = value_stack.len() as u32;
 	let locals_and_params_count = locals.count();
 
 	let depth = value_stack_height
 		.checked_add(locals_and_params_count)
 		.and_then(|x| x.checked_sub(idx))
-		.ok_or_else(||
+		.ok_or_else(|| Error(String::from("Locals range not in 32-bit range")))?;
-			Error(String::from("Locals range not in 32-bit range"))
-		)?;
 	Ok(depth)
 }
 
@@ -1659,8 +1612,7 @@ impl Sink {
 		match self.labels[label.0] {
 			(Label::Resolved(dst_pc), _) => dst_pc,
 			(Label::NotResolved, ref mut unresolved) => {
-				unresolved
+				unresolved.push(reloc_creator());
-					.push(reloc_creator());
 				u32::max_value()
 			}
 		}
@@ -1671,10 +1623,7 @@ impl Sink {
 	}
 
 	fn emit_br(&mut self, target: Target) {
-		let Target {
+		let Target { label, drop_keep } = target;
-			label,
-			drop_keep,
-		} = target;
 		let pc = self.cur_pc();
 		let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::Br { pc });
 		self.ins.push(isa::Instruction::Br(isa::Target {
@@ -1684,10 +1633,7 @@ impl Sink {
 	}
 
 	fn emit_br_eqz(&mut self, target: Target) {
-		let Target {
+		let Target { label, drop_keep } = target;
-			label,
-			drop_keep,
-		} = target;
 		let pc = self.cur_pc();
 		let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::Br { pc });
 		self.ins.push(isa::Instruction::BrIfEqz(isa::Target {
@@ -1697,10 +1643,7 @@ impl Sink {
 	}
 
 	fn emit_br_nez(&mut self, target: Target) {
-		let Target {
+		let Target { label, drop_keep } = target;
-			label,
-			drop_keep,
-		} = target;
 		let pc = self.cur_pc();
 		let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::Br { pc });
 		self.ins.push(isa::Instruction::BrIfNez(isa::Target {
@@ -1716,24 +1659,18 @@ impl Sink {
 		let mut isa_targets = Vec::new();
 		for (idx, &Target { label, drop_keep }) in targets.iter().chain(iter::once(&default)).enumerate() {
 			let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::BrTable { pc, idx });
-			isa_targets.push(
+			isa_targets.push(isa::Target {
-				isa::Target {
 				dst_pc,
 				drop_keep: drop_keep.into(),
-				},
+			});
-			);
 		}
-		self.ins.push(isa::Instruction::BrTable(
+		self.ins.push(isa::Instruction::BrTable(isa_targets.into_boxed_slice()));
-			isa_targets.into_boxed_slice(),
-		));
 	}
 
 	/// Create a new unresolved label.
 	fn new_label(&mut self) -> LabelId {
 		let label_idx = self.labels.len();
-		self.labels.push(
+		self.labels.push((Label::NotResolved, Vec::new()));
-			(Label::NotResolved, Vec::new()),
-		);
 		LabelId(label_idx)
 	}
 
@@ -1762,14 +1699,16 @@ impl Sink {
 	/// Consume this Sink and returns isa::Instructions.
 	fn into_inner(self) -> isa::Instructions {
 		// At this moment all labels should be resolved.
-		assert!({
+		assert!(
-			self.labels.iter().all(|(state, unresolved)|
+			{
-				match (state, unresolved) {
+				self.labels.iter().all(|(state, unresolved)| match (state, unresolved) {
 					(Label::Resolved(_), unresolved) if unresolved.is_empty() => true,
 					_ => false,
-				}
+				})
-			)
+			},
-		}, "there are unresolved labels left: {:?}", self.labels);
+			"there are unresolved labels left: {:?}",
+			self.labels
+		);
 		self.ins
 	}
 }

src/validation/mod.rs

@@ -1,23 +1,23 @@
 #[allow(unused_imports)]
 use alloc::prelude::*;
+use core::fmt;
 #[cfg(feature = "std")]
 use std::error;
-use core::fmt;
 
-#[cfg(feature = "std")]
-use std::collections::HashSet;
 #[cfg(not(feature = "std"))]
 use hashmap_core::HashSet;
+#[cfg(feature = "std")]
+use std::collections::HashSet;
 
-use parity_wasm::elements::{
-	BlockType, External, GlobalEntry, GlobalType, Internal, MemoryType, Module, Instruction,
-	ResizableLimits, TableType, ValueType, InitExpr, Type,
-};
-use common::stack;
 use self::context::ModuleContextBuilder;
 use self::func::FunctionReader;
-use memory_units::Pages;
+use common::stack;
 use isa;
+use memory_units::Pages;
+use parity_wasm::elements::{
+	BlockType, External, GlobalEntry, GlobalType, InitExpr, Instruction, Internal, MemoryType, Module, ResizableLimits,
+	TableType, Type, ValueType,
+};
 
 mod context;
 mod func;
@@ -42,9 +42,9 @@ impl error::Error for Error {
 	}
 }
 
-impl From<stack::Error> for Error {
+impl From<stack::StackOverflow> for Error {
-	fn from(e: stack::Error) -> Error {
+	fn from(_: stack::StackOverflow) -> Error {
-		Error(format!("Stack: {}", e))
+		Error("Stack: exeeded stack limit".into())
 	}
 }
 
@@ -158,7 +158,8 @@ pub fn deny_floating_point(module: &Module) -> Result<(), Error> {
 			if let Some(typ) = types.get(sig.type_ref() as usize) {
 				match *typ {
 					Type::Function(ref func) => {
-						if func.params()
+						if func
+							.params()
 							.iter()
 							.chain(func.return_type().as_ref())
 							.any(|&typ| typ == ValueType::F32 || typ == ValueType::F64)
@@ -189,16 +190,11 @@ pub fn validate_module(module: Module) -> Result<ValidatedModule, Error> {
 					.map(|&Type::Function(ref ty)| ty)
 					.cloned()
 					.collect()
-			})
+			}).unwrap_or_default(),
-			.unwrap_or_default(),
 	);
 
 	// Fill elements with imported values.
-	for import_entry in module
+	for import_entry in module.import_section().map(|i| i.entries()).unwrap_or_default() {
-		.import_section()
-		.map(|i| i.entries())
-		.unwrap_or_default()
-	{
 		match *import_entry.external() {
 			External::Function(idx) => context_builder.push_func_type_index(idx),
 			External::Table(ref table) => context_builder.push_table(table.clone()),
@@ -237,38 +233,29 @@ pub fn validate_module(module: Module) -> Result<ValidatedModule, Error> {
 
 	let context = context_builder.build();
 
-	let function_section_len = module
+	let function_section_len = module.function_section().map(|s| s.entries().len()).unwrap_or(0);
-		.function_section()
-		.map(|s| s.entries().len())
-		.unwrap_or(0);
 	let code_section_len = module.code_section().map(|s| s.bodies().len()).unwrap_or(0);
 	if function_section_len != code_section_len {
 		return Err(Error(format!(
 			"length of function section is {}, while len of code section is {}",
-			function_section_len,
+			function_section_len, code_section_len
-			code_section_len
 		)));
 	}
 
 	// validate every function body in user modules
 	if function_section_len != 0 {
 		// tests use invalid code
-		let function_section = module.function_section().expect(
+		let function_section = module.function_section().expect("function_section_len != 0; qed");
-			"function_section_len != 0; qed",
+		let code_section = module
-		);
+			.code_section()
-		let code_section = module.code_section().expect(
+			.expect("function_section_len != 0; function_section_len == code_section_len; qed");
-			"function_section_len != 0; function_section_len == code_section_len; qed",
-		);
 		// check every function body
 		for (index, function) in function_section.entries().iter().enumerate() {
-			let function_body = code_section.bodies().get(index as usize).ok_or(
+			let function_body = code_section
-				Error(format!(
+				.bodies()
-					"Missing body for function {}",
+				.get(index as usize)
-					index
+				.ok_or(Error(format!("Missing body for function {}", index)))?;
-				)),
+			let code = FunctionReader::read_function(&context, function, function_body).map_err(|e| {
-			)?;
-			let code = FunctionReader::read_function(&context, function, function_body)
-				.map_err(|e| {
 				let Error(ref msg) = e;
 				Error(format!("Function #{} reading/validation error: {}", index, msg))
 			})?;
@@ -280,9 +267,7 @@ pub fn validate_module(module: Module) -> Result<ValidatedModule, Error> {
 	if let Some(start_fn_idx) = module.start_section() {
 		let (params, return_ty) = context.require_function(start_fn_idx)?;
 		if return_ty != BlockType::NoResult || params.len() != 0 {
-			return Err(Error(
+			return Err(Error("start function expected to have type [] -> []".into()));
-				"start function expected to have type [] -> []".into(),
-			));
 		}
 	}
 
@@ -293,9 +278,7 @@ pub fn validate_module(module: Module) -> Result<ValidatedModule, Error> {
 			// HashSet::insert returns false if item already in set.
 			let duplicate = export_names.insert(export.field()) == false;
 			if duplicate {
-				return Err(Error(
+				return Err(Error(format!("duplicate export {}", export.field())));
-					format!("duplicate export {}", export.field()),
-				));
 			}
 			match *export.internal() {
 				Internal::Function(function_index) => {
@@ -323,10 +306,7 @@ pub fn validate_module(module: Module) -> Result<ValidatedModule, Error> {
 				}
 				External::Global(ref global_type) => {
 					if global_type.is_mutable() {
-						return Err(Error(format!(
+						return Err(Error(format!("trying to import mutable global {}", import.field())));
-							"trying to import mutable global {}",
-							import.field()
-						)));
 					}
 				}
 				External::Memory(ref memory_type) => {
@@ -382,10 +362,7 @@ pub fn validate_module(module: Module) -> Result<ValidatedModule, Error> {
 		}
 	}
 
-	Ok(ValidatedModule {
+	Ok(ValidatedModule { module, code_map })
-		module,
-		code_map,
-	})
 }
 
 fn validate_limits(limits: &ResizableLimits) -> Result<(), Error> {
@@ -428,30 +405,22 @@ fn validate_global_entry(global_entry: &GlobalEntry, globals: &[GlobalType]) ->
 fn expr_const_type(init_expr: &InitExpr, globals: &[GlobalType]) -> Result<ValueType, Error> {
 	let code = init_expr.code();
 	if code.len() != 2 {
-		return Err(Error(
+		return Err(Error("Init expression should always be with length 2".into()));
-			"Init expression should always be with length 2".into(),
-		));
 	}
 	let expr_ty: ValueType = match code[0] {
 		Instruction::I32Const(_) => ValueType::I32,
 		Instruction::I64Const(_) => ValueType::I64,
 		Instruction::F32Const(_) => ValueType::F32,
 		Instruction::F64Const(_) => ValueType::F64,
-		Instruction::GetGlobal(idx) => {
+		Instruction::GetGlobal(idx) => match globals.get(idx as usize) {
-			match globals.get(idx as usize) {
 			Some(target_global) => {
 				if target_global.is_mutable() {
 					return Err(Error(format!("Global {} is mutable", idx)));
 				}
 				target_global.content_type()
 			}
-				None => {
+			None => return Err(Error(format!("Global {} doesn't exists or not yet defined", idx))),
-					return Err(Error(
+		},
-						format!("Global {} doesn't exists or not yet defined", idx),
-					))
-				}
-			}
-		}
 		_ => return Err(Error("Non constant opcode in init expr".into())),
 	};
 	if code[1] != Instruction::End {