NoExitRuntime pass: Don't assume arguments have no side effects (#2953)

This bug was present from the very first version of this pass from 2018,
but it went unnoticed until now when a large project broke on it, for
some reason after

emscripten-core/emscripten#11403

Nothing wrong in that PR, probably just luck that it started to happen
there...

3 files changed, 182 insertions, 6 deletions

diff --git a/src/passes/NoExitRuntime.cpp b/src/passes/NoExitRuntime.cpp
index 82a296cfc..deffccd36 100644
--- a/src/passes/NoExitRuntime.cpp
+++ b/src/passes/NoExitRuntime.cpp
@@ -46,7 +46,17 @@ struct NoExitRuntime : public WalkerPass<PostWalker<NoExitRuntime>> {
     }
     for (auto name : ATEXIT_NAMES) {
       if (name == import->base) {
-        replaceCurrent(Builder(*getModule()).replaceWithIdenticalType(curr));
+        // Remove the call, and drop the arguments (which may have side
+        // effects); let other passes clean that up more.
+        Builder builder(*getModule());
+        std::vector<Expression*> args;
+        for (auto* operand : curr->operands) {
+          args.push_back(builder.makeDrop(operand));
+        }
+        // Ensure the block has the right type using the last arg.
+        args.push_back(builder.replaceWithIdenticalType(curr));
+        replaceCurrent(builder.makeBlock(args));
+        break;
       }
     }
   }
diff --git a/test/passes/no-exit-runtime.txt b/test/passes/no-exit-runtime.txt
index 126f997b0..6f367dacc 100644
--- a/test/passes/no-exit-runtime.txt
+++ b/test/passes/no-exit-runtime.txt
@@ -1,6 +1,7 @@
 (module
  (type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
  (type $none_=>_none (func))
+ (type $none_=>_i32 (func (result i32)))
  (import "env" "atexit" (func $fimport$0 (param i32 i32) (result i32)))
  (import "env" "__cxa_atexit" (func $fimport$1 (param i32 i32) (result i32)))
  (import "env" "_atexit" (func $fimport$2 (param i32 i32) (result i32)))
@@ -8,16 +9,48 @@
  (import "env" "other" (func $fimport$4 (param i32 i32) (result i32)))
  (func $caller
   (drop
-   (i32.const 0)
+   (block (result i32)
+    (drop
+     (i32.const 0)
+    )
+    (drop
+     (i32.const 1)
+    )
+    (i32.const 0)
+   )
   )
   (drop
-   (i32.const 0)
+   (block (result i32)
+    (drop
+     (i32.const 0)
+    )
+    (drop
+     (i32.const 1)
+    )
+    (i32.const 0)
+   )
   )
   (drop
-   (i32.const 0)
+   (block (result i32)
+    (drop
+     (i32.const 0)
+    )
+    (drop
+     (i32.const 1)
+    )
+    (i32.const 0)
+   )
   )
   (drop
-   (i32.const 0)
+   (block (result i32)
+    (drop
+     (i32.const 0)
+    )
+    (drop
+     (i32.const 1)
+    )
+    (i32.const 0)
+   )
   )
   (drop
    (call $fimport$4
@@ -26,7 +59,108 @@
    )
   )
   (drop
-   (unreachable)
+   (block
+    (drop
+     (unreachable)
+    )
+    (drop
+     (i32.const 1)
+    )
+    (unreachable)
+   )
+  )
+ )
+ (func $side-effects (result i32)
+  (local $x i32)
+  (drop
+   (block (result i32)
+    (drop
+     (local.tee $x
+      (i32.const 1)
+     )
+    )
+    (drop
+     (i32.const 2)
+    )
+    (i32.const 0)
+   )
+  )
+  (drop
+   (block (result i32)
+    (drop
+     (i32.const 3)
+    )
+    (drop
+     (local.tee $x
+      (i32.const 4)
+     )
+    )
+    (i32.const 0)
+   )
+  )
+  (drop
+   (block (result i32)
+    (drop
+     (local.tee $x
+      (i32.const 5)
+     )
+    )
+    (drop
+     (local.tee $x
+      (i32.const 6)
+     )
+    )
+    (i32.const 0)
+   )
+  )
+  (drop
+   (block
+    (drop
+     (unreachable)
+    )
+    (drop
+     (local.tee $x
+      (i32.const 7)
+     )
+    )
+    (unreachable)
+   )
+  )
+  (drop
+   (block
+    (drop
+     (local.tee $x
+      (i32.const 8)
+     )
+    )
+    (drop
+     (unreachable)
+    )
+    (unreachable)
+   )
+  )
+  (drop
+   (block
+    (drop
+     (unreachable)
+    )
+    (drop
+     (i32.const 9)
+    )
+    (unreachable)
+   )
+  )
+  (drop
+   (block
+    (drop
+     (i32.const 10)
+    )
+    (drop
+     (unreachable)
+    )
+    (unreachable)
+   )
   )
+  (local.get $x)
  )
 )
diff --git a/test/passes/no-exit-runtime.wast b/test/passes/no-exit-runtime.wast
index 1afe5ef8d..15966e1db 100644
--- a/test/passes/no-exit-runtime.wast
+++ b/test/passes/no-exit-runtime.wast
@@ -12,4 +12,36 @@
     (drop (call $fimport$4 (i32.const 0) (i32.const 1)))
     (drop (call $fimport$0 (unreachable) (i32.const 1)))
   )
+  (func $side-effects (result i32)
+    (local $x i32)
+    (drop (call $fimport$0
+      (local.tee $x (i32.const 1))
+      (i32.const 2)
+    ))
+    (drop (call $fimport$0
+      (i32.const 3)
+      (local.tee $x (i32.const 4))
+    ))
+    (drop (call $fimport$0
+      (local.tee $x (i32.const 5))
+      (local.tee $x (i32.const 6))
+    ))
+    (drop (call $fimport$0
+      (unreachable)
+      (local.tee $x (i32.const 7))
+    ))
+    (drop (call $fimport$0
+      (local.tee $x (i32.const 8))
+      (unreachable)
+    ))
+    (drop (call $fimport$0
+      (unreachable)
+      (i32.const 9)
+    ))
+    (drop (call $fimport$0
+      (i32.const 10)
+      (unreachable)
+    ))
+    (local.get $x)
+  )
 )