fix emitting of unreachable ifs (#944)

5 files changed, 160 insertions, 13 deletions

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 4d5d7e176..c723dfe32 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -507,17 +507,12 @@ void WasmBinaryWriter::recursePossibleBlockContents(Expression* curr) {
 
 void WasmBinaryWriter::visitIf(If *curr) {
   if (debug) std::cerr << "zz node: If" << std::endl;
-  if (curr->type == unreachable && curr->ifFalse) {
-    if (curr->condition->type == unreachable) {
-      // this if-else is unreachable because of the condition, i.e., the condition
-      // does not exit. So don't emit the if, but do consume the condition
-      recurse(curr->condition);
-      o << int8_t(BinaryConsts::Unreachable);
-      return;
-    }
-    // an unreachable if-else (with reachable condition) is one where both sides do not fall through.
-    // wasm does not allow this to be emitted directly, so we must do something more. we could do
-    // better, but for now we emit an extra unreachable instruction after the if, so it is not consumed itself
+  if (curr->condition->type == unreachable) {
+    // this if-else is unreachable because of the condition, i.e., the condition
+    // does not exit. So don't emit the if, but do consume the condition
+    recurse(curr->condition);
+    o << int8_t(BinaryConsts::Unreachable);
+    return;
   }
   recurse(curr->condition);
   o << int8_t(BinaryConsts::If);
@@ -533,8 +528,11 @@ void WasmBinaryWriter::visitIf(If *curr) {
   }
   o << int8_t(BinaryConsts::End);
   if (curr->type == unreachable) {
-    // see explanation above - we emitted an if without a return type, so it must not be consumed
-    assert(curr->ifFalse); // otherwise, if without else, that is unreachable, must have an unreachable condition, which was handled earlier
+    // we already handled the case of the condition being unreachable. otherwise,
+    // we may still be unreachable, if we are an if-else with both sides unreachable.
+    // wasm does not allow this to be emitted directly, so we must do something more. we could do
+    // better, but for now we emit an extra unreachable instruction after the if, so it is not consumed itself,
+    assert(curr->ifFalse);
     o << int8_t(BinaryConsts::Unreachable);
   }
 }
diff --git a/test/unit.wast b/test/unit.wast
index 6c3d8e0fa..2cd164238 100644
--- a/test/unit.wast
+++ b/test/unit.wast
@@ -525,4 +525,16 @@
       (return (i32.const 1))
     )
   )
+  (func $unreachable-ifs
+    (if (unreachable) (nop))
+    (if (unreachable) (unreachable))
+    (if (unreachable) (nop) (nop))
+    (if (unreachable) (unreachable) (nop))
+    (if (unreachable) (nop) (unreachable))
+    (if (unreachable) (unreachable) (unreachable))
+    ;;
+    (if (i32.const 1) (unreachable) (nop))
+    (if (i32.const 1) (nop) (unreachable))
+    (if (i32.const 1) (unreachable) (unreachable))
+  )
 )
diff --git a/test/unit.wast.from-wast b/test/unit.wast.from-wast
index b7b914795..99def6ded 100644
--- a/test/unit.wast.from-wast
+++ b/test/unit.wast.from-wast
@@ -557,4 +557,49 @@
    )
   )
  )
+ (func $unreachable-ifs (type $FUNCSIG$v)
+  (if
+   (unreachable)
+   (nop)
+  )
+  (if
+   (unreachable)
+   (unreachable)
+  )
+  (if
+   (unreachable)
+   (nop)
+   (nop)
+  )
+  (if
+   (unreachable)
+   (unreachable)
+   (nop)
+  )
+  (if
+   (unreachable)
+   (nop)
+   (unreachable)
+  )
+  (if
+   (unreachable)
+   (unreachable)
+   (unreachable)
+  )
+  (if
+   (i32.const 1)
+   (unreachable)
+   (nop)
+  )
+  (if
+   (i32.const 1)
+   (nop)
+   (unreachable)
+  )
+  (if
+   (i32.const 1)
+   (unreachable)
+   (unreachable)
+  )
+ )
 )
diff --git a/test/unit.wast.fromBinary b/test/unit.wast.fromBinary
index 7a9b1c58d..266cd2246 100644
--- a/test/unit.wast.fromBinary
+++ b/test/unit.wast.fromBinary
@@ -595,5 +595,51 @@
   )
   (unreachable)
  )
+ (func $unreachable-ifs (type $1)
+  (block $label$0
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (if
+    (i32.const 1)
+    (block $label$1
+     (unreachable)
+    )
+    (block $label$2
+     (nop)
+    )
+   )
+   (if
+    (i32.const 1)
+    (block $label$3
+     (nop)
+    )
+    (block $label$4
+     (unreachable)
+    )
+   )
+   (if
+    (i32.const 1)
+    (block $label$5
+     (unreachable)
+    )
+    (block $label$6
+     (unreachable)
+    )
+   )
+   (unreachable)
+   (unreachable)
+  )
+  (unreachable)
+ )
 )
 
diff --git a/test/unit.wast.fromBinary.noDebugInfo b/test/unit.wast.fromBinary.noDebugInfo
index 704da90f3..6818db1bb 100644
--- a/test/unit.wast.fromBinary.noDebugInfo
+++ b/test/unit.wast.fromBinary.noDebugInfo
@@ -595,5 +595,51 @@
   )
   (unreachable)
  )
+ (func $35 (type $1)
+  (block $label$0
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (unreachable)
+   (if
+    (i32.const 1)
+    (block $label$1
+     (unreachable)
+    )
+    (block $label$2
+     (nop)
+    )
+   )
+   (if
+    (i32.const 1)
+    (block $label$3
+     (nop)
+    )
+    (block $label$4
+     (unreachable)
+    )
+   )
+   (if
+    (i32.const 1)
+    (block $label$5
+     (unreachable)
+    )
+    (block $label$6
+     (unreachable)
+    )
+   )
+   (unreachable)
+   (unreachable)
+  )
+  (unreachable)
+ )
 )