Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix minor findings #1207

Merged
merged 8 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions crates/ir/src/for_each_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
///
Expand All @@ -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
///
Expand All @@ -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,
Expand All @@ -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
///
Expand All @@ -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
///
Expand All @@ -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,
Expand All @@ -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
///
Expand All @@ -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
///
Expand All @@ -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
///
Expand Down Expand Up @@ -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,
Expand All @@ -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
///
Expand All @@ -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
///
Expand All @@ -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,
Expand All @@ -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
///
Expand All @@ -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
///
Expand All @@ -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,
Expand All @@ -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
///
Expand All @@ -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
///
Expand All @@ -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
///
Expand Down
4 changes: 0 additions & 4 deletions crates/ir/src/primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
20 changes: 17 additions & 3 deletions crates/wasmi/src/engine/code_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
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,
Expand Down Expand Up @@ -306,7 +309,9 @@
// 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))
Expand Down Expand Up @@ -739,7 +744,7 @@
/// # 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>,
Expand All @@ -751,6 +756,15 @@
!instrs.is_empty(),
"compiled functions must have at least one instruction"
);
assert!(

Check warning on line 759 in crates/wasmi/src/engine/code_map.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/code_map.rs#L759

Added line #L759 was not covered by tests
// 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(),

Check warning on line 766 in crates/wasmi/src/engine/code_map.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/code_map.rs#L766

Added line #L766 was not covered by tests
);
Self {
instrs,
consts,
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/executor/instrs/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down
1 change: 1 addition & 0 deletions crates/wasmi/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod limits;
mod resumable;
mod traits;
mod translator;
mod utils;

#[cfg(test)]
mod tests;
Expand Down
5 changes: 2 additions & 3 deletions crates/wasmi/src/engine/translator/stack/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 0 additions & 28 deletions crates/wasmi/src/engine/translator/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use crate::{
TranslationError,
},
Error,
FuncType,
};
use std::vec::Vec;

Expand Down Expand Up @@ -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`]
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/translator/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
13 changes: 13 additions & 0 deletions crates/wasmi/src/engine/utils.rs
Original file line number Diff line number Diff line change
@@ -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)* ),

Check warning on line 8 in crates/wasmi/src/engine/utils.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/utils.rs#L8

Added line #L8 was not covered by tests
false => ::core::hint::unreachable_unchecked(),
}
}};
}
pub(crate) use unreachable_unchecked;
Loading