Fix some fuzz bugs (#1528)

 *   remove-unused-brs: handle an if declared as returning a value despite having an unreachable condition
 *   simplify-locals: don't work on loops while the main pass is making changes, as set_locals are being tracked and modified.

6 files changed, 150 insertions, 16 deletions

diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index c8c2277dc..87f53a652 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -683,7 +683,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
         // TODO: handle a condition in the br? need to watch for side effects
         auto* iff = curr->value->dynCast<If>();
         if (!iff) return;
-        if (!isConcreteType(iff->type)) return;
+        if (!isConcreteType(iff->type) || !isConcreteType(iff->condition->type)) return;
         auto tryToOptimize = [&](Expression* one, Expression* two, bool flipCondition) {
           if (one->type == unreachable && two->type != unreachable) {
             if (auto* br = one->dynCast<Break>()) {
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 6f8b346b5..af5a9a4c4 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -203,7 +203,9 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals<a
 
   void visitLoop(Loop* curr) {
     if (allowStructure) {
-      loops.push_back(this->getCurrentPointer());
+      if (canUseLoopReturnValue(curr)) {
+        loops.push_back(this->getCurrentPointer());
+      }
     }
   }
 
@@ -609,20 +611,20 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals<a
         ifsToEnlarge.clear();
         anotherCycle = true;
       }
-      // handle loops (we can't modify set_locals in the main pass, as they are
-      // being tracked)
-      for (auto* currp : loops) {
-        auto* curr = (*currp)->template cast<Loop>();
-        // Optimizing a loop return value is trivial: just see if it contains
-        // a set_local, and pull that out.
-        if (auto* set = curr->body->template dynCast<SetLocal>()) {
-          if (isConcreteType(set->value->type)) {
-            curr->body = set->value;
-            set->value = curr;
-            curr->finalize(curr->body->type);
-            *currp = set;
-            anotherCycle = true;
-          }
+      // handle loops. note that a lot happens in this pass, and we can't just modify
+      // set_locals when we see a loop - it might be tracked - and we can't also just
+      // assume our loop didn't move either (might be in a block now). So we do this
+      // when all other work is done - as loop return values are rare, that is fine.
+      if (!anotherCycle) {
+        for (auto* currp : loops) {
+          auto* curr = (*currp)->template cast<Loop>();
+          assert(canUseLoopReturnValue(curr));
+          auto* set = curr->body->template cast<SetLocal>();
+          curr->body = set->value;
+          set->value = curr;
+          curr->finalize(curr->body->type);
+          *currp = set;
+          anotherCycle = true;
         }
       }
       loops.clear();
@@ -645,6 +647,17 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals<a
     remover.numGetLocals = &getCounter.num;
     remover.walkFunction(func);
   }
+
+  bool canUseLoopReturnValue(Loop* curr) {
+    // Optimizing a loop return value is trivial: just see if it contains
+    // a set_local, and pull that out.
+    if (auto* set = curr->body->template dynCast<SetLocal>()) {
+      if (isConcreteType(set->value->type)) {
+        return true;
+      }
+    }
+    return false;
+  }
 };
 
 Pass *createSimplifyLocalsPass() {
diff --git a/test/passes/remove-unused-brs.txt b/test/passes/remove-unused-brs.txt
index cbfa4d5d5..378ee4812 100644
--- a/test/passes/remove-unused-brs.txt
+++ b/test/passes/remove-unused-brs.txt
@@ -10,6 +10,7 @@
  (type $8 (func (result f32)))
  (type $9 (func (param i32) (result f32)))
  (type $10 (func (param i32) (result i32)))
+ (type $11 (func (param i32 f64 i32 f64 f32 f32) (result i32)))
  (memory $0 256 256)
  (func $b0-yes (; 0 ;) (type $0) (param $i1 i32)
   (block $topmost
@@ -1845,4 +1846,29 @@
   )
   (get_local $p)
  )
+ (func $if-unreachable-but-declares-value (; 74 ;) (type $11) (param $var$0 i32) (param $var$1 f64) (param $var$2 i32) (param $var$3 f64) (param $var$4 f32) (param $var$5 f32) (result i32)
+  (local $var$6 f64)
+  (if
+   (i32.const 0)
+   (drop
+    (loop $label$3 (result i64)
+     (block $label$4 (result i64)
+      (block $label$5
+       (block $label$6
+        (set_local $var$1
+         (if (result f64)
+          (unreachable)
+          (br $label$5)
+          (f64.const 1)
+         )
+        )
+       )
+      )
+      (i64.const 1)
+     )
+    )
+   )
+  )
+  (i32.const 0)
+ )
 )
diff --git a/test/passes/remove-unused-brs.wast b/test/passes/remove-unused-brs.wast
index 7c03b5366..dbb415deb 100644
--- a/test/passes/remove-unused-brs.wast
+++ b/test/passes/remove-unused-brs.wast
@@ -1468,5 +1468,31 @@
     )
     (get_local $p)
   )
+  (func $if-unreachable-but-declares-value (param $var$0 i32) (param $var$1 f64) (param $var$2 i32) (param $var$3 f64) (param $var$4 f32) (param $var$5 f32) (result i32)
+   (local $var$6 f64)
+   (if
+    (i32.const 0)
+    (drop
+     (loop $label$3 (result i64)
+      (block $label$4 (result i64)
+       (block $label$5
+        (block $label$6
+         (set_local $var$1
+          (if (result f64)
+           (unreachable)
+           (br $label$5)
+           (f64.const 1)
+          )
+         )
+        )
+        (nop)
+       )
+       (i64.const 1)
+      )
+     )
+    )
+   )
+   (i32.const 0)
+  )
 )
 
