Followups with trap to remove ExternalInterface

stevenfontanella · stevenfontanella · commit bdde2f344750 · 2026-03-12T20:43:25.000Z
diff --git a/src/ir/runtime-memory.cpp b/src/ir/runtime-memory.cpp
@@ -17,17 +17,18 @@
 #include "ir/runtime-memory.h"
 #include "fp16.h"
 #include "interpreter/exception.h"
+#include <iostream>
 
 namespace wasm {
 
-RuntimeMemory::RuntimeMemory(Memory memory,
-                             ExternalInterface* externalInterface)
-  : externalInterface(externalInterface), memoryDefinition(std::move(memory)) {}
-
 namespace {
 
-Address getFinalAddress(const RuntimeMemory& runtimeMemory,
-                        Address addr,
+[[noreturn]] void trap(std::string_view reason) {
+  std::cout << "[trap " << reason << "]\n";
+  throw TrapException{};
+}
+
+Address getFinalAddress(Address addr,
                         Address offset,
                         Index bytes,
                         Address memorySizeBytes) {
@@ -36,14 +37,14 @@ Address getFinalAddress(const RuntimeMemory& runtimeMemory,
     msg += std::to_string(uint64_t(offset));
     msg += " > ";
     msg += std::to_string(uint64_t(memorySizeBytes));
-    runtimeMemory.trap(msg);
+    trap(msg);
   }
   if (addr > memorySizeBytes - offset) {
     std::string msg = "final > memory: ";
     msg += std::to_string(uint64_t(addr));
     msg += " > ";
     msg += std::to_string(uint64_t(memorySizeBytes - offset));
-    runtimeMemory.trap(msg);
+    trap(msg);
   }
 
   addr = size_t(addr) + offset;
@@ -55,13 +56,12 @@ Address getFinalAddress(const RuntimeMemory& runtimeMemory,
     msg += std::to_string(uint64_t(bytes));
     msg += " > ";
     msg += std::to_string(uint64_t(memorySizeBytes));
-    runtimeMemory.trap(msg);
+    trap(msg);
   }
   return addr;
 }
 
-void checkLoadAddress(const RuntimeMemory& runtimeMemory,
-                      Address addr,
+void checkLoadAddress(Address addr,
                       Index bytes,
                       Address memorySizeBytes) {
   if (addr > memorySizeBytes || bytes > memorySizeBytes - addr) {
@@ -71,19 +71,18 @@ void checkLoadAddress(const RuntimeMemory& runtimeMemory,
     msg += std::to_string(uint64_t(bytes));
     msg += " > ";
     msg += std::to_string(uint64_t(memorySizeBytes));
-    runtimeMemory.trap(msg);
+    trap(msg);
   }
 }
 
-void checkAtomicAddress(const RuntimeMemory& runtimeMemory,
-                        Address addr,
+void checkAtomicAddress(Address addr,
                         Index bytes,
                         Address memorySizeBytes) {
-  checkLoadAddress(runtimeMemory, addr, bytes, memorySizeBytes);
+  checkLoadAddress(addr, bytes, memorySizeBytes);
   // Unaligned atomics trap.
   if (bytes > 1) {
     if (addr & (bytes - 1)) {
-      runtimeMemory.trap("unaligned atomic operation");
+      trap("unaligned atomic operation");
     }
   }
 }
@@ -95,9 +94,8 @@ template<typename T> bool aligned(const uint8_t* address) {
 
 } // namespace
 
-RealRuntimeMemory::RealRuntimeMemory(Memory memory,
-                                     ExternalInterface* externalInterface)
-  : RuntimeMemory(std::move(memory), externalInterface) {
+RealRuntimeMemory::RealRuntimeMemory(Memory memory)
+  : RuntimeMemory(std::move(memory)) {
   resize(memoryDefinition.initialByteSize());
 }
 
@@ -107,9 +105,9 @@ Literal RealRuntimeMemory::load(Address addr,
                                 MemoryOrder order,
                                 Type type,
                                 bool signed_) const {
-  Address final = getFinalAddress(*this, addr, offset, byteCount, size());
+  Address final = getFinalAddress(addr, offset, byteCount, size());
   if (order != MemoryOrder::Unordered) {
-    checkAtomicAddress(*this, final, byteCount, size());
+    checkAtomicAddress(final, byteCount, size());
   }
   switch (type.getBasic()) {
     case Type::i32: {
@@ -170,9 +168,9 @@ void RealRuntimeMemory::store(Address addr,
                               MemoryOrder order,
                               Literal value,
                               Type type) {
-  Address final = getFinalAddress(*this, addr, offset, byteCount, size());
+  Address final = getFinalAddress(addr, offset, byteCount, size());
   if (order != MemoryOrder::Unordered) {
-    checkAtomicAddress(*this, final, byteCount, size());
+    checkAtomicAddress(final, byteCount, size());
   }
   switch (type.getBasic()) {
     case Type::i32: {
@@ -213,9 +211,9 @@ void RealRuntimeMemory::store(Address addr,
     case Type::f32: {
       switch (byteCount) {
         case 2:
-          set<uint16_t>(
-            final,
-            fp16_ieee_from_fp32_value(bit_cast<float>(value.reinterpreti32())));
+          set<uint16_t>(final,
+                        fp16_ieee_from_fp32_value(
+                          bit_cast<float>(value.reinterpreti32())));
           break;
         case 4:
           set<int32_t>(final, value.reinterpreti32());
@@ -261,7 +259,7 @@ void RealRuntimeMemory::init(Address dest,
   if (src > data->data.size() || byteCount > data->data.size() - src) {
     trap("out of bounds segment access in memory.init");
   }
-  Address final = getFinalAddress(*this, dest, 0, byteCount, size());
+  Address final = getFinalAddress(dest, 0, byteCount, size());
   if (byteCount > 0) {
     std::memcpy(&memory[final], &data->data[src], byteCount);
   }
@@ -271,9 +269,8 @@ void RealRuntimeMemory::copy(Address dest,
                              Address src,
                              Address byteCount,
                              const RuntimeMemory* srcMemory) {
-  Address finalDest = getFinalAddress(*this, dest, 0, byteCount, size());
-  Address finalSrc =
-    getFinalAddress(*srcMemory, src, 0, byteCount, srcMemory->size());
+  Address finalDest = getFinalAddress(dest, 0, byteCount, size());
+  Address finalSrc = getFinalAddress(src, 0, byteCount, srcMemory->size());
   const std::vector<uint8_t>* srcBuffer = srcMemory->getBuffer();
   if (!srcBuffer) {
     // If it's not a memory with a direct buffer, we might need another way to
@@ -287,7 +284,7 @@ void RealRuntimeMemory::copy(Address dest,
 }
 
 void RealRuntimeMemory::fill(Address dest, uint8_t value, Address byteCount) {
-  Address final = getFinalAddress(*this, dest, 0, byteCount, size());
+  Address final = getFinalAddress(dest, 0, byteCount, size());
   if (byteCount > 0) {
     std::memset(&memory[final], value, byteCount);
   }
@@ -300,8 +297,7 @@ void RealRuntimeMemory::resize(size_t newSize) {
   size_t newAllocatedSize = std::max(minSize, newSize);
   if (newAllocatedSize > oldAllocatedSize) {
     memory.resize(newAllocatedSize);
-    std::memset(
-      &memory[oldAllocatedSize], 0, newAllocatedSize - oldAllocatedSize);
+    std::memset(&memory[oldAllocatedSize], 0, newAllocatedSize - oldAllocatedSize);
   }
   if (newSize < oldAllocatedSize && newSize < minSize) {
     std::memset(&memory[newSize], 0, minSize - newSize);
@@ -347,7 +343,6 @@ template void RealRuntimeMemory::set<uint32_t>(size_t, uint32_t);
 template void RealRuntimeMemory::set<int64_t>(size_t, int64_t);
 template void RealRuntimeMemory::set<uint64_t>(size_t, uint64_t);
 template void
-RealRuntimeMemory::set<std::array<uint8_t, 16>>(size_t,
-                                                std::array<uint8_t, 16>);
+RealRuntimeMemory::set<std::array<uint8_t, 16>>(size_t, std::array<uint8_t, 16>);
 
 } // namespace wasm
diff --git a/src/ir/runtime-memory.h b/src/ir/runtime-memory.h
@@ -23,13 +23,7 @@ namespace wasm {
 
 class RuntimeMemory {
 public:
-  // Forward declare to avoid circular dependency
-  struct ExternalInterface {
-    virtual ~ExternalInterface() = default;
-    virtual void trap(std::string_view why) = 0;
-  };
-
-  RuntimeMemory(Memory memory, ExternalInterface* externalInterface);
+  RuntimeMemory(Memory memory) : memoryDefinition(std::move(memory)) {}
   virtual ~RuntimeMemory() = default;
 
   virtual Literal load(Address addr,
@@ -50,32 +44,26 @@ class RuntimeMemory {
 
   virtual Address size() const = 0;
 
-  virtual void init(Address dest,
-                    Address src,
-                    Address byteCount,
-                    const DataSegment* data) = 0;
+  virtual void
+  init(Address dest, Address src, Address byteCount, const DataSegment* data) = 0;
 
-  virtual void copy(Address dest,
-                    Address src,
-                    Address byteCount,
-                    const RuntimeMemory* srcMemory) = 0;
+  virtual void
+  copy(Address dest, Address src, Address byteCount, const RuntimeMemory* srcMemory) = 0;
 
   virtual void fill(Address dest, uint8_t value, Address byteCount) = 0;
 
   const Memory* getDefinition() const { return &memoryDefinition; }
 
   virtual const std::vector<uint8_t>* getBuffer() const { return nullptr; }
 
-  void trap(std::string_view why) const { externalInterface->trap(why); }
-
 protected:
-  ExternalInterface* externalInterface;
   const Memory memoryDefinition;
 };
 
 class RealRuntimeMemory : public RuntimeMemory {
 public:
-  RealRuntimeMemory(Memory memory, ExternalInterface* externalInterface);
+  RealRuntimeMemory(Memory memory);
+
   virtual ~RealRuntimeMemory() = default;
 
   Literal load(Address addr,
diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp
@@ -42,6 +42,7 @@
 #include "wasm-interpreter.h"
 #include "wasm-io.h"
 #include "wasm-validator.h"
+#include "interpreter/exception.h"
 
 using namespace wasm;
 
@@ -242,9 +243,8 @@ class EvallingModuleRunner : public ModuleRunnerBase<EvallingModuleRunner> {
             this->wasm,
             [this](Name name, Type type) { return makeFuncData(name, type); });
         },
-        [](Memory memory, ExternalInterface* externalInterface) {
-          return std::make_unique<CtorEvalRuntimeMemory>(memory,
-                                                         externalInterface);
+        [](Memory memory) {
+          return std::make_unique<CtorEvalRuntimeMemory>(memory);
         }) {}
 
   Flow visitGlobalGet(GlobalGet* curr) {
@@ -1117,6 +1117,12 @@ EvalCtorOutcome evalCtor(EvallingModuleRunner& instance,
       Flow flow;
       try {
         flow = instance.visit(curr);
+      } catch (TrapException&) {
+        throw FailToEvalException("trap");
+      } catch (WasmException& exn) {
+        std::stringstream ss;
+        ss << "exception thrown: " << exn;
+        throw FailToEvalException(ss.str());
       } catch (FailToEvalException& fail) {
         if (!quiet) {
           if (successes == 0) {
@@ -1353,7 +1359,15 @@ void evalCtors(Module& wasm,
     // create an instance for evalling
     EvallingModuleRunner instance(
       wasm, &interface, interface.instanceInitialized, linkedInstances);
-    instance.instantiate();
+    try {
+      instance.instantiate();
+    } catch (TrapException&) {
+      throw FailToEvalException("trap");
+    } catch (WasmException& exn) {
+      std::stringstream ss;
+      ss << "exception thrown: " << exn;
+      throw FailToEvalException(ss.str());
+    }
     interface.instanceInitialized = true;
     // go one by one, in order, until we fail
     // TODO: if we knew priorities, we could reorder?
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
@@ -3003,7 +3003,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
   // ExternalInterface provides embedding-specific functionality like calling
   // an imported function or accessing memory.
   //
-  struct ExternalInterface : RuntimeMemory::ExternalInterface {
+  struct ExternalInterface {
     ExternalInterface(
       std::map<Name, std::shared_ptr<SubType>> linkedInstances = {}) {}
     virtual ~ExternalInterface() = default;
@@ -3035,8 +3035,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
   std::unordered_map<Name, RuntimeMemory*> allMemories;
 
   using CreateTableFunc = std::unique_ptr<RuntimeTable>(Literal, Table);
-  using CreateMemoryFunc = std::unique_ptr<RuntimeMemory>(Memory,
-                                                          ExternalInterface*);
+  using CreateMemoryFunc = std::unique_ptr<RuntimeMemory>(Memory);
 
   ModuleRunnerBase(
     Module& wasm,
@@ -3056,13 +3055,14 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
               [](Literal initial, Table t) -> std::unique_ptr<RuntimeTable> {
                 return std::make_unique<RealRuntimeTable>(initial, t);
               })),
-      createMemory(createMemory != nullptr
-                     ? std::move(createMemory)
-                     : static_cast<std::function<CreateMemoryFunc>>(
-                         [externalInterface](Memory m, ExternalInterface* ei)
-                           -> std::unique_ptr<RuntimeMemory> {
-                           return std::make_unique<RealRuntimeMemory>(m, ei);
-                         })) {
+      createMemory(
+        createMemory != nullptr
+          ? std::move(createMemory)
+          : static_cast<std::function<CreateMemoryFunc>>(
+              [](Memory m) -> std::unique_ptr<RuntimeMemory> {
+                return std::make_unique<RealRuntimeMemory>(m);
+              })) {
+
     // Set up a single shared CurrContinuations for all these linked instances,
     // reusing one if it exists.
     std::shared_ptr<ContinuationStore> shared;
@@ -3468,8 +3468,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
         // parsing/validation checked this already.
         assert(inserted && "Unexpected repeated memory name");
       } else {
-        auto& runtimeMemory = definedMemories.emplace_back(
-          createMemory(*memory, externalInterface));
+        auto& runtimeMemory =
+          definedMemories.emplace_back(createMemory(*memory));
         [[maybe_unused]] auto [_, inserted] =
           allMemories.try_emplace(memory->name, runtimeMemory.get());
         assert(inserted && "Unexpected repeated memory name");
