Add validations for imports during instantiation

scripts/test/shared.py

@@ -440,11 +440,10 @@ def get_tests(test_dir, extensions=[], recursive=False):
     'func.wast',    # Duplicate parameter names not properly rejected
     'global.wast',  # Fail to parse table
     'if.wast',      # Requires more precise unreachable validation
-    'imports.wast',  # Missing validation of missing function on instantiation
+    # 'imports.wast',  # Missing validation of missing function on instantiation
     'proposals/threads/imports.wast',  # Missing memory type validation on instantiation
     'linking.wast',  # Missing function type validation on instantiation
     'proposals/threads/memory.wast',  # Missing memory type validation on instantiation
-    'memory64-imports.wast',  # Missing validation on instantiation
     'annotations.wast',  # String annotations IDs should be allowed
     'id.wast',       # Empty IDs should be disallowed
     # Requires correct handling of tag imports from different instances of the same module,
@@ -473,12 +472,8 @@ def get_tests(test_dir, extensions=[], recursive=False):
     'type-rec.wast',  # Missing function type validation on instantiation
     'type-subtyping.wast',  # ShellExternalInterface::callTable does not handle subtyping
     'call_indirect.wast',   # Bug with 64-bit inline element segment parsing
-    'memory64.wast',        # Requires validations for memory size
-    'imports0.wast',        # Missing memory type validation on instantiation
-    'imports2.wast',        # Missing memory type validation on instantiation
-    'imports3.wast',        # Missing memory type validation on instantiation
-    'linking0.wast',        # Missing memory type validation on instantiation
-    'linking3.wast',        # Fatal error on missing table.
+    # 'memory64.wast',        # Requires wast `module definition` support
+    # 'linking0.wast',        # Missing memory type validation on instantiation
     'i16x8_relaxed_q15mulr_s.wast',  # Requires wast `either` support
     'i32x4_relaxed_trunc.wast',      # Requires wast `either` support
     'i8x16_relaxed_swizzle.wast',    # Requires wast `either` support

src/passes/Print.cpp

@@ -3940,4 +3940,10 @@ std::ostream& operator<<(std::ostream& o, wasm::ModuleHeapType pair) {
   return o << "(unnamed)";
 }
 
+std::ostream& operator<<(std::ostream& o, const wasm::Memory& memory) {
+  wasm::PrintSExpression printer(o);
+  printer.visitMemory(const_cast<wasm::Memory*>(&memory));
+  return o;
+}
+
 } // namespace std

src/shell-interface.h

@@ -134,8 +134,9 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
       auto inst = getImportInstance(import);
       auto* exportedGlobal = inst->wasm.getExportOrNull(import->base);
       if (!exportedGlobal || exportedGlobal->kind != ExternalKind::Global) {
-        Fatal() << "importGlobals: unknown import: " << import->module.str
-                << "." << import->name.str;
+        trap((std::stringstream() << "unknown import: " << import->module.str
+                                  << "." << import->name.str)
+               .str());
       }
       globals[import->name] = inst->globals[*exportedGlobal->getInternalName()];
     });
@@ -330,6 +331,11 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
     throw TrapException();
   }
 
+  void trap(std::string_view why) override {
+    std::cout << "[trap " << why << "]\n";
+    throw TrapException();
+  }
+
   void hostLimit(const char* why) override {
     std::cout << "[host limit " << why << "]\n";
     throw HostLimitException();

src/wasm-interpreter.h

@@ -86,7 +86,7 @@ class Flow {
   }
 
   Literals values;
-  Name breakTo; // if non-null, a break is going on
+  Name breakTo;              // if non-null, a break is going on
   Tag* suspendTag = nullptr; // if non-null, breakTo must be SUSPEND_FLOW, and
                              // this is the tag being suspended
 
@@ -2658,6 +2658,8 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
 
   virtual void trap(const char* why) { WASM_UNREACHABLE("unimp"); }
 
+  virtual void trap(std::string_view why) { WASM_UNREACHABLE("unimp"); }
+
   virtual void hostLimit(const char* why) { WASM_UNREACHABLE("unimp"); }
 
   virtual void throwException(const WasmException& exn) {
@@ -2937,6 +2939,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
                            Index oldSize,
                            Index newSize) = 0;
     virtual void trap(const char* why) = 0;
+    virtual void trap(std::string_view why) { trap(std::string(why).c_str()); }
     virtual void hostLimit(const char* why) = 0;
     virtual void throwException(const WasmException& exn) = 0;
     // Get the Tag instance for a tag implemented in the host, that is, not
@@ -3178,6 +3181,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
     // initialize the rest of the external interface
     externalInterface->init(wasm, *self());
 
+    validateImports();
+
     initializeTableContents();
     initializeMemoryContents();
 
@@ -3269,6 +3274,70 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
     Name name;
   };
 
