From dd5a5ebdbcb76ce90e16524b3a7c951022e7348d Mon Sep 17 00:00:00 2001
From: Chris Fallin <cfallin@mozilla.com>
Date: Fri, 7 Aug 2020 18:33:04 -0700
Subject: [PATCH] Wasm translation bugfix: properly clean up value stack for
 else-branch when if-branch ends in unreachable.

The Wasm translation handles unreachable code sections
specially, skipping ops until the end of a block and a control-flow
merger at which code becomes reachable again. Unfortunately, while the
ordinary else-op handler properly sets up the value stack for the
else-branch with the parameters to the if/else, the unreachable-case
else-op handler did not. This resulted in a bad translation and CLIF
type error despite valid Wasm.

Found via fuzzing by :decoder in
https://bugzilla.mozilla.org/show_bug.cgi?id=1657895.
---
 cranelift/wasm/src/code_translator.rs         | 10 ++++-
 .../wasmtests/if-unreachable-else-params.wat  | 41 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 cranelift/wasmtests/if-unreachable-else-params.wat

diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs
index 8e6b008f08..0a0b3ce4a7 100644
--- a/cranelift/wasm/src/code_translator.rs
+++ b/cranelift/wasm/src/code_translator.rs
@@ -1888,13 +1888,21 @@ fn translate_unreachable_operator<FE: FuncEnvironment + ?Sized>(
                                 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(),
+                                );
 
                                 // We change the target of the branch instruction.
                                 builder.change_jump_destination(branch_inst, else_block);
                                 builder.seal_block(else_block);
                                 else_block
                             }
-                            ElseData::WithElse { else_block } => else_block,
+                            ElseData::WithElse { else_block } => {
+                                state.stack.truncate(
+                                    state.control_stack.last().unwrap().original_stack_size(),
+                                );
+                                else_block
+                            }
                         };
 
                         builder.switch_to_block(else_block);
diff --git a/cranelift/wasmtests/if-unreachable-else-params.wat b/cranelift/wasmtests/if-unreachable-else-params.wat
new file mode 100644
index 0000000000..02e2d50f95
--- /dev/null
+++ b/cranelift/wasmtests/if-unreachable-else-params.wat
@@ -0,0 +1,41 @@
+(module
+  (type (;0;) (func (param i32)))
+  (func $main (type 0) (param i32)
+    i32.const 35
+    loop (param i32)  ;; label = @1
+      local.get 0
+      if (param i32)  ;; label = @2
+        i64.load16_s align=1
+        unreachable
+        unreachable
+        unreachable
+        unreachable
+        unreachable
+        local.get 0
+        unreachable
+        unreachable
+        i64.load8_u offset=11789
+        unreachable
+      else
+        i32.popcnt
+        local.set 0
+        return
+        unreachable
+      end
+      unreachable
+      unreachable
+      nop
+      f32.lt
+      i32.store8 offset=82
+      unreachable
+    end
+    unreachable
+    unreachable
+    unreachable
+    unreachable)
+  (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)))