Skip to content

Commit

Permalink
Fix bugs found during fuzz testing (#410)
Browse files Browse the repository at this point in the history
* reorder imports

* fix bug in translation of i32_trunc_sat_{f32,f64}_{s,u}

* fix bug in translation of unreachable control blocks invalidating the stack

* apply rustfmt
  • Loading branch information
Robbepop authored Aug 17, 2022
1 parent b02417c commit 43d7037
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 30 deletions.
25 changes: 7 additions & 18 deletions wasmi_v1/src/engine/func_builder/control_frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ pub struct UnreachableControlFrame {
pub block_type: BlockType,
/// The kind of the unreachable control flow frame.
pub kind: ControlFrameKind,
/// The value stack size upon entering the unreachable control frame.
pub stack_height: u32,
}

/// The kind of a control flow frame.
Expand All @@ -197,24 +195,15 @@ pub enum ControlFrameKind {

impl UnreachableControlFrame {
/// Creates a new [`UnreachableControlFrame`] with the given type and kind.
pub fn new(kind: ControlFrameKind, block_type: BlockType, stack_height: u32) -> Self {
Self {
kind,
block_type,
stack_height,
}
pub fn new(kind: ControlFrameKind, block_type: BlockType) -> Self {
Self { kind, block_type }
}

/// Returns the [`ControlFrameKind`] of the [`UnreachableControlFrame`].
pub fn kind(&self) -> ControlFrameKind {
self.kind
}

/// Returns the value stack height upon entering the [`IfControlFrame`].
pub fn stack_height(&self) -> u32 {
self.stack_height
}

/// Returns the [`BlockType`] of the [`IfControlFrame`].
pub fn block_type(&self) -> BlockType {
self.block_type
Expand Down Expand Up @@ -303,12 +292,12 @@ impl ControlFrame {
}

/// Returns the value stack height upon entering the control flow frame.
pub fn stack_height(&self) -> u32 {
pub fn stack_height(&self) -> Option<u32> {
match self {
Self::Block(frame) => frame.stack_height(),
Self::Loop(frame) => frame.stack_height(),
Self::If(frame) => frame.stack_height(),
Self::Unreachable(frame) => frame.stack_height(),
Self::Block(frame) => Some(frame.stack_height()),
Self::Loop(frame) => Some(frame.stack_height()),
Self::If(frame) => Some(frame.stack_height()),
Self::Unreachable(_frame) => None,
}
}

Expand Down
21 changes: 9 additions & 12 deletions wasmi_v1/src/engine/func_builder/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use alloc::vec::Vec;

mod control_frame;
mod control_stack;
mod inst_builder;
Expand Down Expand Up @@ -38,6 +36,7 @@ use crate::{
ModuleError,
Mutability,
};
use alloc::vec::Vec;
use wasmi_core::{Value, ValueType, F32, F64};

/// The interface to translate a `wasmi` bytecode function using Wasm bytecode.
Expand Down Expand Up @@ -223,7 +222,7 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {
};
// Find out how many values we need to drop.
let current_height = self.value_stack.len();
let origin_height = frame.stack_height();
let origin_height = frame.stack_height().expect("frame is reachable");
assert!(
origin_height <= current_height,
"encountered value stack underflow: current height {}, original height {}",
Expand Down Expand Up @@ -358,8 +357,8 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {

/// Translates a Wasm `block` control flow operator.
pub fn translate_block(&mut self, block_type: BlockType) -> Result<(), ModuleError> {
let stack_height = self.frame_stack_height(block_type);
if self.is_reachable() {
let stack_height = self.frame_stack_height(block_type);
let end_label = self.inst_builder.new_label();
self.control_frames.push_frame(BlockControlFrame::new(
block_type,
Expand All @@ -370,16 +369,15 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {
self.control_frames.push_frame(UnreachableControlFrame::new(
ControlFrameKind::Block,
block_type,
stack_height,
));
}
Ok(())
}

/// Translates a Wasm `loop` control flow operator.
pub fn translate_loop(&mut self, block_type: BlockType) -> Result<(), ModuleError> {
let stack_height = self.frame_stack_height(block_type);
if self.is_reachable() {
let stack_height = self.frame_stack_height(block_type);
let header = self.inst_builder.new_label();
self.inst_builder.resolve_label(header);
self.control_frames
Expand All @@ -388,7 +386,6 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {
self.control_frames.push_frame(UnreachableControlFrame::new(
ControlFrameKind::Loop,
block_type,
stack_height,
));
}
Ok(())
Expand All @@ -413,11 +410,9 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {
self.inst_builder
.push_inst(Instruction::BrIfEqz(branch_target));
} else {
let stack_height = self.frame_stack_height(block_type);
self.control_frames.push_frame(UnreachableControlFrame::new(
ControlFrameKind::If,
block_type,
stack_height,
));
}
Ok(())
Expand Down Expand Up @@ -499,7 +494,9 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {
// frame was reachable upon entering to begin with.
self.reachable = frame_reachable;
}
self.value_stack.shrink_to(frame_stack_height);
if let Some(frame_stack_height) = frame_stack_height {
self.value_stack.shrink_to(frame_stack_height);
}
let frame = self.control_frames.pop_frame();
frame
.block_type()
Expand Down Expand Up @@ -1926,7 +1923,7 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {

/// Translate a Wasm `u64.truncate_sat_f32` instruction.
pub fn translate_u64_truncate_saturate_f32(&mut self) -> Result<(), ModuleError> {
self.translate_conversion(ValueType::F32, ValueType::I32, Instruction::I64TruncSatF32U)
self.translate_conversion(ValueType::F32, ValueType::I64, Instruction::I64TruncSatF32U)
}

/// Translate a Wasm `i64.truncate_sat_f64` instruction.
Expand All @@ -1936,6 +1933,6 @@ impl<'engine, 'parser> FunctionBuilder<'engine, 'parser> {

/// Translate a Wasm `u64.truncate_sat_f64` instruction.
pub fn translate_u64_truncate_saturate_f64(&mut self) -> Result<(), ModuleError> {
self.translate_conversion(ValueType::F64, ValueType::I32, Instruction::I64TruncSatF64U)
self.translate_conversion(ValueType::F64, ValueType::I64, Instruction::I64TruncSatF64U)
}
}

0 comments on commit 43d7037

Please sign in to comment.