Prevent calls from escaping try in CodeFolding (#2883)

In CodeFolding, we should not take an expression that may throw out of a
`try` scope. This patch adds this restriction in `canMove`.

3 files changed, 128 insertions, 11 deletions

diff --git a/src/passes/CodeFolding.cpp b/src/passes/CodeFolding.cpp
index 70a357bbf..10255a670 100644
--- a/src/passes/CodeFolding.cpp
+++ b/src/passes/CodeFolding.cpp
@@ -304,13 +304,28 @@ private:
         // anything exiting that is in all targets is something bad
         return false;
       }
-      // Currently pop instructions are only used for exnref.pop, which is a
-      // pseudo instruction following a catch. We check if the current
-      // expression has a pop child. This can be overly conservative, because
-      // this can also exclude whole try-catches that contain a pop within them.
-      if (getModule()->features.hasExceptionHandling() &&
-          !FindAll<Pop>(item).list.empty()) {
-        return false;
+      if (getModule()->features.hasExceptionHandling()) {
+        // Currently pop instructions are only used for exnref.pop, which is a
+        // pseudo instruction following a catch. We check if the current
+        // expression has a pop child. This can be overly conservative, because
+        // this can also exclude whole try-catches that contain a pop within
+        // them.
+        if (!FindAll<Pop>(item).list.empty()) {
+          return false;
+        }
+        // When an expression can throw and it is within a try scope, taking it
+        // out of the try scope changes the program's behavior, because the
+        // expression that would otherwise have been caught by the try now
+        // throws up to the next try scope or even up to the caller. We restrict
+        // the move if 'outOf' contains a 'try' anywhere in it. This is a
+        // conservative approximation because there can be cases that 'try' is
+        // within the expression that may throw so it is safe to take the
+        // expression out.
+        if (EffectAnalyzer(getPassOptions(), getModule()->features, item)
+              .throws &&
+            !FindAll<Try>(outOf).list.empty()) {
+          return false;
+        }
       }
     }
     return true;
diff --git a/test/passes/remove-unused-names_code-folding_all-features.txt b/test/passes/remove-unused-names_code-folding_all-features.txt
index 6cf344e1f..1cb02c26e 100644
--- a/test/passes/remove-unused-names_code-folding_all-features.txt
+++ b/test/passes/remove-unused-names_code-folding_all-features.txt
@@ -3,6 +3,7 @@
  (type $none_=>_i32 (func (result i32)))
  (type $i32_=>_i32 (func (param i32) (result i32)))
  (type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
+ (type $none_=>_exnref (func (result exnref)))
  (event $e (attr 0) (param))
  (func $ifs
   (if
@@ -1709,7 +1710,7 @@
    (i32.const 2)
   )
  )
- (func $exnref_pop_test
+ (func $exnref_pop-test
   (local $exn exnref)
   (block $folding-inner0
    (try
@@ -1746,7 +1747,7 @@
   )
   (unreachable)
  )
- (func $br_on_exn_target_block
+ (func $br_on_exn-target-block
   (local $exn exnref)
   (block $x
    (if
@@ -1787,4 +1788,59 @@
    (br $x)
   )
  )
+ (func $foo
+  (nop)
+ )
+ (func $try-call-optimize-terminating-tails (result exnref)
+  (try
+   (do
+    (call $foo)
+    (call $foo)
+    (call $foo)
+    (call $foo)
+    (return
+     (ref.null)
+    )
+   )
+   (catch
+    (drop
+     (exnref.pop)
+    )
+    (call $foo)
+    (call $foo)
+    (call $foo)
+    (call $foo)
+    (return
+     (ref.null)
+    )
+   )
+  )
+  (ref.null)
+ )
+ (func $try-call-optimize-expression-tails
+  (local $exn exnref)
+  (block $x
+   (try
+    (do
+     (local.set $exn
+      (exnref.pop)
+     )
+     (call $foo)
+     (call $foo)
+     (call $foo)
+     (br $x)
+    )
+    (catch
+     (local.set $exn
+      (exnref.pop)
+     )
+     (call $foo)
+     (call $foo)
+     (call $foo)
+     (br $x)
+    )
+   )
+   (unreachable)
+  )
+ )
 )
diff --git a/test/passes/remove-unused-names_code-folding_all-features.wast b/test/passes/remove-unused-names_code-folding_all-features.wast
index bbe05648b..ab5f222ca 100644
--- a/test/passes/remove-unused-names_code-folding_all-features.wast
+++ b/test/passes/remove-unused-names_code-folding_all-features.wast
@@ -1192,7 +1192,7 @@
     )
   )
 
-  (func $exnref_pop_test (local $exn exnref)
+  (func $exnref_pop-test (local $exn exnref)
     (try
       (do
         (try
@@ -1219,7 +1219,7 @@
   )
 
   (event $e (attr 0)) ;; exception with no param
-  (func $br_on_exn_target_block (local $exn exnref)
+  (func $br_on_exn-target-block (local $exn exnref)
     ;; Here this block $x is targeted by br_on_exn, so code folding out of this
     ;; block should NOT happen.
     (block $x
@@ -1244,4 +1244,50 @@
       (br $x)
     )
   )
+
+  (func $foo)
+  (func $try-call-optimize-terminating-tails (result exnref)
+    (try
+      (do
+        ;; Expressions that can throw should NOT be taken out of 'try' scope.
+        (call $foo)
+        (call $foo)
+        (call $foo)
+        (call $foo)
+        (return (ref.null))
+      )
+      (catch
+        (drop (exnref.pop))
+        (call $foo)
+        (call $foo)
+        (call $foo)
+        (call $foo)
+        (return (ref.null))
+      )
+    )
+    (ref.null)
+  )
+
+  (func $try-call-optimize-expression-tails (local $exn exnref)
+    (block $x
+      (try
+        (do
+          ;; Expressions that can throw should NOT be taken out of 'try' scope.
+          (local.set $exn (exnref.pop))
+          (call $foo)
+          (call $foo)
+          (call $foo)
+          (br $x)
+        )
+        (catch
+          (local.set $exn (exnref.pop))
+          (call $foo)
+          (call $foo)
+          (call $foo)
+          (br $x)
+        )
+      )
+      (unreachable)
+    )
+  )
 )