[ctor-eval] Remove stack hacks (#4429)

Remove some hackish code for fastcomp's stack handling. The stack pointer arrives
in an imported global there. Upstream does not do this, so this code is completely
unneeded these days (and, frankly, kind of scary as I read it now... it modeled the
stack as separate memory from the heap...).

Remove the tests for this as well. I verified that there was nothing else in those
tests that we need to keep.

10 files changed, 2 insertions, 186 deletions

diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp
index a44ea3ceb..f03bbec1d 100644
--- a/src/tools/wasm-ctor-eval.cpp
+++ b/src/tools/wasm-ctor-eval.cpp
@@ -112,18 +112,6 @@ public:
   Iterator end() { return Iterator(); }
 };
 
-// Use a ridiculously large stack size.
-static Index STACK_SIZE = 32 * 1024 * 1024;
-
-// Start the stack at a ridiculously large location, and do so in
-// a way that works regardless if the stack goes up or down.
-static Index STACK_START = 1024 * 1024 * 1024 + STACK_SIZE;
-
-// Bound the stack location in both directions, so we have bounds
-// that do not depend on the direction it grows.
-static Index STACK_LOWER_LIMIT = STACK_START - STACK_SIZE;
-static Index STACK_UPPER_LIMIT = STACK_START + STACK_SIZE;
-
 class EvallingModuleInstance
   : public ModuleInstanceBase<EvallingGlobalManager, EvallingModuleInstance> {
 public:
@@ -136,36 +124,12 @@ public:
     // global import, which we don't have, and is illegal to use
     ModuleUtils::iterDefinedGlobals(wasm, [&](Global* global) {
       if (!global->init->is<Const>()) {
-        // some constants are ok to use
-        if (auto* get = global->init->dynCast<GlobalGet>()) {
-          auto name = get->name;
-          auto* import = wasm.getGlobal(name);
-          if (import->module == Name(ENV) &&
-              (import->base ==
-                 STACKTOP || // stack constants are special, we handle them
-               import->base == STACK_MAX)) {
-            return; // this is fine
-          }
-        }
         // this global is dangerously initialized by an import, so if it is
         // used, we must fail
         globals.addDangerous(global->name);
       }
     });
   }
-
-  std::vector<char> stack;
-
-  // create C stack space for us to use. We do *NOT* care about their contents,
-  // assuming the stack top was unwound. the memory may have been modified,
-  // but it should not be read afterwards, doing so would be undefined behavior
-  void setupEnvironment() {
-    // prepare scratch memory
-    stack.resize(2 * STACK_SIZE);
-    // tell the module to accept writes up to the stack end
-    auto total = STACK_START + STACK_SIZE;
-    memorySize = total / Memory::kPageSize;
-  }
 };
 
 // Build an artificial `env` module based on a module's imports, so that the
@@ -207,11 +171,7 @@ std::unique_ptr<Module> buildEnvModule(Module& wasm) {
       copied->base = Name();
 
       Builder builder(*env);
-      if (global->base == STACKTOP || global->base == STACK_MAX) {
-        copied->init = builder.makeConst(STACK_START);
-      } else {
-        copied->init = builder.makeConst(Literal::makeZero(global->type));
-      }
+      copied->init = builder.makeConst(Literal::makeZero(global->type));
       env->addExport(
         builder.makeExport(global->base, copied->name, ExternalKind::Global));
     }
