clean up ImportUtils: make getImport return the import (more consistent with other similar APIs) and fix some ctor-evalling handling of imports, which was incorrect - we need to create fake globals when importing globals, not later, which is too late for initialized globals from imports (#1226)

3 files changed, 50 insertions, 31 deletions

diff --git a/src/ast/import-utils.h b/src/ast/import-utils.h
index d12c23182..ff7ca3b83 100644
--- a/src/ast/import-utils.h
+++ b/src/ast/import-utils.h
@@ -25,13 +25,13 @@ namespace wasm {
 namespace ImportUtils {
   // find an import by the module.base that is being imported.
   // return the internal name
-  inline Name getImport(Module& wasm, Name module, Name base) {
+  inline Import* getImport(Module& wasm, Name module, Name base) {
     for (auto& import : wasm.imports) {
       if (import->module == module && import->base == base) {
-        return import->name;
+        return import.get();
       }
     }
-    return Name();
+    return nullptr;
   }
 };
 
diff --git a/src/passes/SafeHeap.cpp b/src/passes/SafeHeap.cpp
index ebaf42358..784bc761d 100644
--- a/src/passes/SafeHeap.cpp
+++ b/src/passes/SafeHeap.cpp
@@ -114,8 +114,9 @@ struct SafeHeap : public Pass {
 
   void addImports(Module* module) {
     // imports
-    dynamicTopPtr = ImportUtils::getImport(*module, ENV, DYNAMICTOP_PTR_IMPORT);
-    if (!dynamicTopPtr.is()) {
+    if (auto* existing = ImportUtils::getImport(*module, ENV, DYNAMICTOP_PTR_IMPORT)) {
+      dynamicTopPtr = existing->name;
+    } else {
       auto* import = new Import;
       import->name = dynamicTopPtr = DYNAMICTOP_PTR_IMPORT;
       import->module = ENV;
@@ -124,8 +125,9 @@ struct SafeHeap : public Pass {
       import->globalType = i32;
       module->addImport(import);
     }
-    segfault = ImportUtils::getImport(*module, ENV, SEGFAULT_IMPORT);
-    if (!segfault.is()) {
+    if (auto* existing = ImportUtils::getImport(*module, ENV, SEGFAULT_IMPORT)) {
+      dynamicTopPtr = existing->name;
+    } else {
       auto* import = new Import;
       import->name = segfault = SEGFAULT_IMPORT;
       import->module = ENV;
@@ -134,8 +136,9 @@ struct SafeHeap : public Pass {
       import->functionType = ensureFunctionType("v", module)->name;
       module->addImport(import);
     }
-    alignfault = ImportUtils::getImport(*module, ENV, ALIGNFAULT_IMPORT);
-    if (!alignfault.is()) {
+    if (auto* existing = ImportUtils::getImport(*module, ENV, ALIGNFAULT_IMPORT)) {
+      dynamicTopPtr = existing->name;
+    } else {
       auto* import = new Import;
       import->name = alignfault = ALIGNFAULT_IMPORT;
       import->module = ENV;
diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp
index 1d08f5baf..aea0646a3 100644
--- a/src/tools/wasm-ctor-eval.cpp
+++ b/src/tools/wasm-ctor-eval.cpp
@@ -33,6 +33,8 @@
 #include "wasm-builder.h"
 #include "ast/memory-utils.h"
 #include "ast/global-utils.h"
+#include "ast/import-utils.h"
+#include "ast/literal-utils.h"
 
 using namespace wasm;
 
@@ -78,11 +80,6 @@ public:
       }
       throw FailToEvalException(std::string("tried to access a dangerous (import-initialized) global: ") + name.str + extra);
     }
-    if (sealed) {
-      if (globals.find(name) == globals.end()) {
-        throw FailToEvalException(std::string("tried to access missing global: ") + name.str);
-      }
-    }
     return globals[name];
   }
 
@@ -114,6 +111,13 @@ public:
   }
 };
 
+enum {
+  // put the stack in some ridiculously high location
+  STACK_START = 0x40000000,
+  // use a ridiculously large stack size
+  STACK_SIZE = 32 * 1024 * 1024
+};
+
 class EvallingModuleInstance : public ModuleInstanceBase<EvallingGlobalManager, EvallingModuleInstance> {
 public:
   EvallingModuleInstance(Module& wasm, ExternalInterface* externalInterface) : ModuleInstanceBase(wasm, externalInterface) {
@@ -138,13 +142,6 @@ public:
     }
   }
 
-  enum {
-    // put the stack in some ridiculously high location
-    STACK_START = 0x40000000,
-    // use a ridiculously large stack size
-    STACK_SIZE = 32 * 1024 * 1024
-  };
-
   std::vector<char> stack;
 
   // create C stack space for us to use. We do *NOT* care about their contents,
@@ -153,13 +150,6 @@ public:
   void setupEnvironment() {
     // prepare scratch memory
     stack.resize(STACK_SIZE);
-    // fill usable values for stack imports
-    if (auto* stackTop = GlobalUtils::getGlobalInitializedToImport(wasm, "env", "STACKTOP")) {
-      globals[stackTop->name] = Literal(int32_t(STACK_START));
-    }
-    if (auto* stackMax = GlobalUtils::getGlobalInitializedToImport(wasm, "env", "STACK_MAX")) {
-      globals[stackMax->name] = Literal(int32_t(STACK_START));
-    }
     // tell the module to accept writes up to the stack end
     auto total = STACK_START + STACK_SIZE;
     memorySize = total / Memory::kPageSize;
@@ -181,6 +171,32 @@ struct CtorEvalExternalInterface : EvallingModuleInstance::ExternalInterface {
   }
 
   void importGlobals(EvallingGlobalManager& globals, Module& wasm_) override {
+    // fill usable values for stack imports, and globals initialized to them
+    if (auto* stackTop = ImportUtils::getImport(wasm_, "env", "STACKTOP")) {
+      globals[stackTop->name] = Literal(int32_t(STACK_START));
+      if (auto* stackTop = GlobalUtils::getGlobalInitializedToImport(wasm_, "env", "STACKTOP")) {
+        globals[stackTop->name] = Literal(int32_t(STACK_START));
+      }
+    }
+    if (auto* stackMax = ImportUtils::getImport(wasm_, "env", "STACK_MAX")) {
+      globals[stackMax->name] = Literal(int32_t(STACK_START));
+      if (auto* stackMax = GlobalUtils::getGlobalInitializedToImport(wasm_, "env", "STACK_MAX")) {
+        globals[stackMax->name] = Literal(int32_t(STACK_START));
+      }
+    }
+    // fill in fake values for everything else, which is dangerous to use
+    for (auto& global : wasm_.globals) {
+      if (globals.find(global->name) == globals.end()) {
+        globals[global->name] = LiteralUtils::makeLiteralZero(global->type);
+      }
+    }
+    for (auto& import : wasm_.imports) {
+      if (import->kind == ExternalKind::Global) {
+        if (globals.find(import->name) == globals.end()) {
+          globals[import->name] = LiteralUtils::makeLiteralZero(import->globalType);
+        }
+      }
+    }
   }
 
   Literal callImport(Import *import, LiteralList& arguments) override {
@@ -248,9 +264,9 @@ private:
   template <typename T>
   T* getMemory(Address address) {
     // if memory is on the stack, use the stack
-    if (address >= instance->STACK_START) {
-      Address relative = address - instance->STACK_START;
-      if (relative + sizeof(T) > instance->STACK_SIZE) {
+    if (address >= STACK_START) {
+      Address relative = address - STACK_START;
+      if (relative + sizeof(T) > STACK_SIZE) {
         throw FailToEvalException("stack usage too high");
       }
       // in range, all is good, use the stack