From 3dc8c237564a301fe803881f1b255f6ae80b64db Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Thu, 10 Jul 2025 19:57:35 +0200 Subject: [PATCH 1/3] put UpdateBranchOffset trait + impl into comparator.rs --- .../wasmi/src/engine/translator/comparator.rs | 111 +++++++++++++++++ .../engine/translator/func/instr_encoder.rs | 113 +----------------- 2 files changed, 112 insertions(+), 112 deletions(-) diff --git a/crates/wasmi/src/engine/translator/comparator.rs b/crates/wasmi/src/engine/translator/comparator.rs index 05729d213c..5d8fb32fe9 100644 --- a/crates/wasmi/src/engine/translator/comparator.rs +++ b/crates/wasmi/src/engine/translator/comparator.rs @@ -784,3 +784,114 @@ fn try_into_cmp_br_comparator(instr: &Instruction) -> Option { }; Some(comparator) } + +/// Extension trait to update the branch offset of an [`Instruction`]. +pub trait UpdateBranchOffset { + /// Updates the [`BranchOffset`] for the branch [`Instruction]. + /// + /// # Panics + /// + /// If `self` is not a branch [`Instruction`]. + fn update_branch_offset( + &mut self, + stack: &mut impl AllocConst, + new_offset: BranchOffset, + ) -> Result<(), Error>; +} + +impl UpdateBranchOffset for Instruction { + #[rustfmt::skip] + fn update_branch_offset(&mut self, stack: &mut impl AllocConst, new_offset: BranchOffset) -> Result<(), Error> { + use Instruction as I; + match self { + I::Branch { offset } | + I::BranchTableTarget { offset, .. } | + I::BranchTableTargetNonOverlapping { offset, .. } => { + offset.init(new_offset); + return Ok(()) + } + _ => {} + }; + let update_status = match self { + I::BranchI32And { offset, .. } | + I::BranchI32Or { offset, .. } | + I::BranchI32Xor { offset, .. } | + I::BranchI32Nand { offset, .. } | + I::BranchI32Nor { offset, .. } | + I::BranchI32Xnor { offset, .. } | + I::BranchI32Eq { offset, .. } | + I::BranchI32Ne { offset, .. } | + I::BranchI32LtS { offset, .. } | + I::BranchI32LtU { offset, .. } | + I::BranchI32LeS { offset, .. } | + I::BranchI32LeU { offset, .. } | + I::BranchI64And { offset, .. } | + I::BranchI64Or { offset, .. } | + I::BranchI64Xor { offset, .. } | + I::BranchI64Nand { offset, .. } | + I::BranchI64Nor { offset, .. } | + I::BranchI64Xnor { offset, .. } | + I::BranchI64Eq { offset, .. } | + I::BranchI64Ne { offset, .. } | + I::BranchI64LtS { offset, .. } | + I::BranchI64LtU { offset, .. } | + I::BranchI64LeS { offset, .. } | + I::BranchI64LeU { offset, .. } | + I::BranchF32Eq { offset, .. } | + I::BranchF32Ne { offset, .. } | + I::BranchF32Lt { offset, .. } | + I::BranchF32Le { offset, .. } | + I::BranchF32NotLt { offset, .. } | + I::BranchF32NotLe { offset, .. } | + I::BranchF64Eq { offset, .. } | + I::BranchF64Ne { offset, .. } | + I::BranchF64Lt { offset, .. } | + I::BranchF64Le { offset, .. } | + I::BranchF64NotLt { offset, .. } | + I::BranchF64NotLe { offset, .. } | + I::BranchI32AndImm16 { offset, .. } | + I::BranchI32OrImm16 { offset, .. } | + I::BranchI32XorImm16 { offset, .. } | + I::BranchI32NandImm16 { offset, .. } | + I::BranchI32NorImm16 { offset, .. } | + I::BranchI32XnorImm16 { offset, .. } | + I::BranchI32EqImm16 { offset, .. } | + I::BranchI32NeImm16 { offset, .. } | + I::BranchI32LtSImm16Lhs { offset, .. } | + I::BranchI32LtSImm16Rhs { offset, .. } | + I::BranchI32LeSImm16Lhs { offset, .. } | + I::BranchI32LeSImm16Rhs { offset, .. } | + I::BranchI32LtUImm16Lhs { offset, .. } | + I::BranchI32LtUImm16Rhs { offset, .. } | + I::BranchI32LeUImm16Lhs { offset, .. } | + I::BranchI32LeUImm16Rhs { offset, .. } | + I::BranchI64AndImm16 { offset, .. } | + I::BranchI64OrImm16 { offset, .. } | + I::BranchI64XorImm16 { offset, .. } | + I::BranchI64NandImm16 { offset, .. } | + I::BranchI64NorImm16 { offset, .. } | + I::BranchI64XnorImm16 { offset, .. } | + I::BranchI64EqImm16 { offset, .. } | + I::BranchI64NeImm16 { offset, .. } | + I::BranchI64LtSImm16Lhs { offset, .. } | + I::BranchI64LtSImm16Rhs { offset, .. } | + I::BranchI64LeSImm16Lhs { offset, .. } | + I::BranchI64LeSImm16Rhs { offset, .. } | + I::BranchI64LtUImm16Lhs { offset, .. } | + I::BranchI64LtUImm16Rhs { offset, .. } | + I::BranchI64LeUImm16Lhs { offset, .. } | + I::BranchI64LeUImm16Rhs { offset, .. } => { + offset.init(new_offset) + } + unexpected => { + panic!("expected a Wasmi branch+cmp instruction but found: {unexpected:?}") + } + }; + if update_status.is_err() { + if let Some(fallback) = self.try_into_cmp_branch_fallback_instr(new_offset, stack)? { + *self = fallback; + } + } + Ok(()) + } +} diff --git a/crates/wasmi/src/engine/translator/func/instr_encoder.rs b/crates/wasmi/src/engine/translator/func/instr_encoder.rs index dfc6f5f382..5e2ce615d4 100644 --- a/crates/wasmi/src/engine/translator/func/instr_encoder.rs +++ b/crates/wasmi/src/engine/translator/func/instr_encoder.rs @@ -6,9 +6,9 @@ use crate::{ CompareResult, LogicalizeCmpInstr, NegateCmpInstr, - TryIntoCmpBranchFallbackInstr, TryIntoCmpBranchInstr, TryIntoCmpSelectInstr, + UpdateBranchOffset as _, }, func::{ stack::RegisterSpace, @@ -1103,117 +1103,6 @@ impl InstrEncoder { } } -/// Extension trait to update the branch offset of an [`Instruction`]. -trait UpdateBranchOffset { - /// Updates the [`BranchOffset`] for the branch [`Instruction]. - /// - /// # Panics - /// - /// If `self` is not a branch [`Instruction`]. - fn update_branch_offset( - &mut self, - stack: &mut ValueStack, - new_offset: BranchOffset, - ) -> Result<(), Error>; -} - -impl UpdateBranchOffset for Instruction { - #[rustfmt::skip] - fn update_branch_offset(&mut self, stack: &mut ValueStack, new_offset: BranchOffset) -> Result<(), Error> { - use Instruction as I; - match self { - Instruction::Branch { offset } | - Instruction::BranchTableTarget { offset, .. } | - Instruction::BranchTableTargetNonOverlapping { offset, .. } => { - offset.init(new_offset); - return Ok(()) - } - _ => {} - }; - let update_status = match self { - I::BranchI32And { offset, .. } | - I::BranchI32Or { offset, .. } | - I::BranchI32Xor { offset, .. } | - I::BranchI32Nand { offset, .. } | - I::BranchI32Nor { offset, .. } | - I::BranchI32Xnor { offset, .. } | - I::BranchI32Eq { offset, .. } | - I::BranchI32Ne { offset, .. } | - I::BranchI32LtS { offset, .. } | - I::BranchI32LtU { offset, .. } | - I::BranchI32LeS { offset, .. } | - I::BranchI32LeU { offset, .. } | - I::BranchI64And { offset, .. } | - I::BranchI64Or { offset, .. } | - I::BranchI64Xor { offset, .. } | - I::BranchI64Nand { offset, .. } | - I::BranchI64Nor { offset, .. } | - I::BranchI64Xnor { offset, .. } | - I::BranchI64Eq { offset, .. } | - I::BranchI64Ne { offset, .. } | - I::BranchI64LtS { offset, .. } | - I::BranchI64LtU { offset, .. } | - I::BranchI64LeS { offset, .. } | - I::BranchI64LeU { offset, .. } | - I::BranchF32Eq { offset, .. } | - I::BranchF32Ne { offset, .. } | - I::BranchF32Lt { offset, .. } | - I::BranchF32Le { offset, .. } | - I::BranchF32NotLt { offset, .. } | - I::BranchF32NotLe { offset, .. } | - I::BranchF64Eq { offset, .. } | - I::BranchF64Ne { offset, .. } | - I::BranchF64Lt { offset, .. } | - I::BranchF64Le { offset, .. } | - I::BranchF64NotLt { offset, .. } | - I::BranchF64NotLe { offset, .. } | - I::BranchI32AndImm16 { offset, .. } | - I::BranchI32OrImm16 { offset, .. } | - I::BranchI32XorImm16 { offset, .. } | - I::BranchI32NandImm16 { offset, .. } | - I::BranchI32NorImm16 { offset, .. } | - I::BranchI32XnorImm16 { offset, .. } | - I::BranchI32EqImm16 { offset, .. } | - I::BranchI32NeImm16 { offset, .. } | - I::BranchI32LtSImm16Lhs { offset, .. } | - I::BranchI32LtSImm16Rhs { offset, .. } | - I::BranchI32LeSImm16Lhs { offset, .. } | - I::BranchI32LeSImm16Rhs { offset, .. } | - I::BranchI32LtUImm16Lhs { offset, .. } | - I::BranchI32LtUImm16Rhs { offset, .. } | - I::BranchI32LeUImm16Lhs { offset, .. } | - I::BranchI32LeUImm16Rhs { offset, .. } | - I::BranchI64AndImm16 { offset, .. } | - I::BranchI64OrImm16 { offset, .. } | - I::BranchI64XorImm16 { offset, .. } | - I::BranchI64NandImm16 { offset, .. } | - I::BranchI64NorImm16 { offset, .. } | - I::BranchI64XnorImm16 { offset, .. } | - I::BranchI64EqImm16 { offset, .. } | - I::BranchI64NeImm16 { offset, .. } | - I::BranchI64LtSImm16Lhs { offset, .. } | - I::BranchI64LtSImm16Rhs { offset, .. } | - I::BranchI64LeSImm16Lhs { offset, .. } | - I::BranchI64LeSImm16Rhs { offset, .. } | - I::BranchI64LtUImm16Lhs { offset, .. } | - I::BranchI64LtUImm16Rhs { offset, .. } | - I::BranchI64LeUImm16Lhs { offset, .. } | - I::BranchI64LeUImm16Rhs { offset, .. } => { - offset.init(new_offset) - } - unexpected => { - panic!("expected a Wasmi branch+cmp instruction but found: {unexpected:?}") - } - }; - if update_status.is_err() { - if let Some(fallback) = self.try_into_cmp_branch_fallback_instr(new_offset, stack)? { - *self = fallback; - } - } - Ok(()) - } -} - #[cfg(test)] mod tests { use super::*; From 3320e0893e4cabb086ace158f68db2edc66abf45 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Thu, 10 Jul 2025 20:04:10 +0200 Subject: [PATCH 2/3] refactor Instruction::update_branch_offset trait method impl --- .../wasmi/src/engine/translator/comparator.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/wasmi/src/engine/translator/comparator.rs b/crates/wasmi/src/engine/translator/comparator.rs index 5d8fb32fe9..a08f5c8fbd 100644 --- a/crates/wasmi/src/engine/translator/comparator.rs +++ b/crates/wasmi/src/engine/translator/comparator.rs @@ -812,7 +812,7 @@ impl UpdateBranchOffset for Instruction { } _ => {} }; - let update_status = match self { + let offset = match self { I::BranchI32And { offset, .. } | I::BranchI32Or { offset, .. } | I::BranchI32Xor { offset, .. } | @@ -881,16 +881,19 @@ impl UpdateBranchOffset for Instruction { I::BranchI64LtUImm16Rhs { offset, .. } | I::BranchI64LeUImm16Lhs { offset, .. } | I::BranchI64LeUImm16Rhs { offset, .. } => { - offset.init(new_offset) + offset } unexpected => { - panic!("expected a Wasmi branch+cmp instruction but found: {unexpected:?}") + panic!("expected a Wasmi `cmp`+`branch` instruction but found: {unexpected:?}") } }; - if update_status.is_err() { - if let Some(fallback) = self.try_into_cmp_branch_fallback_instr(new_offset, stack)? { - *self = fallback; - } + if offset.init(new_offset).is_err() { + // Case: we need to covert `self` into its cmp+branch fallback instruction variant + // since adjusting the 16-bit offset failed. + let Some(fallback) = self.try_into_cmp_branch_fallback_instr(new_offset, stack)? else { + unreachable!("failed to create cmp+branch fallback instruction for: {self:?}"); + }; + *self = fallback; } Ok(()) } From 5c19cff6b5a94840db211f8bfe81d4eb00c18304 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Thu, 10 Jul 2025 20:06:22 +0200 Subject: [PATCH 3/3] change formatting --- .../wasmi/src/engine/translator/comparator.rs | 152 +++++++++--------- 1 file changed, 77 insertions(+), 75 deletions(-) diff --git a/crates/wasmi/src/engine/translator/comparator.rs b/crates/wasmi/src/engine/translator/comparator.rs index a08f5c8fbd..50aac958f8 100644 --- a/crates/wasmi/src/engine/translator/comparator.rs +++ b/crates/wasmi/src/engine/translator/comparator.rs @@ -801,88 +801,90 @@ pub trait UpdateBranchOffset { impl UpdateBranchOffset for Instruction { #[rustfmt::skip] - fn update_branch_offset(&mut self, stack: &mut impl AllocConst, new_offset: BranchOffset) -> Result<(), Error> { + fn update_branch_offset( + &mut self, + stack: &mut impl AllocConst, + new_offset: BranchOffset, + ) -> Result<(), Error> { use Instruction as I; match self { - I::Branch { offset } | - I::BranchTableTarget { offset, .. } | - I::BranchTableTargetNonOverlapping { offset, .. } => { + | I::Branch { offset } + | I::BranchTableTarget { offset, .. } + | I::BranchTableTargetNonOverlapping { offset, .. } => { offset.init(new_offset); - return Ok(()) + return Ok(()); } _ => {} }; let offset = match self { - I::BranchI32And { offset, .. } | - I::BranchI32Or { offset, .. } | - I::BranchI32Xor { offset, .. } | - I::BranchI32Nand { offset, .. } | - I::BranchI32Nor { offset, .. } | - I::BranchI32Xnor { offset, .. } | - I::BranchI32Eq { offset, .. } | - I::BranchI32Ne { offset, .. } | - I::BranchI32LtS { offset, .. } | - I::BranchI32LtU { offset, .. } | - I::BranchI32LeS { offset, .. } | - I::BranchI32LeU { offset, .. } | - I::BranchI64And { offset, .. } | - I::BranchI64Or { offset, .. } | - I::BranchI64Xor { offset, .. } | - I::BranchI64Nand { offset, .. } | - I::BranchI64Nor { offset, .. } | - I::BranchI64Xnor { offset, .. } | - I::BranchI64Eq { offset, .. } | - I::BranchI64Ne { offset, .. } | - I::BranchI64LtS { offset, .. } | - I::BranchI64LtU { offset, .. } | - I::BranchI64LeS { offset, .. } | - I::BranchI64LeU { offset, .. } | - I::BranchF32Eq { offset, .. } | - I::BranchF32Ne { offset, .. } | - I::BranchF32Lt { offset, .. } | - I::BranchF32Le { offset, .. } | - I::BranchF32NotLt { offset, .. } | - I::BranchF32NotLe { offset, .. } | - I::BranchF64Eq { offset, .. } | - I::BranchF64Ne { offset, .. } | - I::BranchF64Lt { offset, .. } | - I::BranchF64Le { offset, .. } | - I::BranchF64NotLt { offset, .. } | - I::BranchF64NotLe { offset, .. } | - I::BranchI32AndImm16 { offset, .. } | - I::BranchI32OrImm16 { offset, .. } | - I::BranchI32XorImm16 { offset, .. } | - I::BranchI32NandImm16 { offset, .. } | - I::BranchI32NorImm16 { offset, .. } | - I::BranchI32XnorImm16 { offset, .. } | - I::BranchI32EqImm16 { offset, .. } | - I::BranchI32NeImm16 { offset, .. } | - I::BranchI32LtSImm16Lhs { offset, .. } | - I::BranchI32LtSImm16Rhs { offset, .. } | - I::BranchI32LeSImm16Lhs { offset, .. } | - I::BranchI32LeSImm16Rhs { offset, .. } | - I::BranchI32LtUImm16Lhs { offset, .. } | - I::BranchI32LtUImm16Rhs { offset, .. } | - I::BranchI32LeUImm16Lhs { offset, .. } | - I::BranchI32LeUImm16Rhs { offset, .. } | - I::BranchI64AndImm16 { offset, .. } | - I::BranchI64OrImm16 { offset, .. } | - I::BranchI64XorImm16 { offset, .. } | - I::BranchI64NandImm16 { offset, .. } | - I::BranchI64NorImm16 { offset, .. } | - I::BranchI64XnorImm16 { offset, .. } | - I::BranchI64EqImm16 { offset, .. } | - I::BranchI64NeImm16 { offset, .. } | - I::BranchI64LtSImm16Lhs { offset, .. } | - I::BranchI64LtSImm16Rhs { offset, .. } | - I::BranchI64LeSImm16Lhs { offset, .. } | - I::BranchI64LeSImm16Rhs { offset, .. } | - I::BranchI64LtUImm16Lhs { offset, .. } | - I::BranchI64LtUImm16Rhs { offset, .. } | - I::BranchI64LeUImm16Lhs { offset, .. } | - I::BranchI64LeUImm16Rhs { offset, .. } => { - offset - } + | I::BranchI32And { offset, .. } + | I::BranchI32Or { offset, .. } + | I::BranchI32Xor { offset, .. } + | I::BranchI32Nand { offset, .. } + | I::BranchI32Nor { offset, .. } + | I::BranchI32Xnor { offset, .. } + | I::BranchI32Eq { offset, .. } + | I::BranchI32Ne { offset, .. } + | I::BranchI32LtS { offset, .. } + | I::BranchI32LtU { offset, .. } + | I::BranchI32LeS { offset, .. } + | I::BranchI32LeU { offset, .. } + | I::BranchI64And { offset, .. } + | I::BranchI64Or { offset, .. } + | I::BranchI64Xor { offset, .. } + | I::BranchI64Nand { offset, .. } + | I::BranchI64Nor { offset, .. } + | I::BranchI64Xnor { offset, .. } + | I::BranchI64Eq { offset, .. } + | I::BranchI64Ne { offset, .. } + | I::BranchI64LtS { offset, .. } + | I::BranchI64LtU { offset, .. } + | I::BranchI64LeS { offset, .. } + | I::BranchI64LeU { offset, .. } + | I::BranchF32Eq { offset, .. } + | I::BranchF32Ne { offset, .. } + | I::BranchF32Lt { offset, .. } + | I::BranchF32Le { offset, .. } + | I::BranchF32NotLt { offset, .. } + | I::BranchF32NotLe { offset, .. } + | I::BranchF64Eq { offset, .. } + | I::BranchF64Ne { offset, .. } + | I::BranchF64Lt { offset, .. } + | I::BranchF64Le { offset, .. } + | I::BranchF64NotLt { offset, .. } + | I::BranchF64NotLe { offset, .. } + | I::BranchI32AndImm16 { offset, .. } + | I::BranchI32OrImm16 { offset, .. } + | I::BranchI32XorImm16 { offset, .. } + | I::BranchI32NandImm16 { offset, .. } + | I::BranchI32NorImm16 { offset, .. } + | I::BranchI32XnorImm16 { offset, .. } + | I::BranchI32EqImm16 { offset, .. } + | I::BranchI32NeImm16 { offset, .. } + | I::BranchI32LtSImm16Lhs { offset, .. } + | I::BranchI32LtSImm16Rhs { offset, .. } + | I::BranchI32LeSImm16Lhs { offset, .. } + | I::BranchI32LeSImm16Rhs { offset, .. } + | I::BranchI32LtUImm16Lhs { offset, .. } + | I::BranchI32LtUImm16Rhs { offset, .. } + | I::BranchI32LeUImm16Lhs { offset, .. } + | I::BranchI32LeUImm16Rhs { offset, .. } + | I::BranchI64AndImm16 { offset, .. } + | I::BranchI64OrImm16 { offset, .. } + | I::BranchI64XorImm16 { offset, .. } + | I::BranchI64NandImm16 { offset, .. } + | I::BranchI64NorImm16 { offset, .. } + | I::BranchI64XnorImm16 { offset, .. } + | I::BranchI64EqImm16 { offset, .. } + | I::BranchI64NeImm16 { offset, .. } + | I::BranchI64LtSImm16Lhs { offset, .. } + | I::BranchI64LtSImm16Rhs { offset, .. } + | I::BranchI64LeSImm16Lhs { offset, .. } + | I::BranchI64LeSImm16Rhs { offset, .. } + | I::BranchI64LtUImm16Lhs { offset, .. } + | I::BranchI64LtUImm16Rhs { offset, .. } + | I::BranchI64LeUImm16Lhs { offset, .. } + | I::BranchI64LeUImm16Rhs { offset, .. } => offset, unexpected => { panic!("expected a Wasmi `cmp`+`branch` instruction but found: {unexpected:?}") }