diff --git a/test/passes/simplify-locals.txt b/test/passes/simplify-locals.txt
index 21a8ab403..acd8b4637 100644
--- a/test/passes/simplify-locals.txt
+++ b/test/passes/simplify-locals.txt
@@ -1197,4 +1197,41 @@
    )
   )
  )
+ (func $loop-modified-during-main-pass-be-careful-fuzz (; 14 ;) (type $FUNCSIG$i) (result i32)
+  (local $0 i32)
+  (nop)
+  (if (result i32)
+   (i32.const 0)
+   (block (result i32)
+    (nop)
+    (i32.const 0)
+   )
+   (block
+    (loop $label$4
+     (br $label$4)
+    )
+    (nop)
+   )
+  )
+ )
+ (func $loop-later (; 15 ;) (type $FUNCSIG$iiiiii) (param $var$0 i32) (param $var$1 i32) (param $var$2 i32) (param $var$3 i32) (param $var$4 i32) (result i32)
+  (drop
+   (loop $label$1 (result i32)
+    (block $label$2 (result i32)
+     (if
+      (i32.const 0)
+      (block $block
+       (nop)
+       (br $label$2
+        (i32.const -1)
+       )
+      )
+     )
+     (nop)
+     (i32.const -1)
+    )
+   )
+  )
+  (i32.const 0)
+ )
 )
diff --git a/test/passes/simplify-locals.wast b/test/passes/simplify-locals.wast
index 28f584f83..3d82fe0a4 100644
--- a/test/passes/simplify-locals.wast
+++ b/test/passes/simplify-locals.wast
@@ -1097,4 +1097,36 @@
     )
     (get_local $x)
   )
+  (func $loop-modified-during-main-pass-be-careful-fuzz (result i32)
+   (local $0 i32)
+   (if
+    (i32.const 0)
+    (set_local $0
+     (i32.const 0)
+    )
+    (loop $label$4
+     (br $label$4)
+    )
+   )
+   (get_local $0)
+  )
+  (func $loop-later (param $var$0 i32) (param $var$1 i32) (param $var$2 i32) (param $var$3 i32) (param $var$4 i32) (result i32)
+   (loop $label$1
+    (block $label$2
+     (if
+      (i32.const 0)
+      (block
+       (set_local $var$0
+        (i32.const -1)
+       )
+       (br $label$2)
+      )
+     )
+     (set_local $var$0
+      (i32.const -1)
+     )
+    )
+   )
+   (i32.const 0)
+  )
 )