Fix bug in multi-value br_table translation (#1972)

* fix outdated docs

* add skip parameter to copy_operands_to_temp

* copy operands to temp for multi-value br_table

before this, operands were copied to the default target's branch params. However, this could lead to incorrect overwrites if the default target had a lower stack height than other targets.

* update Wasmi .wast testsuite

Author: Robbepop

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

@@ -730,13 +730,18 @@ impl FuncTranslator {
     fn copy_operands_to_temp(
         &mut self,
         len: u16,
+        skip: u16,
         fuel_pos: Option<Pos<ir::BlockFuel>>,
     ) -> Result<BoundedSlotSpan, Error> {
         if len == 0 {
             return Ok(BoundedSlotSpan::new(self.stack.next_temp_slots(), 0));
         }
-        let dst = self.stack.peek(usize::from(len) - 1).temp_slots().span();
-        self.copy_operands_to_dst(dst, len, 0, fuel_pos)
+        let dst = self
+            .stack
+            .peek(usize::from(len + skip) - 1)
+            .temp_slots()
+            .span();
+        self.copy_operands_to_dst(dst, len, skip, fuel_pos)
     }
 
     /// Skips the top `skip` operands and copies the remaining top `len` operands to `dst`.
@@ -1087,11 +1092,10 @@ impl FuncTranslator {
         &mut self,
         table: wasmparser::BrTable,
         index: Location,
-        default_branch_params: BranchParams,
+        values: BoundedSlotSpan,
     ) -> Result<(), Error> {
         let len_targets = table.len() + 1;
         debug_assert_eq!(self.immediates.len(), len_targets as usize);
-        let values = default_branch_params.temp_slots();
         let op = match index {
             Location::Reg(_) => Op::branch_table_span_r(len_targets, values),
             Location::Slot(index) => Op::branch_table_span_s(len_targets, index, values),
@@ -1615,8 +1619,7 @@ impl FuncTranslator {
     fn adjust_stack_for_call(&mut self, ty: &FuncType) -> Result<BoundedSlotSpan, Error> {
         let fuel_pos = self.stack.fuel_pos();
         self.preserve_regs(fuel_pos)?;
-        let fuel_pos = self.stack.fuel_pos();
-        let params = self.copy_operands_to_temp(ty.len_params(), fuel_pos)?;
+        let params = self.copy_operands_to_temp(ty.len_params(), 0, fuel_pos)?;
         for _ in 0..ty.len_params() {
             self.stack.pop();
         }

crates/wasmi/src/engine/translator/func/stack/operands.rs

@@ -616,8 +616,7 @@ impl OperandStack {
     ///
     /// # Note
     ///
-    /// - Returns the [`Operand`] at `depth` before being converted to an [`Operand::Temp`].
-    /// - [`Operand::Temp`] will have their optional `instr` set to `None`.
+    /// Returns the [`Operand`] at `depth` before being converted to an [`Operand::Temp`].
     ///
     /// # Panics
     ///

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

@@ -378,12 +378,15 @@ impl<'a> VisitOperator<'a> for FuncTranslator {
                 }
             }
         };
-        self.copy_branch_params(default_branch_params, fuel_pos)?;
-        let required_temp_copies = default_branch_params.len_temps() != 0;
-        let requires_branch_copies = required_temp_copies && equal_heights;
+        let len_temps = default_branch_params.len_temps();
+        let len_regs = default_branch_params.len_regs();
+        let values = self.copy_operands_to_temp(len_temps, len_regs, fuel_pos)?;
+        self.copy_branch_params_regs(default_branch_params, fuel_pos)?;
+        let required_temp_copies = len_temps != 0;
+        let requires_branch_copies = required_temp_copies && !equal_heights;
         match requires_branch_copies {
             false => self.encode_br_table_0(table, index_loc)?,
-            true => self.encode_br_table_n(table, index_loc, default_branch_params)?,
+            true => self.encode_br_table_n(table, index_loc, values)?,
         };
         self.reachable = false;
         Ok(())