when we re-finalize a function body, we may have changed it from unreachable to none. that is bad if the function has a return value, as unreachable was ok but none is not. in that case, we must add an unreachable (#1193)

3 files changed, 39 insertions, 0 deletions

diff --git a/src/ast_utils.h b/src/ast_utils.h
index 95f725636..dfa233f26 100644
--- a/src/ast_utils.h
+++ b/src/ast_utils.h
@@ -176,6 +176,15 @@ struct ReFinalize : public WalkerPass<PostWalker<ReFinalize>> {
   void visitNop(Nop *curr) { curr->finalize(); }
   void visitUnreachable(Unreachable *curr) { curr->finalize(); }
 
+  void visitFunction(Function* curr) {
+    // we may have changed the body from unreachable to none, which might be bad
+    // if the function has a return value
+    if (curr->result != none && curr->body->type == none) {
+      Builder builder(*getModule());
+      curr->body = builder.blockify(curr->body, builder.makeUnreachable());
+    }
+  }
+
   WasmType getValueType(Expression* value) {
     return value ? value->type : none;
   }
diff --git a/test/passes/remove-unused-brs.txt b/test/passes/remove-unused-brs.txt
index 6991680f7..39a4d89f3 100644
--- a/test/passes/remove-unused-brs.txt
+++ b/test/passes/remove-unused-brs.txt
@@ -1121,4 +1121,20 @@
    )
   )
  )
+ (func $unreachable-return-loop-value (type $7) (result i64)
+  (loop $loop
+   (br_if $loop
+    (i32.eqz
+     (i32.const 1)
+    )
+   )
+   (block $block
+    (br_if $block
+     (br $loop)
+    )
+    (br $loop)
+   )
+  )
+  (unreachable)
+ )
 )
diff --git a/test/passes/remove-unused-brs.wast b/test/passes/remove-unused-brs.wast
index 5c6776226..14b4abc4c 100644
--- a/test/passes/remove-unused-brs.wast
+++ b/test/passes/remove-unused-brs.wast
@@ -1006,5 +1006,19 @@
     )
    )
   )
+  (func $unreachable-return-loop-value (result i64)
+   (loop $loop
+    (if
+     (i32.const 1)
+     (block $block
+      (br_if $block
+       (br $loop)
+      )
+      (br $loop)
+     )
+    )
+    (br $loop) ;; we 100% go back to the loop top, the loop is never exited. but opts move code around so that is not obvious anymore, and the loop becomes a nop, but the func has a return value
+   )
+  )
 )