@@ -405,17 +365,7 @@ private:
   // TODO: handle unaligned too, see shell-interface
 
   template<typename T> T* getMemory(Address address) {
-    // if memory is on the stack, use the stack
-    if (address >= STACK_LOWER_LIMIT) {
-      if (address >= STACK_UPPER_LIMIT) {
-        throw FailToEvalException("stack usage too high");
-      }
-      Address relative = address - STACK_LOWER_LIMIT;
-      // in range, all is good, use the stack
-      return (T*)(&instance->stack[relative]);
-    }
-
-    // otherwise, this must be in the singleton segment. resize as needed
+    // this must be in the singleton segment. resize as needed
     if (wasm->memory.segments.size() == 0) {
       std::vector<char> temp;
       Builder builder(*wasm);
@@ -452,7 +402,6 @@ void evalCtors(Module& wasm, std::vector<std::string> ctors) {
   CtorEvalExternalInterface envInterface;
   auto envInstance =
     std::make_shared<EvallingModuleInstance>(*envModule, &envInterface);
-  envInstance->setupEnvironment();
 
   std::map<Name, std::shared_ptr<EvallingModuleInstance>> linkedInstances;
   linkedInstances["env"] = envInstance;
@@ -466,8 +415,6 @@ void evalCtors(Module& wasm, std::vector<std::string> ctors) {
 
     // create an instance for evalling
     EvallingModuleInstance instance(wasm, &interface, linkedInstances);
-    // set up the stack area and other environment details
-    instance.setupEnvironment();
     // we should not add new globals from here on; as a result, using
     // an imported global will fail, as it is missing and so looks new
     instance.globals.seal();
diff --git a/test/ctor-eval/imported-min.wast b/test/ctor-eval/imported-min.wast
deleted file mode 100644
index 48608d526..000000000
--- a/test/ctor-eval/imported-min.wast
+++ /dev/null
@@ -1,47 +0,0 @@
-(module
-  (memory 256 256)
-  (data (i32.const 10) "waka waka waka waka waka")
-  ;; stack imports are special
-  (import "env" "STACKTOP" (global $STACKTOP$asm2wasm$import i32))
-  (import "env" "STACK_MAX" (global $STACK_MAX$asm2wasm$import i32))
-  (import "env" "tempDoublePtr" (global $tempDoublePtr i32))
-  (global $tempDoublePtrMut (mut i32) (global.get $tempDoublePtr))
-  (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))
-  ;; stack imports are ok to use. their uses are the same as other
-  ;; globals - must keep the same value (which means, unwind the stack)
-  ;; here the global names are "minified"
-  (global $global0 (mut i32) (global.get $STACKTOP$asm2wasm$import))
-  (global $global1 (mut i32) (global.get $STACK_MAX$asm2wasm$import))
-  ;; a global initialized by an import, so bad, but ok if not used
-  (global $do-not-use (mut i32) (global.get $tempDoublePtr))
-  (func $test1
-    (local $temp i32)
-    (global.set $mine (i32.const 1))
-    (local.set $temp (global.get $global0))
-    (global.set $global0 (i32.const 1337)) ;; bad
-    (global.set $global0 (local.get $temp)) ;; save us
-    (global.set $global1 (i32.const 913370)) ;; bad
-    (global.set $global1 (local.get $temp)) ;; save us
-    ;; use the stack memory
-    (i32.store (local.get $temp) (i32.const 1337))
-    (if
-      (i32.ne
-        (i32.load (local.get $temp))
-        (i32.const 1337)
-      )
-      (unreachable) ;; they should be equal, never get here
-    )
-    ;; finally, do a valid store
-    (i32.store8 (i32.const 12) (i32.const 115))
-  )
-  (func $test2
-    (i32.store8 (i32.const 13) (i32.const 115))
-  )
-  (func $test3
-    (i32.store8 (i32.const 14) (i32.const 115))
-  )
-)
diff --git a/test/ctor-eval/imported-min.wast.ctors b/test/ctor-eval/imported-min.wast.ctors
deleted file mode 100644
index c7060ede5..000000000
--- a/test/ctor-eval/imported-min.wast.ctors
+++ /dev/null
@@ -1 +0,0 @@
-test1,test2,test3
diff --git a/test/ctor-eval/imported-min.wast.out b/test/ctor-eval/imported-min.wast.out
deleted file mode 100644
index 4427f36e0..000000000
--- a/test/ctor-eval/imported-min.wast.out
+++ /dev/null
@@ -1,2 +0,0 @@
-(module
-)
diff --git a/test/ctor-eval/imported.wast b/test/ctor-eval/imported.wast
deleted file mode 100644
index 96cf837c3..000000000
--- a/test/ctor-eval/imported.wast
+++ /dev/null
@@ -1,45 +0,0 @@
-(module
-  (memory 256 256)
-  (data (i32.const 10) "waka waka waka waka waka")
-  ;; stack imports are special
-  (import "env" "STACKTOP" (global $STACKTOP$asm2wasm$import i32))
-  (import "env" "STACK_MAX" (global $STACK_MAX$asm2wasm$import i32))
-  ;; other imports must not be touched!
-  (import "env" "tempDoublePtr" (global $tempDoublePtr i32))
-  (global $tempDoublePtrMut (mut i32) (global.get $tempDoublePtr))
-  (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))
-  ;; stack imports are ok to use. their uses are the same as other
-  ;; globals - must keep the same value (which means, unwind the stack)
-  (global $STACKTOP (mut i32) (global.get $STACKTOP$asm2wasm$import))
-  (global $STACK_MAX (mut i32) (global.get $STACK_MAX$asm2wasm$import))
-  ;; a global initialized by an import, so bad, but ok if not used
-  (global $do-not-use (mut i32) (global.get $tempDoublePtr))
-  (func $test1
-    (local $temp i32)
-    (global.set $mine (i32.const 1))
-    (local.set $temp (global.get $STACKTOP))
-    (global.set $STACKTOP (i32.const 1337)) ;; bad
-    (global.set $STACKTOP (local.get $temp)) ;; save us
-    ;; use the stack memory
-    (i32.store (local.get $temp) (i32.const 1337))
-    (if
-      (i32.ne
-        (i32.load (local.get $temp))
-        (i32.const 1337)
-      )
-      (unreachable) ;; they should be equal, never get here
-    )
-    ;; finally, do a valid store
-    (i32.store8 (i32.const 12) (i32.const 115))
-  )
-  (func $test2
-    (i32.store8 (i32.const 13) (i32.const 115))
-  )
-  (func $test3
-    (i32.store8 (i32.const 14) (i32.const 115))
-  )
-)
diff --git a/test/ctor-eval/imported.wast.ctors b/test/ctor-eval/imported.wast.ctors
deleted file mode 100644
index c7060ede5..000000000
--- a/test/ctor-eval/imported.wast.ctors
+++ /dev/null
@@ -1 +0,0 @@
-test1,test2,test3
diff --git a/test/ctor-eval/imported.wast.out b/test/ctor-eval/imported.wast.out
deleted file mode 100644
index 4427f36e0..000000000
--- a/test/ctor-eval/imported.wast.out
+++ /dev/null
@@ -1,2 +0,0 @@
-(module
-)
diff --git a/test/ctor-eval/stack-direction.wast b/test/ctor-eval/stack-direction.wast
deleted file mode 100644
index 38465557e..000000000
--- a/test/ctor-eval/stack-direction.wast
+++ /dev/null
@@ -1,30 +0,0 @@
-(module
- (type $0 (func))
- (import "env" "memory" (memory $1 256 256))
- (import "env" "STACKTOP" (global $gimport$0 i32))
- (global $global$0 (mut i32) (global.get $gimport$0))
- (export "__post_instantiate" (func $0))
- ;; if the stack goes **down**, this may seem to write to memory we care about
- (func $0 (; 0 ;) (type $0)
-  (local $0 i32)
-  (i32.store offset=12
-   (local.tee $0
-    (i32.sub
-     (global.get $global$0)
-     (i32.const 16)
-    )
-   )
-   (i32.const 10)
-  )
-  (i32.store offset=12
-   (local.get $0)
-   (i32.add
-    (i32.load offset=12
-     (local.get $0)
-    )
-    (i32.const 1)
-   )
-  )
- )
-)
-
diff --git a/test/ctor-eval/stack-direction.wast.ctors b/test/ctor-eval/stack-direction.wast.ctors
deleted file mode 100644
index a9d301395..000000000
--- a/test/ctor-eval/stack-direction.wast.ctors
+++ /dev/null
@@ -1 +0,0 @@
-__post_instantiate
diff --git a/test/ctor-eval/stack-direction.wast.out b/test/ctor-eval/stack-direction.wast.out
deleted file mode 100644
index 4427f36e0..000000000
--- a/test/ctor-eval/stack-direction.wast.out
+++ /dev/null
@@ -1,2 +0,0 @@
-(module
-)