+  // Trap if types don't match between all imports and their corresponding
+  // exports. Imported memories and tables must also be a subtype of their
+  // export.
+  void validateImports() {
+    ModuleUtils::iterImportable(
+      wasm,
+      [this](ExternalKind kind,
+             std::variant<Function*, Memory*, Tag*, Global*, Table*> import) {
+        Importable* importable = std::visit(
+          [](const auto& import) -> Importable* { return import; }, import);
+
+        SubType* importedInstance =
+          linkedInstances.at(importable->module).get();
+        Export* export_ =
+          importedInstance->wasm.getExportOrNull(importable->base);
+
+        // In case functions are imported from the special "spectest" module,
+        // don't check them here, since they won't show up in exports.
+        if (!export_ && kind != ExternalKind::Function) {
+          std::cerr << "importedinstance " << importedInstance
+                    << ". Importable: " << importable->module << " "
+                    << importable->base << "\n";
+          trap((std::stringstream()
+                << "Export " << importable->base << " doesn't exist.")
+                 .str());
+        }
+        if (export_ && export_->kind != kind) {
+          trap("Exported kind doesn't match");
+        }
+
+        if (auto** memory = std::get_if<Memory*>(&import)) {
+          SubType* importedInstance =
+            linkedInstances.at((*memory)->module).get();
+          Export* export_ =
+            importedInstance->wasm.getExportOrNull((*memory)->base);
+          Memory exportedMemory =
+            *importedInstance->wasm.getMemory(*export_->getInternalName());
+          exportedMemory.initial =
+            importedInstance->getMemorySize(*export_->getInternalName());
+
+          // todo which way?
+          // if (!(*memory)->isSubType(exportedMemory)) {
+          if (!exportedMemory.isSubType(**memory)) {
+            trap((std::stringstream()
+                  << "Imported memory isn't compatible. Imported memory: "
+                  << **memory << ". Exported memory: " << exportedMemory)
+                   .str());
+          }
+        }
+
+        if (auto** table = std::get_if<Table*>(&import)) {
+          SubType* importedInstance =
+            linkedInstances.at((*table)->module).get();
+          Export* export_ =
+            importedInstance->wasm.getExportOrNull((*table)->base);
+          Table* exportedTable =
+            importedInstance->wasm.getTable(*export_->getInternalName());
+          if (!(*table)->isSubType(*exportedTable)) {
+            trap("Imported table isn't compatible");
+          }
+        }
+      });
+  }
+
   TableInstanceInfo getTableInstanceInfo(Name name) {
     auto* table = wasm.getTable(name);
     if (table->imported()) {
@@ -4682,12 +4751,13 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
   }
   Flow visitStackSwitch(StackSwitch* curr) { return Flow(NONCONSTANT_FLOW); }
 
-  void trap(const char* why) override {
-    // Traps break all current continuations - they will never be resumable.
+  void trap(std::string_view why) override {
     self()->clearContinuationStore();
     externalInterface->trap(why);
   }
 
+  void trap(const char* why) override { trap(std::string_view(why)); }
+
   void hostLimit(const char* why) override {
     self()->clearContinuationStore();
     externalInterface->hostLimit(why);

src/wasm.h

@@ -2435,6 +2435,28 @@ class Table : public Importable {
     initial = 0;
     max = kMaxSize;
   }
+
+  static bool isSubType(const Table& a, const Table& b) {
+    if (a.addressType != b.addressType) {
+      return false;
+    }
+
+    if (!Type::isSubType(a.type, b.type)) {
+      return false;
+    }
+
+    if (a.initial > b.initial) {
+      return false;
+    }
+
+    if (a.max < b.max) {
+      return false;
+    }
+
+    return true;
+  }
+
+  bool isSubType(const Table& other) { return Table::isSubType(*this, other); }
 };
 
 class DataSegment : public Named {
@@ -2470,6 +2492,15 @@ class Memory : public Importable {
     shared = false;
     addressType = Type::i32;
   }
+
+  static bool isSubType(const Memory& a, const Memory& b) {
+    return a.addressType == b.addressType && a.initial >= b.initial &&
+           a.max <= b.max;
+  }
+
+  bool isSubType(const Memory& other) {
+    return Memory::isSubType(*this, other);
+  }
 };
 
 class Global : public Importable {
@@ -2661,6 +2692,7 @@ std::ostream& operator<<(std::ostream& o, wasm::ModuleExpression pair);
 std::ostream& operator<<(std::ostream& o, wasm::ShallowExpression expression);
 std::ostream& operator<<(std::ostream& o, wasm::ModuleType pair);
 std::ostream& operator<<(std::ostream& o, wasm::ModuleHeapType pair);
+std::ostream& operator<<(std::ostream& o, const wasm::Memory& memory);
 
 } // namespace std
 

test/spec/exact-func-import.wast

@@ -1,29 +1,25 @@
-;; TODO: Use the upstream version from the custom descriptors proposal.
-
-(module
+(module definition
   (type $f (func))
   (import "" "" (func $1 (exact (type 0))))
-  (import "" "" (func $2 (exact (type $f) (param) (result))))
-  (import "" "" (func $3 (exact (type $f))))
-  (import "" "" (func $4 (exact (type 1)))) ;; Implicitly defined next
-  (import "" "" (func $5 (exact (param i32) (result i64))))
+  ;; (import "" "" (func $2 (exact (type $f) (param) (result)))) ;; TODO: parser support
+  (import "" "" (func $2 (exact (type $f))))
+  (import "" "" (func $3 (exact (type 1)))) ;; Implicitly defined next
+  (import "" "" (func $4 (exact (param i32) (result i64))))
 
-  (func $6 (import "" "") (exact (type 0)))
-  (func $7 (import "" "") (exact (type $f) (param) (result)))
-  (func $8 (import "" "") (exact (type $f)))
-  (func $9 (import "" "") (exact (type 2))) ;; Implicitly defined next
-  (func $10 (import "" "") (exact (param i64) (result i32)))
+  (func $5 (import "" "") (exact (type 0)))
+  ;; (func $6 (import "" "") (exact (type $f) (param) (result))) ;; TODO: parser support
+  (func $6 (import "" "") (exact (type $f)))
+  ;; (func $7 (import "" "") (exact (type 2))) ;; Implicitly defined next
+  ;; (func $8 (import "" "") (exact (param i64) (result i32))) ;; TODO: parser support
 
   (global (ref (exact $f)) (ref.func $1))
   (global (ref (exact $f)) (ref.func $2))
-  (global (ref (exact $f)) (ref.func $3))
+  (global (ref (exact 1)) (ref.func $3))
   (global (ref (exact 1)) (ref.func $4))
-  (global (ref (exact 1)) (ref.func $5))
+  (global (ref (exact $f)) (ref.func $5))
   (global (ref (exact $f)) (ref.func $6))
-  (global (ref (exact $f)) (ref.func $7))
-  (global (ref (exact $f)) (ref.func $8))
-  (global (ref (exact 2)) (ref.func $9))
-  (global (ref (exact 2)) (ref.func $10))
+  ;; (global (ref (exact 2)) (ref.func $7))
+  ;; (global (ref (exact 2)) (ref.func $8))
 )
 
 ;; References to inexact imports are not exact.
@@ -51,7 +47,7 @@
 
 ;; Inexact imports can still be referenced inexactly, though.
 
-(module
+(module definition
   (type $f (func))
   (import "" "" (func $1 (type $f)))
   (global (ref $f) (ref.func $1))
@@ -70,7 +66,9 @@
 ;; Import and re-export inexactly.
 (module $B
   (type $f (func))
-  (func (export "f") (import "A" "f") (type $f))
+  ;; (func (import "A" "f") (export "f") (type $f))
+  (func (import "A" "f") (type $f))
+  (export "f" (func 0))
 )
 (register "B")
 
@@ -220,7 +218,7 @@
 ;; Test the binary format
 
 ;; Exact function imports use 0x20.
-(module binary
+(module definition binary
   "\00asm" "\01\00\00\00"
   "\01"  ;; Type section id
   "\04"  ;; Type section length
@@ -265,4 +263,4 @@
     "\0b"  ;; End
   )
   "malformed export kind"
-)
+)