Track wasm reachability explicitly.

Maintain an explicit "reachable" flag when decoding wasm. Push placeholder
frames on the control-flow stack instead of just maintaining a count of
the stack depth in unreachable code, so that we can whether If blocks
have Elses, and whether block exits are branched to, in all contexts.

Fixes #217.

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))
+)

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<FE: FuncEnvironment + ?Sized>(
     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<FE: FuncEnvironment + ?Sized>(
             // 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<FE: FuncEnvironment + ?Sized>(
             // 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] {
+            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),
+                    } => (
+                        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<FE: FuncEnvironment + ?Sized>(
             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<FE: FuncEnvironment + ?Sized>(
                 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<FE: FuncEnvironment + ?Sized>(
                 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<FE: FuncEnvironment + ?Sized>(
                 let val = state.pop1();
                 let mut data = JumpTableData::with_capacity(depths.len());
                 for depth in depths {
+                    let ebb = {
                         let i = state.control_stack.len() - 1 - (depth as usize);
                         let frame = &mut state.control_stack[i];
-                    let ebb = frame.br_destination();
+                        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 ebb = {
                     let i = state.control_stack.len() - 1 - (default as usize);
                     let frame = &mut state.control_stack[i];
-                let ebb = frame.br_destination();
+                    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<FE: FuncEnvironment + ?Sized>(
                 }
                 let jt = builder.create_jump_table(data);
                 builder.ins().br_table(val, jt);
-                let default_ebb = state.control_stack[state.control_stack.len() - 1 -
+                let default_ebb = {
-                                                          (default as usize)]
+                    let i = state.control_stack.len() - 1 - (default as usize);
-                    .br_destination();
+                    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<FE: FuncEnvironment + ?Sized>(
                 }
             }
             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<Local>,
     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;
+            state.push_block(ir::Ebb::reserved_value(), 0);
-        }
-        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;
-            }
         }
         Operator::Else => {
-            if state.phantom_unreachable_stack_depth > 0 {
+            let i = state.control_stack.len() - 1;
-                // This is part of a phantom if-then-else, we do nothing
+            match state.control_stack[i] {
-            } 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,
+                    ref mut reachable_from_top,
                     ..
-                    } => (branch_inst, original_stack_size),
+                } => {
-                    _ => panic!("should not happen"),
+                    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(original_stack_size);
+            stack.truncate(frame.original_stack_size());
-                state.real_unreachable_stack_depth = 0;
+
+            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;
             }
         }
         _ => {

lib/wasm/src/func_translator.rs

@@ -206,10 +206,12 @@ fn parse_function_body<FE: FuncEnvironment + ?Sized>(
     //
     // 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.
+    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,
     // or the end of the function is unreachable.

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::If { exit_is_branched_to, .. } |
-            ControlStackFrame::Block { reachable, .. } |
+            ControlStackFrame::Block { exit_is_branched_to, .. } => exit_is_branched_to,
-            ControlStackFrame::Loop { reachable, .. } => reachable,
+            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::If { ref mut exit_is_branched_to, .. } |
-            ControlStackFrame::Block { ref mut reachable, .. } |
+            ControlStackFrame::Block { ref mut exit_is_branched_to, .. } => {
-            ControlStackFrame::Loop { ref mut reachable, .. } => *reachable = true,
+                *exit_is_branched_to = true
+            }
+            ControlStackFrame::Loop { .. } => {}
         }
     }
 }
@@ -105,8 +107,7 @@ impl ControlStackFrame {
 pub struct TranslationState {
     pub stack: Vec<Value>,
     pub control_stack: Vec<ControlStackFrame>,
-    pub phantom_unreachable_stack_depth: usize,
+    pub reachable: bool,
-    pub real_unreachable_stack_depth: usize,
 
     // Map of global variables that have already been created by `FuncEnvironment::make_global`.
     globals: HashMap<GlobalIndex, GlobalValue>,
@@ -130,8 +131,7 @@ impl TranslationState {
         Self {
             stack: Vec::new(),
             control_stack: Vec::new(),
-            phantom_unreachable_stack_depth: 0,
+            reachable: true,
-            real_unreachable_stack_depth: 0,
             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);
+        self.reachable = true;
-        debug_assert_eq!(self.real_unreachable_stack_depth, 0);
         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.