Fix a merge-blocks fuzz bug (#1730)

If a block has code after an unreachable element, it makes merging to an outer block tricky - the child block may be unreachable, but the parent have a return type,

(block (result i32)
  ..
  (block
    (unreachable)
    (nop)
  )
)

It's ok to end an unreachable block with a nop, but not a typed one.

To avoid this, if a child block has dce-able code, just ignore it.

3 files changed, 152 insertions, 22 deletions

diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 352a0a08d..50610afed 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -175,6 +175,18 @@ static bool hasUnreachableChild(Block* block) {
   return false;
 }
 
+// Checks for code after an unreachable element.
+static bool hasDeadCode(Block* block) {
+  auto& list = block->list;
+  auto size = list.size();
+  for (size_t i = 1; i < size; i++) {
+    if (list[i - 1]->type == unreachable) {
+      return true;
+    }
+  }
+  return false;
+}
+
 // core block optimizer routine
 static void optimizeBlock(Block* curr, Module* module, PassOptions& passOptions) {
   bool more = true;
@@ -244,6 +256,9 @@ static void optimizeBlock(Block* curr, Module* module, PassOptions& passOptions)
       auto& childList = childBlock->list;
       auto childSize = childList.size();
       if (childSize == 0) continue;
+      // If the child has items after an unreachable, ignore it - dce should have
+      // been run, and we prefer to not handle the complexity here.
+      if (hasDeadCode(childBlock)) continue;
       // If this is a loop, we may be removing only the tail.
       Index start = 0;
       Index end = childSize;
@@ -304,7 +319,9 @@ static void optimizeBlock(Block* curr, Module* module, PassOptions& passOptions)
       break;
     }
   }
-  if (changed) curr->finalize(curr->type);
+  if (changed) {
+    curr->finalize(curr->type);
+  }
 }
 
 void BreakValueDropper::visitBlock(Block* curr) {
diff --git a/test/passes/remove-unused-names_merge-blocks.txt b/test/passes/remove-unused-names_merge-blocks.txt
index 3e2755396..5cc6ca891 100644
--- a/test/passes/remove-unused-names_merge-blocks.txt
+++ b/test/passes/remove-unused-names_merge-blocks.txt
@@ -912,20 +912,24 @@
   )
  )
  (func $merging-with-unreachable-in-middle (; 30 ;) (type $4) (result i32)
-  (return
-   (i32.const 21536)
-  )
-  (block $label$15
-   (br $label$15)
+  (block (result i32)
+   (return
+    (i32.const 21536)
+   )
+   (block $label$15
+    (br $label$15)
+   )
+   (i32.const 19299)
   )
-  (i32.const 19299)
  )
  (func $remove-br-after-unreachable (; 31 ;) (type $3)
   (block $label$9
    (drop
     (block
-     (return)
-     (br $label$9)
+     (block
+      (return)
+      (br $label$9)
+     )
     )
    )
   )
@@ -1076,12 +1080,12 @@
     (i32.const -1)
    )
    (br $l1)
-  )
-  (drop
-   (i32.const 0)
-  )
-  (drop
-   (i32.const 1)
+   (drop
+    (i32.const 0)
+   )
+   (drop
+    (i32.const 1)
+   )
   )
   (loop $l2
    (br_if $l2
@@ -1245,12 +1249,12 @@
     (i32.const -1)
    )
    (br $l1)
-  )
-  (drop
-   (i32.const 0)
-  )
-  (drop
-   (i32.const 1)
+   (drop
+    (i32.const 0)
+   )
+   (drop
+    (i32.const 1)
+   )
   )
   (drop
    (i32.const 2)
@@ -1318,3 +1322,56 @@
   (unreachable)
  )
 )
+(module
+ (type $0 (func (param f64 i32) (result i32)))
+ (type $1 (func (result i32)))
+ (func $unreachable-in-sub-block (; 0 ;) (type $0) (param $0 f64) (param $1 i32) (result i32)
+  (local $2 i32)
+  (local $9 i32)
+  (loop $label$1
+   (set_local $9
+    (tee_local $2
+     (block $label$2 (result i32)
+      (drop
+       (br_if $label$2
+        (tee_local $2
+         (i32.const 0)
+        )
+        (i32.const 0)
+       )
+      )
+      (br_if $label$1
+       (i32.const 0)
+      )
+      (block
+       (unreachable)
+       (nop)
+      )
+     )
+    )
+   )
+  )
+  (nop)
+  (get_local $9)
+ )
+ (func $trivial (; 1 ;) (type $1) (result i32)
+  (block
+   (unreachable)
+   (nop)
+  )
+ )
+ (func $trivial-more (; 2 ;) (type $1) (result i32)
+  (block
+   (nop)
+   (unreachable)
+   (nop)
+   (nop)
+   (nop)
+  )
+  (block
+   (nop)
+   (unreachable)
+   (nop)
+  )
+ )
+)
diff --git a/test/passes/remove-unused-names_merge-blocks.wast b/test/passes/remove-unused-names_merge-blocks.wast
index cd11135f0..3e6add49c 100644
--- a/test/passes/remove-unused-names_merge-blocks.wast
+++ b/test/passes/remove-unused-names_merge-blocks.wast
@@ -1322,4 +1322,60 @@
   )
  )
 )
-
+(module
+ (func $unreachable-in-sub-block (param $0 f64) (param $1 i32) (result i32)
+  (local $2 i32)
+  (local $9 i32)
+  (loop $label$1
+   (set_local $9
+    (tee_local $2
+     (block $label$2 (result i32)
+      (block
+       (drop
+        (br_if $label$2
+         (tee_local $2
+          (i32.const 0)
+         )
+         (i32.const 0)
+        )
+       )
+      )
+      (br_if $label$1
+       (i32.const 0)
+      )
+      (block
+       (unreachable)
+       (nop) ;; bad if moved out to the outer block which is i32. current state works since this block is unreachable!
+      )
+     )
+    )
+   )
+  )
+  (nop)
+  (get_local $9)
+ )
+ (func $trivial (result i32)
+   (block (result i32)
+     (block
+       (unreachable)
+       (nop)
+     )
+   )
+ )
+ (func $trivial-more (result i32)
+   (block (result i32)
+     (block
+       (nop)
+       (unreachable)
+       (nop)
+       (nop)
+       (nop)
+     )
+     (block
+       (nop)
+       (unreachable)
+       (nop)
+     )
+   )
+ )
+)