diff --git a/cranelift/wasmtests/unreachable_code.wat b/cranelift/wasmtests/unreachable_code.wat new file mode 100644 index 0000000000..38c1a315ce --- /dev/null +++ b/cranelift/wasmtests/unreachable_code.wat @@ -0,0 +1,77 @@ +(module + (type (;0;) (func (param i32 i64 f64) (result f64))) + (type (;1;) (func)) + (type (;2;) (func (result f32))) + (type (;3;) (func (result f64))) + (type (;4;) (func (param f64 f64) (result f64))) + (type (;5;) (func (result i32))) + (func (result i32) + block (result i32) + unreachable + end + block + end + i32.clz + ) + (func (result i32) + loop (result i32) + unreachable + end + block + end + i32.clz + ) + (func (;0;) (type 5) (result i32) + nop + block (result i32) ;; label = @1 + block ;; label = @2 + block ;; label = @3 + nop + block ;; label = @4 + i32.const 1 + if ;; label = @5 + nop + block ;; label = @6 + nop + nop + loop (result i32) ;; label = @7 + nop + block (result i32) ;; label = @8 + nop + nop + block (result i32) ;; label = @9 + nop + unreachable + end + end + end + block (result i32) ;; label = @7 + block ;; label = @8 + nop + end + i32.const 0 + end + br_if 5 (;@1;) + drop + end + else + nop + end + nop + end + end + end + unreachable + end) + (func + block (result i32) + block (result i32) + i32.const 1 + br 1 + end + end + drop + ) + (table (;0;) 16 anyfunc) + (elem (i32.const 0)) +) diff --git a/lib/wasm/src/code_translator.rs b/lib/wasm/src/code_translator.rs index 903b9cd9e1..abaaaef3bf 100644 --- a/lib/wasm/src/code_translator.rs +++ b/lib/wasm/src/code_translator.rs @@ -25,6 +25,7 @@ use cretonne::ir::{self, InstBuilder, MemFlags, JumpTableData}; use cretonne::ir::types::*; use cretonne::ir::condcodes::{IntCC, FloatCC}; +use cretonne::packed_option::ReservedValue; use cton_frontend::FunctionBuilder; use wasmparser::{Operator, MemoryImmediate}; use translation_utils::{f32_translation, f64_translation, type_to_type, num_return_values, Local}; @@ -42,7 +43,7 @@ pub fn translate_operator( state: &mut TranslationState, environ: &mut FE, ) { - if state.in_unreachable_code() { + if !state.reachable { return translate_unreachable_operator(op, builder, state); } @@ -105,7 +106,7 @@ pub fn translate_operator( // We use `trap user0` to indicate a user-generated trap. // We could make the trap code configurable if need be. builder.ins().trap(ir::TrapCode::User(0)); - state.real_unreachable_stack_depth = 1; + state.reachable = false; } /***************************** Control flow blocks ********************************** * When starting a control flow block, we create a new `Ebb` that will hold the code @@ -155,15 +156,24 @@ pub fn translate_operator( // and push a new control frame with a new ebb for the code after the if/then/else // At the end of the then clause we jump to the destination let i = state.control_stack.len() - 1; - let (destination, return_count, branch_inst) = match state.control_stack[i] { - ControlStackFrame::If { - destination, - num_return_values, - branch_inst, - .. - } => (destination, num_return_values, branch_inst), - _ => panic!("should not happen"), - }; + let (destination, return_count, branch_inst, ref mut reachable_from_top) = + match state.control_stack[i] { + ControlStackFrame::If { + destination, + num_return_values, + branch_inst, + reachable_from_top, + .. + } => ( + destination, + num_return_values, + branch_inst, + reachable_from_top, + ), + _ => panic!("should not happen"), + }; + // The if has an else, so there's no branch to the end from the top. + *reachable_from_top = false; builder.ins().jump(destination, state.peekn(return_count)); state.popn(return_count); // We change the target of the branch instruction @@ -219,7 +229,7 @@ pub fn translate_operator( let (return_count, br_destination) = { let frame = &mut state.control_stack[i]; // We signal that all the code that follows until the next End is unreachable - frame.set_reachable(); + frame.set_branched_to_exit(); let return_count = if frame.is_loop() { 0 } else { @@ -232,7 +242,7 @@ pub fn translate_operator( state.peekn(return_count), ); state.popn(return_count); - state.real_unreachable_stack_depth = 1 + relative_depth as usize; + state.reachable = false; } Operator::BrIf { relative_depth } => { let val = state.pop1(); @@ -241,7 +251,7 @@ pub fn translate_operator( let frame = &mut state.control_stack[i]; // The values returned by the branch are still available for the reachable // code that comes after it - frame.set_reachable(); + frame.set_branched_to_exit(); let return_count = if frame.is_loop() { 0 } else { @@ -277,20 +287,23 @@ pub fn translate_operator( let val = state.pop1(); let mut data = JumpTableData::with_capacity(depths.len()); for depth in depths { - let i = state.control_stack.len() - 1 - (depth as usize); - let frame = &mut state.control_stack[i]; - let ebb = frame.br_destination(); + let ebb = { + let i = state.control_stack.len() - 1 - (depth as usize); + let frame = &mut state.control_stack[i]; + frame.set_branched_to_exit(); + frame.br_destination() + }; data.push_entry(ebb); - frame.set_reachable(); } let jt = builder.create_jump_table(data); builder.ins().br_table(val, jt); - let i = state.control_stack.len() - 1 - (default as usize); - let frame = &mut state.control_stack[i]; - let ebb = frame.br_destination(); + let ebb = { + let i = state.control_stack.len() - 1 - (default as usize); + let frame = &mut state.control_stack[i]; + frame.set_branched_to_exit(); + frame.br_destination() + }; builder.ins().jump(ebb, &[]); - state.real_unreachable_stack_depth = 1 + min_depth as usize; - frame.set_reachable(); } else { // Here we have jump arguments, but Cretonne's br_table doesn't support them // We then proceed to split the edges going out of the br_table @@ -312,29 +325,32 @@ pub fn translate_operator( } let jt = builder.create_jump_table(data); builder.ins().br_table(val, jt); - let default_ebb = state.control_stack[state.control_stack.len() - 1 - - (default as usize)] - .br_destination(); + let default_ebb = { + let i = state.control_stack.len() - 1 - (default as usize); + let frame = &mut state.control_stack[i]; + frame.set_branched_to_exit(); + frame.br_destination() + }; builder.ins().jump(default_ebb, state.peekn(return_count)); for (depth, dest_ebb) in dest_ebb_sequence { builder.switch_to_block(dest_ebb); builder.seal_block(dest_ebb); - let i = state.control_stack.len() - 1 - depth; let real_dest_ebb = { + let i = state.control_stack.len() - 1 - depth; let frame = &mut state.control_stack[i]; - frame.set_reachable(); + frame.set_branched_to_exit(); frame.br_destination() }; builder.ins().jump(real_dest_ebb, state.peekn(return_count)); } state.popn(return_count); - state.real_unreachable_stack_depth = 1 + min_depth as usize; } + state.reachable = false; } Operator::Return => { let (return_count, br_destination) = { let frame = &mut state.control_stack[0]; - frame.set_reachable(); + frame.set_branched_to_exit(); let return_count = frame.num_return_values(); (return_count, frame.br_destination()) }; @@ -347,7 +363,7 @@ pub fn translate_operator( } } state.popn(return_count); - state.real_unreachable_stack_depth = 1; + state.reachable = false; } /************************************ Calls **************************************** * The call instructions pop off their arguments from the stack and append their @@ -900,76 +916,74 @@ fn translate_unreachable_operator( builder: &mut FunctionBuilder, state: &mut TranslationState, ) { - let stack = &mut state.stack; - let control_stack = &mut state.control_stack; - - // We don't translate because the code is unreachable - // Nevertheless we have to record a phantom stack for this code - // to know when the unreachable code ends match *op { - Operator::If { ty: _ } | + Operator::If { ty: _ } => { + // Push a placeholder control stack entry. The if isn't reachable, + // so we don't have any branches anywhere. + state.push_if(ir::Inst::reserved_value(), ir::Ebb::reserved_value(), 0); + } Operator::Loop { ty: _ } | Operator::Block { ty: _ } => { - state.phantom_unreachable_stack_depth += 1; - } - Operator::End => { - if state.phantom_unreachable_stack_depth > 0 { - state.phantom_unreachable_stack_depth -= 1; - } else { - // This End corresponds to a real control stack frame - // We switch to the destination block but we don't insert - // a jump instruction since the code is still unreachable - let frame = control_stack.pop().unwrap(); - - builder.switch_to_block(frame.following_code()); - builder.seal_block(frame.following_code()); - match frame { - // If it is a loop we also have to seal the body loop block - ControlStackFrame::Loop { header, .. } => builder.seal_block(header), - // If it is an if then the code after is reachable again - ControlStackFrame::If { .. } => { - state.real_unreachable_stack_depth = 1; - } - _ => {} - } - if frame.is_reachable() { - state.real_unreachable_stack_depth = 1; - } - // 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()); - // And add the return values of the block but only if the next block is reachble - // (which corresponds to testing if the stack depth is 1) - if state.real_unreachable_stack_depth == 1 { - stack.extend_from_slice(builder.ebb_params(frame.following_code())); - } - state.real_unreachable_stack_depth -= 1; - } + state.push_block(ir::Ebb::reserved_value(), 0); } Operator::Else => { - if state.phantom_unreachable_stack_depth > 0 { - // This is part of a phantom if-then-else, we do nothing - } else { - // Encountering an real else means that the code in the else - // clause is reachable again - let (branch_inst, original_stack_size) = match control_stack[control_stack.len() - - 1] { - ControlStackFrame::If { - branch_inst, - original_stack_size, - .. - } => (branch_inst, original_stack_size), - _ => panic!("should not happen"), - }; - // We change the target of the branch instruction - let else_ebb = builder.create_ebb(); - builder.change_jump_destination(branch_inst, else_ebb); - builder.seal_block(else_ebb); - builder.switch_to_block(else_ebb); - // Now we have to split off the stack the values not used - // by unreachable code that hasn't been translated - stack.truncate(original_stack_size); - state.real_unreachable_stack_depth = 0; + let i = state.control_stack.len() - 1; + match state.control_stack[i] { + ControlStackFrame::If { + branch_inst, + ref mut reachable_from_top, + .. + } => { + if *reachable_from_top { + // We have a branch from the top of the if to the else. + state.reachable = true; + // And because there's an else, there can no longer be a + // branch from the top directly to the end. + *reachable_from_top = false; + + // We change the target of the branch instruction + let else_ebb = builder.create_ebb(); + builder.change_jump_destination(branch_inst, else_ebb); + builder.seal_block(else_ebb); + builder.switch_to_block(else_ebb); + } + } + _ => {} + } + } + Operator::End => { + let stack = &mut state.stack; + 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()); + + let reachable_anyway = match frame { + // If it is a loop we also have to seal the body loop block + ControlStackFrame::Loop { header, .. } => { + builder.seal_block(header); + // And loops can't have branches to the end. + false + } + ControlStackFrame::If { reachable_from_top, .. } => { + // A reachable if without an else has a branch from the top + // directly to the bottom. + reachable_from_top + } + // All other control constructs are already handled. + _ => false, + }; + + if frame.exit_is_branched_to() || reachable_anyway { + builder.switch_to_block(frame.following_code()); + builder.seal_block(frame.following_code()); + + // And add the return values of the block but only if the next block is reachble + // (which corresponds to testing if the stack depth is 1) + stack.extend_from_slice(builder.ebb_params(frame.following_code())); + state.reachable = true; } } _ => { diff --git a/lib/wasm/src/func_translator.rs b/lib/wasm/src/func_translator.rs index 5e8555c194..a9389342e9 100644 --- a/lib/wasm/src/func_translator.rs +++ b/lib/wasm/src/func_translator.rs @@ -206,9 +206,11 @@ fn parse_function_body( // // If the exit block is unreachable, it may not have the correct arguments, so we would // generate a return instruction that doesn't match the signature. - debug_assert!(builder.is_pristine()); - if !builder.is_unreachable() { - builder.ins().return_(&state.stack); + if state.reachable { + debug_assert!(builder.is_pristine()); + if !builder.is_unreachable() { + builder.ins().return_(&state.stack); + } } // Discard any remaining values on the stack. Either we just returned them, diff --git a/lib/wasm/src/state.rs b/lib/wasm/src/state.rs index f9f76d2934..1943f31326 100644 --- a/lib/wasm/src/state.rs +++ b/lib/wasm/src/state.rs @@ -25,20 +25,20 @@ pub enum ControlStackFrame { branch_inst: Inst, num_return_values: usize, original_stack_size: usize, - reachable: bool, + exit_is_branched_to: bool, + reachable_from_top: bool, }, Block { destination: Ebb, num_return_values: usize, original_stack_size: usize, - reachable: bool, + exit_is_branched_to: bool, }, Loop { destination: Ebb, header: Ebb, num_return_values: usize, original_stack_size: usize, - reachable: bool, }, } @@ -80,19 +80,21 @@ impl ControlStackFrame { } } - pub fn is_reachable(&self) -> bool { + pub fn exit_is_branched_to(&self) -> bool { match *self { - ControlStackFrame::If { reachable, .. } | - ControlStackFrame::Block { reachable, .. } | - ControlStackFrame::Loop { reachable, .. } => reachable, + ControlStackFrame::If { exit_is_branched_to, .. } | + ControlStackFrame::Block { exit_is_branched_to, .. } => exit_is_branched_to, + ControlStackFrame::Loop { .. } => false, } } - pub fn set_reachable(&mut self) { + pub fn set_branched_to_exit(&mut self) { match *self { - ControlStackFrame::If { ref mut reachable, .. } | - ControlStackFrame::Block { ref mut reachable, .. } | - ControlStackFrame::Loop { ref mut reachable, .. } => *reachable = true, + ControlStackFrame::If { ref mut exit_is_branched_to, .. } | + ControlStackFrame::Block { ref mut exit_is_branched_to, .. } => { + *exit_is_branched_to = true + } + ControlStackFrame::Loop { .. } => {} } } } @@ -105,8 +107,7 @@ impl ControlStackFrame { pub struct TranslationState { pub stack: Vec, pub control_stack: Vec, - pub phantom_unreachable_stack_depth: usize, - pub real_unreachable_stack_depth: usize, + pub reachable: bool, // Map of global variables that have already been created by `FuncEnvironment::make_global`. globals: HashMap, @@ -130,8 +131,7 @@ impl TranslationState { Self { stack: Vec::new(), control_stack: Vec::new(), - phantom_unreachable_stack_depth: 0, - real_unreachable_stack_depth: 0, + reachable: true, globals: HashMap::new(), heaps: HashMap::new(), signatures: HashMap::new(), @@ -142,8 +142,7 @@ impl TranslationState { fn clear(&mut self) { debug_assert!(self.stack.is_empty()); debug_assert!(self.control_stack.is_empty()); - debug_assert_eq!(self.phantom_unreachable_stack_depth, 0); - debug_assert_eq!(self.real_unreachable_stack_depth, 0); + self.reachable = true; self.globals.clear(); self.heaps.clear(); self.signatures.clear(); @@ -219,7 +218,7 @@ impl TranslationState { destination: following_code, original_stack_size: self.stack.len(), num_return_values: num_result_types, - reachable: false, + exit_is_branched_to: false, }); } @@ -230,7 +229,6 @@ impl TranslationState { destination: following_code, original_stack_size: self.stack.len(), num_return_values: num_result_types, - reachable: false, }); } @@ -241,19 +239,10 @@ impl TranslationState { destination: following_code, original_stack_size: self.stack.len(), num_return_values: num_result_types, - reachable: false, + exit_is_branched_to: false, + reachable_from_top: self.reachable, }); } - - /// Test if the translation state is currently in unreachable code. - pub fn in_unreachable_code(&self) -> bool { - if self.real_unreachable_stack_depth > 0 { - true - } else { - debug_assert_eq!(self.phantom_unreachable_stack_depth, 0, "in reachable code"); - false - } - } } /// Methods for handling entity references.