diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index 3a4bea1294..18f7e72901 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -332,24 +332,7 @@ pub fn translate_operator( builder.seal_block(header) } - // The "If" frame pushes its parameters twice, so they're available to the else block - // (see also `FuncTranslationState::push_if`). - // Yet, the original_stack_size member accounts for them only once, so that the else - // block can see the same number of parameters as the consequent block. As a matter of - // fact, we need to substract an extra number of parameter values for if blocks. - let num_duplicated_params = if let ControlStackFrame::If { - num_param_values, .. - } = frame - { - debug_assert!(num_param_values <= frame.original_stack_size()); - num_param_values - } else { - 0 - }; - - state - .stack - .truncate(frame.original_stack_size() - num_duplicated_params); + frame.truncate_value_stack_to_original_size(&mut state.stack); state .stack .extend_from_slice(builder.block_params(next_block)); @@ -1908,9 +1891,8 @@ fn translate_unreachable_operator( let (params, _results) = blocktype_params_results(module_translation_state, blocktype)?; let else_block = block_with_params(builder, params, environ)?; - state.stack.truncate( - state.control_stack.last().unwrap().original_stack_size(), - ); + let frame = state.control_stack.last().unwrap(); + frame.truncate_value_stack_to_else_params(&mut state.stack); // We change the target of the branch instruction. builder.change_jump_destination(branch_inst, else_block); @@ -1918,9 +1900,8 @@ fn translate_unreachable_operator( else_block } ElseData::WithElse { else_block } => { - state.stack.truncate( - state.control_stack.last().unwrap().original_stack_size(), - ); + let frame = state.control_stack.last().unwrap(); + frame.truncate_value_stack_to_else_params(&mut state.stack); else_block } }; @@ -1941,9 +1922,8 @@ fn translate_unreachable_operator( let control_stack = &mut state.control_stack; let frame = control_stack.pop().unwrap(); - // Now we have to split off the stack the values not used - // by unreachable code that hasn't been translated - stack.truncate(frame.original_stack_size()); + // Pop unused parameters from stack. + frame.truncate_value_stack_to_original_size(stack); let reachable_anyway = match frame { // If it is a loop we also have to seal the body loop block diff --git a/cranelift/wasm/src/state/func_state.rs b/cranelift/wasm/src/state/func_state.rs index 24ef5ff0cb..2af264f59e 100644 --- a/cranelift/wasm/src/state/func_state.rs +++ b/cranelift/wasm/src/state/func_state.rs @@ -128,7 +128,9 @@ impl ControlStackFrame { Self::Loop { header, .. } => header, } } - pub fn original_stack_size(&self) -> usize { + /// Private helper. Use `truncate_value_stack_to_else_params()` or + /// `truncate_value_stack_to_original_size()` to restore value-stack state. + fn original_stack_size(&self) -> usize { match *self { Self::If { original_stack_size, @@ -178,6 +180,33 @@ impl ControlStackFrame { Self::Loop { .. } => {} } } + + /// Pop values from the value stack so that it is left at the + /// input-parameters to an else-block. + pub fn truncate_value_stack_to_else_params(&self, stack: &mut Vec) { + debug_assert!(matches!(self, &ControlStackFrame::If { .. })); + stack.truncate(self.original_stack_size()); + } + + /// Pop values from the value stack so that it is left at the state it was + /// before this control-flow frame. + pub fn truncate_value_stack_to_original_size(&self, stack: &mut Vec) { + // The "If" frame pushes its parameters twice, so they're available to the else block + // (see also `FuncTranslationState::push_if`). + // Yet, the original_stack_size member accounts for them only once, so that the else + // block can see the same number of parameters as the consequent block. As a matter of + // fact, we need to substract an extra number of parameter values for if blocks. + let num_duplicated_params = match self { + &ControlStackFrame::If { + num_param_values, .. + } => { + debug_assert!(num_param_values <= self.original_stack_size()); + num_param_values + } + _ => 0, + }; + stack.truncate(self.original_stack_size() - num_duplicated_params); + } } /// Contains information passed along during a function's translation and that records: diff --git a/cranelift/wasmtests/if-unreachable-else-params-2.wat b/cranelift/wasmtests/if-unreachable-else-params-2.wat new file mode 100644 index 0000000000..369ea4db65 --- /dev/null +++ b/cranelift/wasmtests/if-unreachable-else-params-2.wat @@ -0,0 +1,18 @@ +(module + (type (;0;) (func (param i32 i32) (result f64))) + (func $main (type 0) (param i32 i32) (result f64) + f64.const 1.0 + local.get 0 + local.get 1 + if (param i32) ;; label = @2 + i64.load16_s align=1 + drop + else + unreachable + end) + (table (;0;) 63 255 funcref) + (memory (;0;) 13 16) + (export "t1" (table 0)) + (export "m1" (memory 0)) + (export "main" (func $main)) + (export "memory" (memory 0)))