[ctor-eval] Eval and store changes to globals (#4430)

This is necessary for being able to optimize real-world code, as it lets us
use the stack pointer for example. With this PR we allow changes to
globals, and we simply store the final state of the global in the global at
the end. Basically the same as we do for memory, but for globals.

Remove a test that now fails ("imported2"). Replace it with a nicer test
of saving the values of globals. Also add a test for an imported global,
which we do not allow (we never did, but I don't see a test for it).

13 files changed, 56 insertions, 66 deletions

diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp
index 009a91c0b..88b7d18b4 100644
--- a/src/tools/wasm-ctor-eval.cpp
+++ b/src/tools/wasm-ctor-eval.cpp
@@ -62,15 +62,6 @@ public:
 
   void seal() { sealed = true; }
 
-  // for equality purposes, we just care about the globals
-  // and whether they have changed
-  bool operator==(const EvallingGlobalManager& other) {
-    return globals == other.globals;
-  }
-  bool operator!=(const EvallingGlobalManager& other) {
-    return !(*this == other);
-  }
-
   Literals& operator[](Name name) {
     if (dangerousGlobals.count(name) > 0) {
       std::string extra;
@@ -110,6 +101,15 @@ public:
   }
 
   Iterator end() { return Iterator(); }
+
+  // Receives a module and applies the state of globals here into the globals
+  // in that module.
+  void applyToModule(Module& wasm) {
+    Builder builder(wasm);
+    for (const auto& [name, value] : globals) {
+      wasm.getGlobal(name)->init = builder.makeConstantExpression(value);
+    }
+  }
 };
 
 class EvallingModuleInstance
@@ -220,6 +220,8 @@ struct CtorEvalExternalInterface : EvallingModuleInstance::ExternalInterface {
     if (!memory.empty()) {
       applyMemoryToModule();
     }
+
+    instance->globals.applyToModule(*wasm);
   }
 
   void init(Module& wasm_, EvallingModuleInstance& instance_) override {
@@ -499,9 +501,6 @@ void evalCtors(Module& wasm, std::vector<std::string> ctors) {
     // TODO: if we knew priorities, we could reorder?
     for (auto& ctor : ctors) {
       std::cerr << "trying to eval " << ctor << '\n';
-      // snapshot globals (note that STACKTOP might be modified, but should
-      // be returned, so that works out)
-      auto globalsBefore = instance.globals;
       Export* ex = wasm.getExportOrNull(ctor);
       if (!ex) {
         Fatal() << "export not found: " << ctor;
@@ -514,10 +513,6 @@ void evalCtors(Module& wasm, std::vector<std::string> ctors) {
         std::cerr << "  ...stopping since could not eval: " << fail.why << "\n";
         return;
       }
-      if (instance.globals != globalsBefore) {
-        std::cerr << "  ...stopping since globals modified\n";
-        return;
-      }
       std::cerr << "  ...success on " << ctor << ".\n";
 
       // Success, the entire function was evalled! Apply the results of
diff --git a/test/ctor-eval/global-get-init.wast b/test/ctor-eval/global-get-init.wast
new file mode 100644
index 000000000..125e672d6
--- /dev/null
+++ b/test/ctor-eval/global-get-init.wast
@@ -0,0 +1,8 @@
+(module
+  (import "import" "global" (global $imported i32))
+  (func $test1 (export "test1")
+    ;; This should be safe to eval in theory, but the imported global stops us,
+    ;; so this function will not be optimized out.
+    ;; TODO: perhaps if we never use that global that is ok?
+  )
+)
diff --git a/test/ctor-eval/imported3.wast.ctors b/test/ctor-eval/global-get-init.wast.ctors
index a5bce3fd2..a5bce3fd2 100644
--- a/test/ctor-eval/imported3.wast.ctors
+++ b/test/ctor-eval/global-get-init.wast.ctors
diff --git a/test/ctor-eval/global-get-init.wast.out b/test/ctor-eval/global-get-init.wast.out
new file mode 100644
index 000000000..ad0110039
--- /dev/null
+++ b/test/ctor-eval/global-get-init.wast.out
@@ -0,0 +1,7 @@
+(module
+ (type $none_=>_none (func))
+ (export "test1" (func $test1))
+ (func $test1
+  (nop)
+ )
+)
diff --git a/test/ctor-eval/globals.wast b/test/ctor-eval/globals.wast
new file mode 100644
index 000000000..47a2023e1
--- /dev/null
+++ b/test/ctor-eval/globals.wast
@@ -0,0 +1,16 @@
+(module
+  (global $g1 (mut i32) (i32.const 10))
+  (global $g2 (mut i32) (i32.const 20))
+  (func $test1 (export "test1")
+    (global.set $g1 (i32.const 30))
+    (global.set $g2 (i32.const 40))
+    (global.set $g1 (i32.const 50)) ;; this overrides the previous 30
+  )
+  (func $keepalive (export "keepalive") (result i32)
+    ;; Keep the globals alive so we can see their values.
+    (i32.add
+      (global.get $g1)
+      (global.get $g2)
+    )
+  )
+)
diff --git a/test/ctor-eval/globals.wast.ctors b/test/ctor-eval/globals.wast.ctors
new file mode 100644
index 000000000..a5bce3fd2
--- /dev/null
+++ b/test/ctor-eval/globals.wast.ctors
@@ -0,0 +1 @@
+test1
diff --git a/test/ctor-eval/globals.wast.out b/test/ctor-eval/globals.wast.out
new file mode 100644
index 000000000..1165d9255
--- /dev/null
+++ b/test/ctor-eval/globals.wast.out
@@ -0,0 +1,12 @@
+(module
+ (type $none_=>_i32 (func (result i32)))
+ (global $g1 (mut i32) (i32.const 50))
+ (global $g2 (mut i32) (i32.const 40))
+ (export "keepalive" (func $keepalive))
+ (func $keepalive (result i32)
+  (i32.add
+   (global.get $g1)
+   (global.get $g2)
+  )
+ )
+)
diff --git a/test/ctor-eval/imported3.wast b/test/ctor-eval/imported-global.wast
index 92fda3fa0..92fda3fa0 100644
--- a/test/ctor-eval/imported3.wast
+++ b/test/ctor-eval/imported-global.wast
diff --git a/test/ctor-eval/imported-global.wast.ctors b/test/ctor-eval/imported-global.wast.ctors
new file mode 100644
index 000000000..a5bce3fd2
--- /dev/null
+++ b/test/ctor-eval/imported-global.wast.ctors
@@ -0,0 +1 @@
+test1
diff --git a/test/ctor-eval/imported3.wast.out b/test/ctor-eval/imported-global.wast.out
index e141211b1..e141211b1 100644
--- a/test/ctor-eval/imported3.wast.out
+++ b/test/ctor-eval/imported-global.wast.out
diff --git a/test/ctor-eval/imported2.wast b/test/ctor-eval/imported2.wast
deleted file mode 100644
index 579a8f383..000000000
--- a/test/ctor-eval/imported2.wast
+++ /dev/null
@@ -1,26 +0,0 @@
-(module
-  (memory 256 256)
-  (data (i32.const 10) "waka waka waka waka waka")
-  ;; stack imports are special-cased
-  (import "env" "STACKTOP" (global $STACKTOP i32))
-  (import "env" "STACK_MAX" (global $STACK_MAX i32))
-  ;; other imports must not be touched!
-  (import "env" "tempDoublePtr" (global $tempDoublePtr i32))
-  (export "test1" $test1)
-  (export "test2" $test2)
-  (export "test3" $test3)
-  ;; ok to modify a global, if we keep it the same value
-  (global $mine (mut i32) (i32.const 1))
-  (func $test1
-    (global.set $mine (i32.const 2))
-    (global.set $mine (i32.const 1)) ;; restore!
-    (i32.store8 (i32.const 12) (i32.const 115))
-  )
-  (func $test2
-    (global.set $mine (i32.const 2)) ;; embadden
-    (i32.store8 (i32.const 13) (i32.const 115))
-  )
-  (func $test3
-    (i32.store8 (i32.const 14) (i32.const 115))
-  )
-)
diff --git a/test/ctor-eval/imported2.wast.ctors b/test/ctor-eval/imported2.wast.ctors
deleted file mode 100644
index c7060ede5..000000000
--- a/test/ctor-eval/imported2.wast.ctors
+++ /dev/null
@@ -1 +0,0 @@
-test1,test2,test3
diff --git a/test/ctor-eval/imported2.wast.out b/test/ctor-eval/imported2.wast.out
deleted file mode 100644
index 6fabeadd6..000000000
--- a/test/ctor-eval/imported2.wast.out
+++ /dev/null
@@ -1,23 +0,0 @@
-(module
- (type $none_=>_none (func))
- (global $mine (mut i32) (i32.const 1))
- (memory $0 256 256)
- (data (i32.const 10) "wasa waka waka waka waka")
- (export "test2" (func $test2))
- (export "test3" (func $test3))
- (func $test2
-  (global.set $mine
-   (i32.const 2)
-  )
-  (i32.store8
-   (i32.const 13)
-   (i32.const 115)
-  )
- )
- (func $test3
-  (i32.store8
-   (i32.const 14)
-   (i32.const 115)
-  )
- )
-)