From bcdedab9a97dbea1c2ee151ca1013e3896f16c8a Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Mon, 7 Oct 2024 13:43:08 -0700
Subject: Fix a fuzz issue with #6984 (#6988)

When I refactored the optimizeDroppedBlock logic in #6982, I didn't move the
unreachability check with that code, which was wrong. When that function
was called from another place in #6984, the fuzzer found an issue.

Diff without whitespace is smaller. This reverts almost all the test updates
from #6984 - those changes were on blocks with unreachable children.
The change was safe on them, but in general removing a block value in the
presence of unreachable code is tricky, so it's best to avoid it.

The testcase is a little bizarre, but it's the one the fuzzer found and I can't
find a way to generate a better one (other than to reduce it, which I did).
---
 src/passes/MergeBlocks.cpp | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

(limited to 'src')

diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 12f67329d..7518c73ca 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -206,6 +206,11 @@ static bool optimizeDroppedBlock(Drop* drop,
                                  PassOptions& options,
                                  BranchUtils::BranchSeekerCache& branchInfo) {
   assert(drop->value == block);
+  if (hasUnreachableChild(block)) {
+    // Don't move around unreachable code, as it can change types (leave it for
+    // DCE).
+    return false;
+  }
   if (block->name.is()) {
     // There may be breaks: see if we can remove their values.
     Expression* expression = block;
@@ -261,20 +266,14 @@ static void optimizeBlock(Block* curr,
         // drop into the block, and remove br values. This allows more merging.
         if (auto* drop = list[i]->dynCast<Drop>()) {
           childBlock = drop->value->dynCast<Block>();
-          if (childBlock) {
-            if (hasUnreachableChild(childBlock)) {
-              // don't move around unreachable code, as it can change types
-              // dce should have been run anyhow
-              continue;
-            }
-            if (optimizeDroppedBlock(
-                  drop, childBlock, *module, passOptions, branchInfo)) {
-              child = list[i] = childBlock;
-              more = true;
-              changed = true;
-            } else {
-              childBlock = nullptr;
-            }
+          if (childBlock &&
+              optimizeDroppedBlock(
+                drop, childBlock, *module, passOptions, branchInfo)) {
+            child = list[i] = childBlock;
+            more = true;
+            changed = true;
+          } else {
+            childBlock = nullptr;
           }
         } else if ((loop = list[i]->dynCast<Loop>())) {
           // We can merge a loop's "tail" - if the body is a block and has
-- 
cgit v1.2.3