Add validations for imports during instantiation

scripts/test/shared.py

@@ -418,15 +418,14 @@ 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',  # Requires fixing handling of mutation to imported globals
     '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,
-    # ref.null wast constants, and splitting for module instances
+    # Requires correct handling of tag imports from different instances of the same module
+    # and splitting for module instances
     'instance.wast',
     'table64.wast',   # Requires validations for table size
     'table_grow.wast',  # Incorrect table linking semantics in interpreter
@@ -451,12 +450,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 validations on the max memory size
+    'imports3.wast',  # Requires better checking of exports from the special "spectest" module
     '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/ir/memory-utils.cpp

@@ -20,6 +20,11 @@
 
 namespace wasm::MemoryUtils {
 
+bool isSubType(const Memory& a, const Memory& b) {
+  return a.shared == b.shared && a.addressType == b.addressType &&
+         a.initial >= b.initial && a.max <= b.max;
+}
+
 bool flatten(Module& wasm) {
   // If there are no memories then they are already flat, in the empty sense.
   if (wasm.memories.empty()) {

src/ir/memory-utils.h

@@ -28,6 +28,8 @@
 
 namespace wasm::MemoryUtils {
 
+bool isSubType(const Memory& a, const Memory& b);
+
 // Flattens memory into a single data segment, or no segment. If there is
 // a segment, it starts at 0.
 // Returns true if successful (e.g. relocatable segments cannot be flattened).

src/ir/table-utils.cpp

@@ -21,6 +21,26 @@
 
 namespace wasm::TableUtils {
 
+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;
+}
+
 std::set<Name> getFunctionsNeedingElemDeclare(Module& wasm) {
   // Without reference types there are no ref.funcs or elem declare.
   if (!wasm.features.hasReferenceTypes()) {

src/ir/table-utils.h

@@ -26,6 +26,8 @@
 
 namespace wasm::TableUtils {
 
+bool isSubType(const Table& a, const Table& b);
+
 struct FlatTable {
   std::vector<Name> names;
   bool valid;

src/shell-interface.h

@@ -134,8 +134,10 @@ 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()
+              << "importGlobals: unknown import: " << import->module.str << "."
+              << import->name.str)
+               .str());
       }
       globals[import->name] = inst->globals[*exportedGlobal->getInternalName()];
     });

src/support/json.h

@@ -280,6 +280,8 @@ struct Value {
         curr++;
       }
     };
+    (void)is_json_space;
+    (void)skip;
     skip();
     if (*curr == '"') {
       // String

src/tools/wasm-shell.cpp

@@ -175,7 +175,9 @@ struct Shell {
       // This is not an optimization: we want to execute anything, even relaxed
       // SIMD instructions.
       instance->setRelaxedBehavior(ModuleRunner::RelaxedBehavior::Execute);
-      instance->instantiate();
+      instance->instantiate(/* validateImports_=*/true);
+    } catch (const std::exception& e) {
+      return Err{std::string("failed to instantiate module: ") + e.what()};
     } catch (...) {
       return Err{"failed to instantiate module"};
     }

src/wasm-interpreter.h

@@ -29,15 +29,18 @@
 #define wasm_wasm_interpreter_h
 
 #include <cmath>
+#include <iomanip>
 #include <limits.h>
 #include <sstream>
 #include <variant>
 
 #include "fp16.h"
 #include "ir/intrinsics.h"
 #include "ir/iteration.h"
+#include "ir/memory-utils.h"
 #include "ir/module-utils.h"
 #include "ir/properties.h"
+#include "ir/table-utils.h"
 #include "support/bits.h"
 #include "support/safe_integer.h"
 #include "support/stdckdint.h"
@@ -3207,7 +3210,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
   // Start up this instance. This must be called before doing anything else.
   // (This is separate from the constructor so that it does not occur
   // synchronously, which makes some code patterns harder to write.)
-  void instantiate() {
+  void instantiate(bool validateImports_ = false) {
     // import globals from the outside
     externalInterface->importGlobals(globals, wasm);
     // generate internal (non-imported) globals
@@ -3218,6 +3221,10 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
     // initialize the rest of the external interface
     externalInterface->init(wasm, *self());
 
+    if (validateImports_) {
+      validateImports();
+    }
+
     initializeTableContents();
     initializeMemoryContents();
 
@@ -3309,6 +3316,81 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
     Name name;
   };
 
+  // Validates that the export that provides `importable` exists and has the
+  // same kind that the import expects (`kind`).
+  void validateImportKindMatches(ExternalKind kind,
+                                 const Importable& importable) {
+    auto it = linkedInstances.find(importable.module);
+    if (it == linkedInstances.end()) {
+      trap((std::stringstream()
+            << "Import module " << std::quoted(importable.module.toString())
+            << " doesn't exist.")
+             .str());
+    }
+    auto* importedInstance = it->second.get();
+
+    Export* export_ = importedInstance->wasm.getExportOrNull(importable.base);
+
+    if (!export_) {
+      trap((std::stringstream()
+            << "Export " << importable.base << " doesn't exist.")
+             .str());
+    }
+    if (export_->kind != kind) {
+      trap((std::stringstream() << "Exported kind: " << export_->kind
+                                << " doesn't match expected kind: " << kind)
+             .str());
+    }
+  }
+
+  // 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);
+
+        // These two modules are injected implicitly to tests. We won't find any
+        // import information for them.
+        if (importable->module == "binaryen-intrinsics" ||
+            (importable->module == "spectest" &&
+             importable->base.startsWith("print")) ||
+            importable->module == "fuzzing-support") {
+          return;
+        }
+
+        validateImportKindMatches(kind, *importable);
+
+        SubType* importedInstance =
+          linkedInstances.at(importable->module).get();
+        Export* export_ =
+          importedInstance->wasm.getExportOrNull(importable->base);
+
+        if (auto** memory = std::get_if<Memory*>(&import)) {
+          Memory exportedMemory =
+            *importedInstance->wasm.getMemory(*export_->getInternalName());
+          exportedMemory.initial =
+            importedInstance->getMemorySize(*export_->getInternalName());
+
+          if (!MemoryUtils::isSubType(exportedMemory, **memory)) {
+            trap("Imported memory isn't compatible.");
+          }
+        }
+
+        if (auto** table = std::get_if<Table*>(&import)) {
+          Table* exportedTable =
+            importedInstance->wasm.getTable(*export_->getInternalName());
+          if (!TableUtils::isSubType(**table, *exportedTable)) {
+            trap("Imported table isn't compatible");
+          }
+        }
+      });
+  }
+
   TableInstanceInfo getTableInstanceInfo(Name name) {
     auto* table = wasm.getTable(name);
     if (table->imported()) {
@@ -3366,12 +3448,16 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
   };
 
   MemoryInstanceInfo getMemoryInstanceInfo(Name name) {
-    auto* memory = wasm.getMemory(name);
-    if (memory->imported()) {
-      auto& importedInstance = linkedInstances.at(memory->module);
-      auto* memoryExport = importedInstance->wasm.getExport(memory->base);
-      return importedInstance->getMemoryInstanceInfo(
-        *memoryExport->getInternalName());
+    auto* instance = self();
+    Export* memoryExport = nullptr;
+    for (auto* memory = instance->wasm.getMemory(name); memory->imported();
+         memory = instance->wasm.getMemory(*memoryExport->getInternalName())) {
+      instance = instance->linkedInstances.at(memory->module).get();
+      memoryExport = instance->wasm.getExport(memory->base);
+    }
+
+    if (memoryExport) {
+      return instance->getMemoryInstanceInfo(*memoryExport->getInternalName());
     }
 
     return MemoryInstanceInfo{self(), name};
@@ -3425,6 +3511,11 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
   }
 
   Address getMemorySize(Name memory) {
+    auto info = getMemoryInstanceInfo(memory);
+    if (info.instance != self()) {
+      return info.instance->getMemorySize(info.name);
+    }
+
     auto iter = memorySizes.find(memory);
     if (iter == memorySizes.end()) {
       externalInterface->trap("getMemorySize called on non-existing memory");
@@ -3437,7 +3528,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
     if (iter == memorySizes.end()) {
       externalInterface->trap("setMemorySize called on non-existing memory");
     }
-    memorySizes[memory] = size;
+    iter->second = size;
   }
 
 public:
@@ -4967,7 +5058,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
     if (lhs > rhs) {
       std::stringstream ss;
       ss << msg << ": " << lhs << " > " << rhs;
-      externalInterface->trap(ss.str().c_str());
+      externalInterface->trap(ss.str());
     }
   }
 

test/spec/exact-func-import.wast

@@ -1,29 +1,27 @@
 ;; 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 +49,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 +68,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 +220,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 +265,4 @@
     "\0b"  ;; End
   )
   "malformed export kind"
-)
+)

test/spec/ref_func.wast

@@ -1,6 +1,9 @@
+;; TODO: use test/spec/testsuite/ref_func.wast once it's fixed.
+
 (module
   (func (export "f") (param $x i32) (result i32) (local.get $x))
 )
+(register "M")
 (module
   (func $f (import "M" "f") (param i32) (result i32))
   (func $g (param $x i32) (result i32) (i32.add (local.get $x) (i32.const 1)))

test/spec/tags.wast

@@ -1,5 +1,11 @@
 ;; Test tags
 
+(module
+  (tag (export "im0") (param i32))
+  (tag (export "im1") (param i32 f32))
+)
+(register "env")
+
 (module
   (tag $e-import (import "env" "im0") (param i32))
   (import "env" "im1" (tag (param i32 f32)))