Fix minor findings (#1207)

* correctly spell [un]signed

* fix last_index comment

We maybe also want to rename the method.

* remove unused method

* re-order statments to match call and call_indirect

* fix comment stating `store` instead of `load`

* re-instate panic for CompiledFuncEntity::new

Also added comment to state why this check exists.

* remove invalid comment about panics

This function might return a BranchOffset of 0 for certain inputs.

* get_compiled: conditionally panic if debug_assertions are enabled

crates/ir/src/for_each_op.rs

@@ -3726,7 +3726,7 @@ macro_rules! for_each_op {
                 rhs: Const16<i32>,
             },
 
-            /// `i32` singed-division instruction: `r0 = r1 / r2`
+            /// `i32` signed-division instruction: `r0 = r1 / r2`
             #[snake_name(i32_div_s)]
             I32DivS {
                 @result: Reg,
@@ -3735,7 +3735,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i32` singed-division immediate instruction: `r0 = r1 / c0`
+            /// `i32` signed-division immediate instruction: `r0 = r1 / c0`
             ///
             /// # Note
             ///
@@ -3749,7 +3749,7 @@ macro_rules! for_each_op {
                 /// The 16-bit immediate value.
                 rhs: Const16<NonZeroI32>,
             },
-            /// `i32` singed-division immediate instruction: `r0 = c0 / r1`
+            /// `i32` signed-division immediate instruction: `r0 = c0 / r1`
             ///
             /// # Note
             ///
@@ -3765,7 +3765,7 @@ macro_rules! for_each_op {
                 rhs: Reg,
             },
 
-            /// `i32` unsinged-division instruction: `r0 = r1 / r2`
+            /// `i32` unsigned-division instruction: `r0 = r1 / r2`
             #[snake_name(i32_div_u)]
             I32DivU {
                 @result: Reg,
@@ -3774,7 +3774,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i32` unsinged-division immediate instruction: `r0 = r1 / c0`
+            /// `i32` unsigned-division immediate instruction: `r0 = r1 / c0`
             ///
             /// # Note
             ///
@@ -3791,7 +3791,7 @@ macro_rules! for_each_op {
                 /// The 16-bit immediate value.
                 rhs: Const16<NonZeroU32>,
             },
-            /// `i32` unsinged-division immediate instruction: `r0 = c0 / r1`
+            /// `i32` unsigned-division immediate instruction: `r0 = c0 / r1`
             ///
             /// # Note
             ///
@@ -3807,7 +3807,7 @@ macro_rules! for_each_op {
                 rhs: Reg,
             },
 
-            /// `i32` singed-remainder instruction: `r0 = r1 % r2`
+            /// `i32` signed-remainder instruction: `r0 = r1 % r2`
             #[snake_name(i32_rem_s)]
             I32RemS {
                 @result: Reg,
@@ -3816,7 +3816,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i32` singed-remainder immediate instruction: `r0 = r1 % c0`
+            /// `i32` signed-remainder immediate instruction: `r0 = r1 % c0`
             ///
             /// # Note
             ///
@@ -3830,7 +3830,7 @@ macro_rules! for_each_op {
                 /// The 16-bit immediate value.
                 rhs: Const16<NonZeroI32>,
             },
-            /// `i32` singed-remainder immediate instruction: `r0 = c0 % r1`
+            /// `i32` signed-remainder immediate instruction: `r0 = c0 % r1`
             ///
             /// # Note
             ///
@@ -3855,7 +3855,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i32` singed-remainder immediate instruction: `r0 = r1 % c0`
+            /// `i32` signed-remainder immediate instruction: `r0 = r1 % c0`
             ///
             /// # Note
             ///
@@ -4240,7 +4240,7 @@ macro_rules! for_each_op {
                 rhs: Const16<i64>,
             },
 
-            /// `i64` singed-division instruction: `r0 = r1 / r2`
+            /// `i64` signed-division instruction: `r0 = r1 / r2`
             #[snake_name(i64_div_s)]
             I64DivS {
                 @result: Reg,
@@ -4249,7 +4249,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i64` singed-division immediate instruction: `r0 = r1 / c0`
+            /// `i64` signed-division immediate instruction: `r0 = r1 / c0`
             ///
             /// # Note
             ///
@@ -4263,7 +4263,7 @@ macro_rules! for_each_op {
                 /// The 16-bit immediate value.
                 rhs: Const16<NonZeroI64>,
             },
-            /// `i32` singed-division immediate instruction: `r0 = c0 / r1`
+            /// `i32` signed-division immediate instruction: `r0 = c0 / r1`
             ///
             /// # Note
             ///
@@ -4279,7 +4279,7 @@ macro_rules! for_each_op {
                 rhs: Reg,
             },
 
-            /// `i64` unsinged-division instruction: `r0 = r1 / r2`
+            /// `i64` unsigned-division instruction: `r0 = r1 / r2`
             #[snake_name(i64_div_u)]
             I64DivU {
                 @result: Reg,
@@ -4288,7 +4288,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i64` unsinged-division immediate instruction: `r0 = r1 / c0`
+            /// `i64` unsigned-division immediate instruction: `r0 = r1 / c0`
             ///
             /// # Note
             ///
@@ -4305,7 +4305,7 @@ macro_rules! for_each_op {
                 /// The 16-bit immediate value.
                 rhs: Const16<NonZeroU64>,
             },
-            /// `i64` unsinged-division immediate instruction: `r0 = c0 / r1`
+            /// `i64` unsigned-division immediate instruction: `r0 = c0 / r1`
             ///
             /// # Note
             ///
@@ -4321,7 +4321,7 @@ macro_rules! for_each_op {
                 rhs: Reg,
             },
 
-            /// `i64` singed-remainder instruction: `r0 = r1 % r2`
+            /// `i64` signed-remainder instruction: `r0 = r1 % r2`
             #[snake_name(i64_rem_s)]
             I64RemS {
                 @result: Reg,
@@ -4330,7 +4330,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i64` singed-remainder immediate instruction: `r0 = r1 % c0`
+            /// `i64` signed-remainder immediate instruction: `r0 = r1 % c0`
             ///
             /// # Note
             ///
@@ -4344,7 +4344,7 @@ macro_rules! for_each_op {
                 /// The 16-bit immediate value.
                 rhs: Const16<NonZeroI64>,
             },
-            /// `i64` singed-remainder immediate instruction: `r0 = c0 % r1`
+            /// `i64` signed-remainder immediate instruction: `r0 = c0 % r1`
             ///
             /// # Note
             ///
@@ -4369,7 +4369,7 @@ macro_rules! for_each_op {
                 /// The register holding the right-hand side value.
                 rhs: Reg,
             },
-            /// `i64` singed-remainder immediate instruction: `r0 = r1 % c0`
+            /// `i64` signed-remainder immediate instruction: `r0 = r1 % c0`
             ///
             /// # Note
             ///

crates/ir/src/primitive.rs

@@ -152,10 +152,6 @@ impl BranchOffset {
     /// # Errors
     ///
     /// If the resulting [`BranchOffset`] is out of bounds.
-    ///
-    /// # Panics
-    ///
-    /// If the resulting [`BranchOffset`] is uninitialized, aka equal to 0.
     pub fn from_src_to_dst(src: Instr, dst: Instr) -> Result<Self, Error> {
         let src = i64::from(u32::from(src));
         let dst = i64::from(u32::from(dst));

crates/wasmi/src/engine/code_map.rs

@@ -15,7 +15,10 @@ use super::{
 use crate::{
     collections::arena::{Arena, ArenaIndex},
     core::{TrapCode, UntypedVal},
-    engine::bytecode::{index::InternalFunc, Instruction},
+    engine::{
+        bytecode::{index::InternalFunc, Instruction},
+        utils::unreachable_unchecked,
+    },
     module::{FuncIdx, ModuleHeader},
     store::{Fuel, FuelError},
     Config,
@@ -306,7 +309,9 @@ impl CodeMap {
             // Safety: this is just called internally with function indices
             //         that are known to be valid. Since this is a performance
             //         critical path we need to leave out this check.
-            unsafe { core::hint::unreachable_unchecked() }
+            unsafe {
+                unreachable_unchecked!("encountered invalid function index for engine: {func:?}")
+            }
         };
         let cref = entity.get_compiled()?;
         Some(self.adjust_cref_lifetime(cref))
@@ -739,7 +744,7 @@ impl CompiledFuncEntity {
     /// # Panics
     ///
     /// - If `instrs` is empty.
-    /// - If `instrs` contains more than `u32::MAX` instructions.
+    /// - If `instrs` contains more than `i32::MAX` instructions.
     pub fn new<I, C>(len_registers: u16, instrs: I, consts: C) -> Self
     where
         I: IntoIterator<Item = Instruction>,
@@ -751,6 +756,15 @@ impl CompiledFuncEntity {
             !instrs.is_empty(),
             "compiled functions must have at least one instruction"
         );
+        assert!(
+            // Generally, Wasmi has no issues with more than `i32::MAX` instructions.
+            // However, Wasmi's branch instructions can jump across at most `i32::MAX`
+            // forwards or `i32::MIN` instructions backwards and thus having more than
+            // `i32::MAX` instructions might introduce problems.
+            instrs.len() <= i32::MAX as usize,
+            "compiled function has too many instructions: {}",
+            instrs.len(),
+        );
         Self {
             instrs,
             consts,

crates/wasmi/src/engine/executor/instrs/load.rs

@@ -70,7 +70,7 @@ impl<'engine> Executor<'engine> {
         store.resolve_memory(&memory).data()
     }
 
-    /// Executes a generic Wasm `store[N_{s|u}]` operation.
+    /// Executes a generic Wasm `load[N_{s|u}]` operation.
     ///
     /// # Note
     ///

crates/wasmi/src/engine/mod.rs

@@ -10,6 +10,7 @@ mod limits;
 mod resumable;
 mod traits;
 mod translator;
+mod utils;
 
 #[cfg(test)]
 mod tests;

crates/wasmi/src/engine/translator/stack/consts.rs

@@ -46,9 +46,8 @@ impl FuncLocalConsts {
     ///
     /// # Note
     ///
-    /// The minimum index is the last index to be assignable to a function local
-    /// constant value. Once it has been assigned no more function local constant
-    /// values can be assigned anymore.
+    /// This index is not assignable to a function local constant value and acts
+    /// as a bound to guard against overflowing the range of indices.
     fn last_index() -> i16 {
         i16::MIN
     }

crates/wasmi/src/engine/translator/stack/mod.rs

@@ -18,7 +18,6 @@ use crate::{
         TranslationError,
     },
     Error,
-    FuncType,
 };
 use std::vec::Vec;
 
@@ -83,33 +82,6 @@ impl ValueStack {
         }
     }
 
-    /// Adjusts the [`ValueStack`] given the [`FuncType`] of the call.
-    ///
-    /// - Returns the [`RegSpan`] for the `call` results.
-    /// - The `provider_buffer` will hold all [`Provider`] call parameters.
-    /// - The `params_buffer` will hold all call parameters converted to [`Reg`]. \
-    ///   Any constant value parameter will be allocated as function local constant.
-    ///
-    /// # Note
-    ///
-    /// Both `provider_buffer` and `params_buffer` will be cleared before this operation.
-    ///
-    /// # Errors
-    ///
-    /// - If not enough call parameters are on the [`ValueStack`].
-    /// - If too many function local constants are being registered as call parameters.
-    /// - If too many registers are registered as call results.
-    pub fn adjust_for_call(
-        &mut self,
-        func_type: &FuncType,
-        provider_buffer: &mut Vec<TypedProvider>,
-    ) -> Result<RegSpan, Error> {
-        let (params, results) = func_type.params_results();
-        self.pop_n(params.len(), provider_buffer);
-        let results = self.push_dynamic_n(results.len())?;
-        Ok(results)
-    }
-
     /// Preserves `local.get` on the [`ProviderStack`] by shifting to the preservation space.
     ///
     /// In case there are `local.get n` with `n == preserve_index` on the [`ProviderStack`]

crates/wasmi/src/engine/translator/visit.rs

@@ -530,9 +530,9 @@ impl<'a> VisitOperator<'a> for FuncTranslator {
         self.bump_fuel_consumption(FuelCosts::call)?;
         let type_index = FuncType::from(type_index);
         let func_type = self.func_type_at(type_index);
-        let (params, results) = func_type.params_results();
         let index = self.alloc.stack.pop();
         let indirect_params = self.call_indirect_params(index, table_index)?;
+        let (params, results) = func_type.params_results();
         let provider_params = &mut self.alloc.buffer.providers;
         self.alloc.stack.pop_n(params.len(), provider_params);
         let results = self.alloc.stack.push_dynamic_n(results.len())?;

crates/wasmi/src/engine/utils.rs

@@ -0,0 +1,13 @@
+/// Expands to
+///
+/// - [`core::unreachable`] if `debug_assertions` are enabled.
+/// - [`core::hint::unreachable_unchecked`], otherwise.
+macro_rules! unreachable_unchecked {
+    ($($arg:tt)*) => {{
+        match cfg!(debug_assertions) {
+            true => ::core::unreachable!( $($arg)* ),
+            false => ::core::hint::unreachable_unchecked(),
+        }
+    }};
+}
+pub(crate) use unreachable_unchecked;