6 files changed, 46 insertions, 12 deletions

diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
index c94867032..96edb4c21 100644
--- a/src/binaryen-c.cpp
+++ b/src/binaryen-c.cpp
@@ -2898,6 +2898,10 @@ BinaryenSideEffects BinaryenSideEffectIsAtomic(void) {
 BinaryenSideEffects BinaryenSideEffectThrows(void) {
   return static_cast<BinaryenSideEffects>(EffectAnalyzer::SideEffects::Throws);
 }
+BinaryenSideEffects BinaryenSideEffectDanglingPop(void) {
+  return static_cast<BinaryenSideEffects>(
+    EffectAnalyzer::SideEffects::DanglingPop);
+}
 BinaryenSideEffects BinaryenSideEffectAny(void) {
   return static_cast<BinaryenSideEffects>(EffectAnalyzer::SideEffects::Any);
 }
diff --git a/src/binaryen-c.h b/src/binaryen-c.h
index 53fe69b92..9fd08f968 100644
--- a/src/binaryen-c.h
+++ b/src/binaryen-c.h
@@ -1580,6 +1580,7 @@ BINARYEN_API BinaryenSideEffects BinaryenSideEffectWritesMemory(void);
 BINARYEN_API BinaryenSideEffects BinaryenSideEffectImplicitTrap(void);
 BINARYEN_API BinaryenSideEffects BinaryenSideEffectIsAtomic(void);
 BINARYEN_API BinaryenSideEffects BinaryenSideEffectThrows(void);
+BINARYEN_API BinaryenSideEffects BinaryenSideEffectDanglingPop(void);
 BINARYEN_API BinaryenSideEffects BinaryenSideEffectAny(void);
 
 BINARYEN_API BinaryenSideEffects BinaryenExpressionGetSideEffects(
diff --git a/src/ir/effects.h b/src/ir/effects.h
index d2afc8133..f6c87a4e5 100644
--- a/src/ir/effects.h
+++ b/src/ir/effects.h
@@ -76,6 +76,11 @@ struct EffectAnalyzer
   // inner try, we don't mark it as 'throws', because it will be caught by an
   // inner catch.
   size_t tryDepth = 0;
+  // The nested depth of catch. This is necessary to track danglng pops.
+  size_t catchDepth = 0;
+  // If this expression contains 'exnref.pop's that are not enclosed in 'catch'
+  // body. For example, (drop (exnref.pop)) should set this to true.
+  bool danglingPop = false;
 
   static void scan(EffectAnalyzer* self, Expression** currp) {
     Expression* curr = *currp;
@@ -83,6 +88,7 @@ struct EffectAnalyzer
     // separately
     if (curr->is<Try>()) {
       self->pushTask(doVisitTry, currp);
+      self->pushTask(doEndCatch, currp);
       self->pushTask(scan, &curr->cast<Try>()->catchBody);
       self->pushTask(doStartCatch, currp);
       self->pushTask(scan, &curr->cast<Try>()->body);
@@ -100,6 +106,12 @@ struct EffectAnalyzer
   static void doStartCatch(EffectAnalyzer* self, Expression** currp) {
     assert(self->tryDepth > 0 && "try depth cannot be negative");
     self->tryDepth--;
+    self->catchDepth++;
+  }
+
+  static void doEndCatch(EffectAnalyzer* self, Expression** currp) {
+    assert(self->catchDepth > 0 && "catch depth cannot be negative");
+    self->catchDepth--;
   }
 
   // Helper functions to check for various effect types
@@ -128,7 +140,7 @@ struct EffectAnalyzer
   }
   bool hasSideEffects() const {
     return hasGlobalSideEffects() || localsWritten.size() > 0 ||
-           transfersControlFlow() || implicitTrap;
+           transfersControlFlow() || implicitTrap || danglingPop;
   }
   bool hasAnything() const {
     return hasSideEffects() || accessesLocal() || readsMemory ||
@@ -148,7 +160,8 @@ struct EffectAnalyzer
     if ((transfersControlFlow() && other.hasSideEffects()) ||
         (other.transfersControlFlow() && hasSideEffects()) ||
         ((writesMemory || calls) && other.accessesMemory()) ||
-        (accessesMemory() && (other.writesMemory || other.calls))) {
+        (accessesMemory() && (other.writesMemory || other.calls)) ||
+        (danglingPop || other.danglingPop)) {
       return true;
     }
     // All atomics are sequentially consistent for now, and ordered wrt other
@@ -203,6 +216,7 @@ struct EffectAnalyzer
     implicitTrap = implicitTrap || other.implicitTrap;
     isAtomic = isAtomic || other.isAtomic;
     throws = throws || other.throws;
+    danglingPop = danglingPop || other.danglingPop;
     for (auto i : other.localsRead) {
       localsRead.insert(i);
     }
@@ -470,7 +484,11 @@ struct EffectAnalyzer
   }
   void visitNop(Nop* curr) {}
   void visitUnreachable(Unreachable* curr) { branchesOut = true; }
-  void visitPop(Pop* curr) { calls = true; }
+  void visitPop(Pop* curr) {
+    if (catchDepth == 0) {
+      danglingPop = true;
+    }
+  }
   void visitTupleMake(TupleMake* curr) {}
   void visitTupleExtract(TupleExtract* curr) {}
 
@@ -500,7 +518,8 @@ struct EffectAnalyzer
     ImplicitTrap = 1 << 8,
     IsAtomic = 1 << 9,
     Throws = 1 << 10,
-    Any = (1 << 11) - 1
+    DanglingPop = 1 << 11,
+    Any = (1 << 12) - 1
   };
   uint32_t getSideEffects() const {
     uint32_t effects = 0;
@@ -537,6 +556,9 @@ struct EffectAnalyzer
     if (throws) {
       effects |= SideEffects::Throws;
     }
+    if (danglingPop) {
+      effects |= SideEffects::DanglingPop;
+    }
     return effects;
   }
 
diff --git a/src/js/binaryen.js-post.js b/src/js/binaryen.js-post.js
index 8a2d60a2f..2d7bd3f36 100644
--- a/src/js/binaryen.js-post.js
+++ b/src/js/binaryen.js-post.js
@@ -485,6 +485,7 @@ function initializeConstants() {
     'ImplicitTrap',
     'IsAtomic',
     'Throws',
+    'DanglingPop',
     'Any'
   ].forEach(function(name) {
     Module['SideEffects'][name] = Module['_BinaryenSideEffect' + name]();
diff --git a/src/passes/CodeFolding.cpp b/src/passes/CodeFolding.cpp
index 10255a670..04819a04d 100644
--- a/src/passes/CodeFolding.cpp
+++ b/src/passes/CodeFolding.cpp
@@ -305,12 +305,12 @@ private:
         return false;
       }
       if (getModule()->features.hasExceptionHandling()) {
+        EffectAnalyzer effects(getPassOptions(), getModule()->features, item);
         // 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()) {
+        // pseudo instruction following a catch. We cannot move expressions
+        // containing pops if they are not enclosed in a 'catch' body, because a
+        // pop instruction should follow right after 'catch'.
+        if (effects.danglingPop) {
           return false;
         }
         // When an expression can throw and it is within a try scope, taking it
@@ -321,9 +321,7 @@ private:
         // 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()) {
+        if (effects.throws && !FindAll<Try>(outOf).list.empty()) {
           return false;
         }
       }
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 2f269a00d..85c719d4c 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -411,6 +411,14 @@ struct SimplifyLocals
     if (set->isTee()) {
       return false;
     }
+    // We cannot move expressions containing exnref.pops that are not enclosed
+    // in 'catch', because 'exnref.pop' should follow right after 'catch'.
+    FeatureSet features = this->getModule()->features;
+    if (features.hasExceptionHandling() &&
+        EffectAnalyzer(this->getPassOptions(), features, set->value)
+          .danglingPop) {
+      return false;
+    }
     // if in the first cycle, or not allowing tees, then we cannot sink if >1
     // use as that would make a tee
     if ((firstCycle || !allowTee) && getCounter.num[set->index] > 1) {