diff --git a/scripts/test/shared.py b/scripts/test/shared.py index 30c3593d6d2..49fc445cd2d 100644 --- a/scripts/test/shared.py +++ b/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 diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 39573372d78..6004342c878 100644 --- a/src/passes/Print.cpp +++ b/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(&memory)); + return o; +} + } // namespace std diff --git a/src/shell-interface.h b/src/shell-interface.h index 7a42617e711..1146bd217aa 100644 --- a/src/shell-interface.h +++ b/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(); diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index f6fc5586a94..b173a30fc65 100644 --- a/src/wasm-interpreter.h +++ b/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 { 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 { 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 { // initialize the rest of the external interface externalInterface->init(wasm, *self()); + validateImports(); + initializeTableContents(); initializeMemoryContents(); @@ -3269,6 +3274,70 @@ class ModuleRunnerBase : public ExpressionRunner { 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 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(&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(&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 { } 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); diff --git a/src/wasm.h b/src/wasm.h index 605c7025395..62a13259252 100644 --- a/src/wasm.h +++ b/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 diff --git a/test/spec/exact-func-import.wast b/test/spec/exact-func-import.wast index 49330fc7b84..bd152ef2a1e 100644 --- a/test/spec/exact-func-import.wast +++ b/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" -) +) \ No newline at end of file