diff --git a/crates/wasmi/Cargo.toml b/crates/wasmi/Cargo.toml index c370c77b4c..0d93d44084 100644 --- a/crates/wasmi/Cargo.toml +++ b/crates/wasmi/Cargo.toml @@ -57,6 +57,18 @@ std = [ # An example of such an environment is `wasm32-unknown-unknown`. no-hash-maps = ["wasmi_collections/no-hash-maps"] +# Enables extra checks performed during Wasmi bytecode execution. +# +# These checks are unnecessary as long as Wasmi translation works as intended. +# If Wasmi translation invariants are broken due to bugs, these checks prevent +# Wasmi execution to exhibit undefined behavior (UB) in certain cases. +# +# Expected execution overhead is upt to 20%, if enabled. +# +# - Enable if your focus is on safety. +# - Disable if your focus is on execution speed. +extra-checks = [] + [[bench]] name = "benches" harness = false diff --git a/crates/wasmi/src/engine/code_map.rs b/crates/wasmi/src/engine/code_map.rs index a974c181c5..7ea8b03ca5 100644 --- a/crates/wasmi/src/engine/code_map.rs +++ b/crates/wasmi/src/engine/code_map.rs @@ -487,7 +487,9 @@ impl FuncEntity { // Safety: we just asserted that `self` must be an uncompiled function // since otherwise we would have returned `None` above. // Since this is a performance critical path we need to leave out this check. - unsafe { core::hint::unreachable_unchecked() } + unsafe { + unreachable_unchecked!("expected uncompiled function but found: {self:?}") + } } } } diff --git a/crates/wasmi/src/engine/executor/instrs.rs b/crates/wasmi/src/engine/executor/instrs.rs index 17e5290088..7b3ab2b26a 100644 --- a/crates/wasmi/src/engine/executor/instrs.rs +++ b/crates/wasmi/src/engine/executor/instrs.rs @@ -7,6 +7,7 @@ use crate::{ bytecode::{index, BlockFuel, Const16, Instruction, Reg}, code_map::CodeMap, executor::stack::{CallFrame, FrameRegisters, ValueStack}, + utils::unreachable_unchecked, DedupFuncType, EngineFunc, }, @@ -1418,9 +1419,14 @@ macro_rules! get_entity { unsafe { self.cache.$name(index) } .unwrap_or_else(|| { const ENTITY_NAME: &'static str = ::core::stringify!($id_ty); - ::core::unreachable!( - "missing {ENTITY_NAME} at index {index:?} for the currently used instance", - ) + // Safety: within the Wasmi executor it is assumed that store entity + // indices within the Wasmi bytecode are always valid for the + // store. This is an invariant of the Wasmi translation. + unsafe { + unreachable_unchecked!( + "missing {ENTITY_NAME} at index {index:?} for the currently used instance", + ) + } }) } )* @@ -1727,7 +1733,13 @@ impl<'engine> Executor<'engine> { /// This includes [`Instruction`] variants such as [`Instruction::TableIndex`] /// that primarily carry parameters for actually executable [`Instruction`]. fn invalid_instruction_word(&mut self) -> Result<(), Error> { - self.execute_trap(TrapCode::UnreachableCodeReached) + // Safety: Wasmi translation guarantees that branches are never taken to instruction parameters directly. + unsafe { + unreachable_unchecked!( + "expected instruction but found instruction parameter: {:?}", + *self.ip.get() + ) + } } /// Executes a Wasm `unreachable` instruction. diff --git a/crates/wasmi/src/engine/executor/instrs/branch.rs b/crates/wasmi/src/engine/executor/instrs/branch.rs index 3d6b147260..2dd45f3ad6 100644 --- a/crates/wasmi/src/engine/executor/instrs/branch.rs +++ b/crates/wasmi/src/engine/executor/instrs/branch.rs @@ -1,14 +1,17 @@ use super::Executor; use crate::{ core::UntypedVal, - engine::bytecode::{ - BranchOffset, - BranchOffset16, - Comparator, - ComparatorAndOffset, - Const16, - Instruction, - Reg, + engine::{ + bytecode::{ + BranchOffset, + BranchOffset16, + Comparator, + ComparatorAndOffset, + Const16, + Instruction, + Reg, + }, + utils::unreachable_unchecked, }, }; use core::cmp; @@ -58,7 +61,14 @@ impl<'engine> Executor<'engine> { Instruction::Const32 { value } => UntypedVal::from(u32::from(value)), Instruction::I64Const32 { value } => UntypedVal::from(i64::from(value)), Instruction::F64Const32 { value } => UntypedVal::from(f64::from(value)), - _ => unreachable!(), + unexpected => { + // Safety: one of the above instruction parameters is guaranteed to exist by the Wasmi translation. + unsafe { + unreachable_unchecked!( + "expected instruction parameter for `Instruction::BranchTable1` but found: {unexpected:?}" + ) + } + } }; self.ip.add(offset); if let Instruction::BranchTableTarget { results, offset } = *self.ip.get() { @@ -72,8 +82,16 @@ impl<'engine> Executor<'engine> { pub fn execute_branch_table_2(&mut self, index: Reg, len_targets: u32) { let offset = self.fetch_branch_table_offset(index, len_targets); self.ip.add(1); - let Instruction::Register2 { regs } = *self.ip.get() else { - unreachable!() + let regs = match *self.ip.get() { + Instruction::Register2 { regs } => regs, + unexpected => { + // Safety: Wasmi translation guarantees that `Instruction::Register2` follows. + unsafe { + unreachable_unchecked!( + "expected `Instruction::Register2` but found: {unexpected:?}" + ) + } + } }; self.ip.add(offset); if let Instruction::BranchTableTarget { results, offset } = *self.ip.get() { @@ -91,8 +109,16 @@ impl<'engine> Executor<'engine> { pub fn execute_branch_table_3(&mut self, index: Reg, len_targets: u32) { let offset = self.fetch_branch_table_offset(index, len_targets); self.ip.add(1); - let Instruction::Register3 { regs } = *self.ip.get() else { - unreachable!() + let regs = match *self.ip.get() { + Instruction::Register3 { regs } => regs, + unexpected => { + // Safety: Wasmi translation guarantees that `Instruction::Register3` follows. + unsafe { + unreachable_unchecked!( + "expected `Instruction::Register3` but found: {unexpected:?}" + ) + } + } }; self.ip.add(offset); if let Instruction::BranchTableTarget { results, offset } = *self.ip.get() { @@ -110,8 +136,16 @@ impl<'engine> Executor<'engine> { pub fn execute_branch_table_span(&mut self, index: Reg, len_targets: u32) { let offset = self.fetch_branch_table_offset(index, len_targets); self.ip.add(1); - let Instruction::RegisterSpan { span: values } = *self.ip.get() else { - unreachable!() + let values = match *self.ip.get() { + Instruction::RegisterSpan { span } => span, + unexpected => { + // Safety: Wasmi translation guarantees that `Instruction::RegisterSpan` follows. + unsafe { + unreachable_unchecked!( + "expected `Instruction::RegisterSpan` but found: {unexpected:?}" + ) + } + } }; let len = values.len(); let values = values.span(); @@ -154,7 +188,12 @@ impl<'engine> Executor<'engine> { // will point to `Instruction::Return` which does the job for us. // This has some technical advantages for us. } - _ => unreachable!(), + unexpected => { + // Safety: Wasmi translator guarantees that one of the above `Instruction` variants exists. + unsafe { + unreachable_unchecked!("expected target for `Instruction::BranchTableMany` but found: {unexpected:?}") + } + } } } diff --git a/crates/wasmi/src/engine/executor/instrs/call.rs b/crates/wasmi/src/engine/executor/instrs/call.rs index fd5b7914de..3abdc140df 100644 --- a/crates/wasmi/src/engine/executor/instrs/call.rs +++ b/crates/wasmi/src/engine/executor/instrs/call.rs @@ -5,6 +5,7 @@ use crate::{ bytecode::{index, Instruction, Reg, RegSpan}, code_map::CompiledFuncRef, executor::stack::{CallFrame, FrameParams, ValueStack}, + utils::unreachable_unchecked, EngineFunc, FuncParams, }, @@ -184,9 +185,14 @@ impl<'engine> Executor<'engine> { let index = u32::from(self.get_register(index)); (index, table) } - unexpected => unreachable!( - "expected `Instruction::CallIndirectParams[Imm16]` but found {unexpected:?}" - ), + unexpected => { + // Safety: Wasmi translation guarantees that correct instruction parameter follows. + unsafe { + unreachable_unchecked!( + "expected `Instruction::CallIndirectParams` but found {unexpected:?}" + ) + } + } } } @@ -208,9 +214,14 @@ impl<'engine> Executor<'engine> { let index = u32::from(index); (index, table) } - unexpected => unreachable!( - "expected `Instruction::CallIndirectParams[Imm16]` but found {unexpected:?}" - ), + unexpected => { + // Safety: Wasmi translation guarantees that correct instruction parameter follows. + unsafe { + unreachable_unchecked!( + "expected `Instruction::CallIndirectParamsImm16` but found {unexpected:?}" + ) + } + } } } @@ -260,9 +271,12 @@ impl<'engine> Executor<'engine> { self.copy_regs(uninit_params, regs); } unexpected => { - unreachable!( - "unexpected Instruction found while copying call parameters: {unexpected:?}" - ) + // Safety: Wasmi translation guarantees that register list finalizer exists. + unsafe { + unreachable_unchecked!( + "expected register-list finalizer but found: {unexpected:?}" + ) + } } } } diff --git a/crates/wasmi/src/engine/executor/instrs/copy.rs b/crates/wasmi/src/engine/executor/instrs/copy.rs index 117a84fb3b..9bad016add 100644 --- a/crates/wasmi/src/engine/executor/instrs/copy.rs +++ b/crates/wasmi/src/engine/executor/instrs/copy.rs @@ -1,7 +1,10 @@ use super::{Executor, InstructionPtr}; use crate::{ core::UntypedVal, - engine::bytecode::{AnyConst32, Const32, FixedRegSpan, Instruction, Reg, RegSpan}, + engine::{ + bytecode::{AnyConst32, Const32, FixedRegSpan, Instruction, Reg, RegSpan}, + utils::unreachable_unchecked, + }, }; use core::slice; use smallvec::SmallVec; @@ -132,9 +135,14 @@ impl<'engine> Executor<'engine> { Instruction::Register { reg } => slice::from_ref(reg), Instruction::Register2 { regs } => regs, Instruction::Register3 { regs } => regs, - unexpected => unreachable!( - "unexpected Instruction found while copying many values: {unexpected:?}" - ), + unexpected => { + // Safety: Wasmi translator guarantees that register-list finalizer exists. + unsafe { + unreachable_unchecked!( + "expected register-list finalizer but found: {unexpected:?}" + ) + } + } }; tmp.extend(values.iter().map(|value| self.get_register(*value))); for (result, value) in results.iter_sized(tmp.len()).zip(tmp) { @@ -175,7 +183,14 @@ impl<'engine> Executor<'engine> { Instruction::Register { reg } => slice::from_ref(reg), Instruction::Register2 { regs } => regs, Instruction::Register3 { regs } => regs, - unexpected => unreachable!("unexpected Instruction found while copying many non-overlapping values: {unexpected:?}"), + unexpected => { + // Safety: Wasmi translator guarantees that register-list finalizer exists. + unsafe { + unreachable_unchecked!( + "expected register-list finalizer but found: {unexpected:?}" + ) + } + } }; copy_values(values); ip diff --git a/crates/wasmi/src/engine/executor/instrs/load.rs b/crates/wasmi/src/engine/executor/instrs/load.rs index 6a578f7f0b..f9129d6aae 100644 --- a/crates/wasmi/src/engine/executor/instrs/load.rs +++ b/crates/wasmi/src/engine/executor/instrs/load.rs @@ -4,6 +4,7 @@ use crate::{ engine::{ bytecode::{Const16, Reg}, executor::instr_ptr::InstructionPtr, + utils::unreachable_unchecked, }, ir::{index::Memory, Instruction}, store::StoreInner, @@ -22,7 +23,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::RegisterAndImm32 { reg, imm } => (reg, u32::from(imm)), instr => { - unreachable!("expected an `Instruction::RegisterAndImm32` but found: {instr:?}") + // Safety: Wasmi translation guarantees that `Instruction::RegisterAndImm32` exists. + unsafe { + unreachable_unchecked!( + "expected an `Instruction::RegisterAndImm32` but found: {instr:?}" + ) + } } } } diff --git a/crates/wasmi/src/engine/executor/instrs/memory.rs b/crates/wasmi/src/engine/executor/instrs/memory.rs index d76e8ae4a2..b33b250ded 100644 --- a/crates/wasmi/src/engine/executor/instrs/memory.rs +++ b/crates/wasmi/src/engine/executor/instrs/memory.rs @@ -1,7 +1,10 @@ use super::{Executor, InstructionPtr}; use crate::{ core::TrapCode, - engine::bytecode::{index::Data, Const16, Instruction, Reg}, + engine::{ + bytecode::{index::Data, Const16, Instruction, Reg}, + utils::unreachable_unchecked, + }, error::EntityGrowError, ir::index::Memory, store::{ResourceLimiterRef, StoreInner}, @@ -17,7 +20,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::MemoryIndex { index } => index, unexpected => { - unreachable!("expected `Instruction::MemoryIndex` but found: {unexpected:?}") + // Safety: Wasmi translation guarantees that [`Instruction::MemoryIndex`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::MemoryIndex` but found: {unexpected:?}" + ) + } } } } @@ -29,7 +37,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::DataIndex { index } => index, unexpected => { - unreachable!("expected `Instruction::DataIndex` but found: {unexpected:?}") + // Safety: Wasmi translation guarantees that [`Instruction::DataIndex`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::DataIndex` but found: {unexpected:?}" + ) + } } } } diff --git a/crates/wasmi/src/engine/executor/instrs/return_.rs b/crates/wasmi/src/engine/executor/instrs/return_.rs index e89a491a8c..60b00a10d8 100644 --- a/crates/wasmi/src/engine/executor/instrs/return_.rs +++ b/crates/wasmi/src/engine/executor/instrs/return_.rs @@ -4,6 +4,7 @@ use crate::{ engine::{ bytecode::{AnyConst32, BoundedRegSpan, Const32, Instruction, Reg, RegSpan}, executor::stack::FrameRegisters, + utils::unreachable_unchecked, }, store::StoreInner, }; @@ -236,7 +237,14 @@ impl<'engine> Executor<'engine> { Instruction::Register { reg } => slice::from_ref(reg), Instruction::Register2 { regs } => regs, Instruction::Register3 { regs } => regs, - unexpected => unreachable!("unexpected `Instruction` found while executing `Instruction::ReturnMany`: {unexpected:?}"), + unexpected => { + // Safety: Wasmi translation guarantees that a register-list finalizer exists. + unsafe { + unreachable_unchecked!( + "unexpected register-list finalizer but found: {unexpected:?}" + ) + } + } }; copy_results(values); } diff --git a/crates/wasmi/src/engine/executor/instrs/select.rs b/crates/wasmi/src/engine/executor/instrs/select.rs index 800b463550..41d5249439 100644 --- a/crates/wasmi/src/engine/executor/instrs/select.rs +++ b/crates/wasmi/src/engine/executor/instrs/select.rs @@ -1,7 +1,10 @@ use super::{Executor, InstructionPtr}; use crate::{ core::UntypedVal, - engine::bytecode::{AnyConst32, Const32, Instruction, Reg}, + engine::{ + bytecode::{AnyConst32, Const32, Instruction, Reg}, + utils::unreachable_unchecked, + }, }; impl<'engine> Executor<'engine> { @@ -12,7 +15,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::Register2 { regs: [reg0, reg1] } => (reg0, reg1), unexpected => { - unreachable!("expected `Instruction::Register2` but found {unexpected:?}") + // Safety: Wasmi translation guarantees that [`Instruction::Register2`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::Register2` but found {unexpected:?}" + ) + } } } } @@ -27,7 +35,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::RegisterAndImm32 { reg, imm } => (reg, T::from(imm)), unexpected => { - unreachable!("expected `Instruction::RegisterAndImm32` but found {unexpected:?}") + // Safety: Wasmi translation guarantees that [`Instruction::RegisterAndImm32`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::RegisterAndImm32` but found {unexpected:?}" + ) + } } } } diff --git a/crates/wasmi/src/engine/executor/instrs/store.rs b/crates/wasmi/src/engine/executor/instrs/store.rs index 51cec05ae9..87d0743ba6 100644 --- a/crates/wasmi/src/engine/executor/instrs/store.rs +++ b/crates/wasmi/src/engine/executor/instrs/store.rs @@ -1,7 +1,10 @@ use super::{Executor, InstructionPtr}; use crate::{ core::{TrapCode, UntypedVal}, - engine::bytecode::{Const16, Instruction, Reg}, + engine::{ + bytecode::{Const16, Instruction, Reg}, + utils::unreachable_unchecked, + }, ir::{index::Memory, AnyConst16}, store::StoreInner, Error, @@ -22,8 +25,13 @@ impl<'engine> Executor<'engine> { addr.add(1); match *addr.get() { Instruction::RegisterAndImm32 { reg, imm } => (reg, u32::from(imm)), - instr => { - unreachable!("expected an `Instruction::RegisterAndImm32` but found: {instr:?}") + unexpected => { + // Safety: Wasmi translation guarantees that [`Instruction::RegisterAndImm32`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::RegisterAndImm32` but found {unexpected:?}" + ) + } } } } @@ -37,8 +45,13 @@ impl<'engine> Executor<'engine> { addr.add(1); match *addr.get() { Instruction::Imm16AndImm32 { imm16, imm32 } => (T::from(imm16), u32::from(imm32)), - instr => { - unreachable!("expected an `Instruction::Imm16AndImm32` but found: {instr:?}") + unexpected => { + // Safety: Wasmi translation guarantees that [`Instruction::Imm16AndImm32`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::Imm16AndImm32` but found {unexpected:?}" + ) + } } } } diff --git a/crates/wasmi/src/engine/executor/instrs/table.rs b/crates/wasmi/src/engine/executor/instrs/table.rs index 506607535e..3d839d19ef 100644 --- a/crates/wasmi/src/engine/executor/instrs/table.rs +++ b/crates/wasmi/src/engine/executor/instrs/table.rs @@ -1,11 +1,14 @@ use super::{Executor, InstructionPtr}; use crate::{ core::TrapCode, - engine::bytecode::{ - index::{Elem, Table}, - Const16, - Instruction, - Reg, + engine::{ + bytecode::{ + index::{Elem, Table}, + Const16, + Instruction, + Reg, + }, + utils::unreachable_unchecked, }, error::EntityGrowError, store::{ResourceLimiterRef, StoreInner}, @@ -22,7 +25,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::TableIndex { index } => index, unexpected => { - unreachable!("expected `Instruction::TableIndex` but found: {unexpected:?}") + // Safety: Wasmi translation guarantees that [`Instruction::TableIndex`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::TableIndex` but found: {unexpected:?}" + ) + } } } } @@ -34,7 +42,12 @@ impl<'engine> Executor<'engine> { match *addr.get() { Instruction::ElemIndex { index } => index, unexpected => { - unreachable!("expected `Instruction::ElemIndex` but found: {unexpected:?}") + // Safety: Wasmi translation guarantees that [`Instruction::ElemIndex`] exists. + unsafe { + unreachable_unchecked!( + "expected `Instruction::ElemIndex` but found: {unexpected:?}" + ) + } } } } diff --git a/crates/wasmi/src/engine/utils.rs b/crates/wasmi/src/engine/utils.rs index 55cc7a874e..4fc042e8ab 100644 --- a/crates/wasmi/src/engine/utils.rs +++ b/crates/wasmi/src/engine/utils.rs @@ -4,7 +4,7 @@ /// - [`core::hint::unreachable_unchecked`], otherwise. macro_rules! unreachable_unchecked { ($($arg:tt)*) => {{ - match cfg!(debug_assertions) { + match cfg!(debug_assertions) || cfg!(feature = "extra-checks") { true => ::core::unreachable!( $($arg)* ), false => ::core::hint::unreachable_unchecked(), }