Skip to content

Commit 32967bb

Browse files
stevenfontanellastevenfontanella
committed
Check that module imports match their corresponding exports
1 parent 9d94003 commit 32967bb

File tree

8 files changed

+163
-41
lines changed

8 files changed

+163
-41
lines changed

‎scripts/test/shared.py‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,6 @@ def get_tests(test_dir, extensions=[], recursive=False):
444444
'proposals/threads/imports.wast', # Missing memory type validation on instantiation
445445
'linking.wast', # Missing function type validation on instantiation
446446
'proposals/threads/memory.wast', # Missing memory type validation on instantiation
447-
'memory64-imports.wast', # Missing validation on instantiation
448447
'annotations.wast', # String annotations IDs should be allowed
449448
'id.wast', # Empty IDs should be disallowed
450449
# Requires correct handling of tag imports from different instances of the same module,
@@ -473,12 +472,7 @@ def get_tests(test_dir, extensions=[], recursive=False):
473472
'type-rec.wast', # Missing function type validation on instantiation
474473
'type-subtyping.wast', # ShellExternalInterface::callTable does not handle subtyping
475474
'call_indirect.wast', # Bug with 64-bit inline element segment parsing
476-
'memory64.wast', # Requires validations for memory size
477-
'imports0.wast', # Missing memory type validation on instantiation
478-
'imports2.wast', # Missing memory type validation on instantiation
479-
'imports3.wast', # Missing memory type validation on instantiation
480-
'linking0.wast', # Missing memory type validation on instantiation
481-
'linking3.wast', # Fatal error on missing table.
475+
'memory64.wast', # Requires validations on the max memory size
482476
'i16x8_relaxed_q15mulr_s.wast', # Requires wast `either` support
483477
'i32x4_relaxed_trunc.wast', # Requires wast `either` support
484478
'i8x16_relaxed_swizzle.wast', # Requires wast `either` support

‎src/shell-interface.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,9 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
134134
auto inst = getImportInstance(import);
135135
auto* exportedGlobal = inst->wasm.getExportOrNull(import->base);
136136
if (!exportedGlobal || exportedGlobal->kind != ExternalKind::Global) {
137-
Fatal() << "importGlobals: unknown import: " << import->module.str
138-
<< "." << import->name.str;
137+
trap((std::stringstream() << "unknown import: " << import->module.str
138+
<< "." << import->name.str)
139+
.str());
139140
}
140141
globals[import->name] = inst->globals[*exportedGlobal->getInternalName()];
141142
});
@@ -325,7 +326,9 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
325326
return true;
326327
}
327328

328-
void trap(const char* why) override {
329+
void trap(const char* why) override { trap(std::string_view(why)); }
330+
331+
void trap(std::string_view why) override {
329332
std::cout << "[trap " << why << "]\n";
330333
throw TrapException();
331334
}

‎src/tools/wasm-shell.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ struct Shell {
176176
// SIMD instructions.
177177
instance->setRelaxedBehavior(ModuleRunner::RelaxedBehavior::Execute);
178178
instance->instantiate();
179+
} catch (const std::exception& e) {
180+
return Err{std::string("failed to instantiate module: ") + e.what()};
179181
} catch (...) {
180182
return Err{"failed to instantiate module"};
181183
}

‎src/wasm-interpreter.h‎

Lines changed: 96 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#define wasm_wasm_interpreter_h
3030

3131
#include <cmath>
32+
#include <iomanip>
3233
#include <limits.h>
3334
#include <sstream>
3435
#include <variant>
@@ -86,7 +87,7 @@ class Flow {
8687
}
8788

8889
Literals values;
89-
Name breakTo; // if non-null, a break is going on
90+
Name breakTo; // if non-null, a break is going on
9091
Tag* suspendTag = nullptr; // if non-null, breakTo must be SUSPEND_FLOW, and
9192
// this is the tag being suspended
9293

@@ -2658,6 +2659,8 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
26582659

26592660
virtual void trap(const char* why) { WASM_UNREACHABLE("unimp"); }
26602661

2662+
virtual void trap(std::string_view why) { WASM_UNREACHABLE("unimp"); }
2663+
26612664
virtual void hostLimit(const char* why) { WASM_UNREACHABLE("unimp"); }
26622665

26632666
virtual void throwException(const WasmException& exn) {
@@ -2937,6 +2940,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
29372940
Index oldSize,
29382941
Index newSize) = 0;
29392942
virtual void trap(const char* why) = 0;
2943+
virtual void trap(std::string_view why) { trap(std::string(why).c_str()); }
29402944
virtual void hostLimit(const char* why) = 0;
29412945
virtual void throwException(const WasmException& exn) = 0;
29422946
// Get the Tag instance for a tag implemented in the host, that is, not
@@ -3178,6 +3182,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
31783182
// initialize the rest of the external interface
31793183
externalInterface->init(wasm, *self());
31803184

3185+
validateImports();
3186+
31813187
initializeTableContents();
31823188
initializeMemoryContents();
31833189

@@ -3269,6 +3275,76 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
32693275
Name name;
32703276
};
32713277

3278+
// TODO: function imports are allowed to not exist, since they
3279+
// may come from the special "spectest" module.
3280+
void validateImportKindMatches(ExternalKind kind,
3281+
const Importable& importable) {
3282+
auto it = linkedInstances.find(importable.module);
3283+
if (it == linkedInstances.end()) {
3284+
trap((std::stringstream()
3285+
<< "Import module " << std::quoted(importable.module.toString())
3286+
<< " doesn't exist.")
3287+
.str());
3288+
}
3289+
SubType* importedInstance = it->second.get();
3290+
3291+
Export* export_ = importedInstance->wasm.getExportOrNull(importable.base);
3292+
3293+
// In case functions are imported from the special "spectest" module,
3294+
// don't check them here, since they won't show up in exports.
3295+
if (!export_ && kind != ExternalKind::Function) {
3296+
trap((std::stringstream()
3297+
<< "Export " << importable.base << " doesn't exist.")
3298+
.str());
3299+
}
3300+
if (export_ && export_->kind != kind) {
3301+
trap("Exported kind doesn't match");
3302+
}
3303+
}
3304+
3305+
// Trap if types don't match between all imports and their corresponding
3306+
// exports. Imported memories and tables must also be a subtype of their
3307+
// export.
3308+
void validateImports() {
3309+
ModuleUtils::iterImportable(
3310+
wasm,
3311+
[this](ExternalKind kind,
3312+
std::variant<Function*, Memory*, Tag*, Global*, Table*> import) {
3313+
Importable* importable = std::visit(
3314+
[](const auto& import) -> Importable* { return import; }, import);
3315+
3316+
validateImportKindMatches(kind, *importable);
3317+
3318+
if (auto** memory = std::get_if<Memory*>(&import)) {
3319+
SubType* importedInstance =
3320+
linkedInstances.at((*memory)->module).get();
3321+
Export* export_ =
3322+
importedInstance->wasm.getExportOrNull((*memory)->base);
3323+
3324+
Memory exportedMemory =
3325+
*importedInstance->wasm.getMemory(*export_->getInternalName());
3326+
exportedMemory.initial =
3327+
importedInstance->getMemorySize(*export_->getInternalName());
3328+
3329+
if (!exportedMemory.isSubType(**memory)) {
3330+
trap("Imported memory isn't compatible.");
3331+
}
3332+
}
3333+
3334+
if (auto** table = std::get_if<Table*>(&import)) {
3335+
SubType* importedInstance =
3336+
linkedInstances.at((*table)->module).get();
3337+
Export* export_ =
3338+
importedInstance->wasm.getExportOrNull((*table)->base);
3339+
Table* exportedTable =
3340+
importedInstance->wasm.getTable(*export_->getInternalName());
3341+
if (!(*table)->isSubType(*exportedTable)) {
3342+
trap("Imported table isn't compatible");
3343+
}
3344+
}
3345+
});
3346+
}
3347+
32723348
TableInstanceInfo getTableInstanceInfo(Name name) {
32733349
auto* table = wasm.getTable(name);
32743350
if (table->imported()) {
@@ -3326,12 +3402,16 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
33263402
};
33273403

33283404
MemoryInstanceInfo getMemoryInstanceInfo(Name name) {
3329-
auto* memory = wasm.getMemory(name);
3330-
if (memory->imported()) {
3331-
auto& importedInstance = linkedInstances.at(memory->module);
3332-
auto* memoryExport = importedInstance->wasm.getExport(memory->base);
3333-
return importedInstance->getMemoryInstanceInfo(
3334-
*memoryExport->getInternalName());
3405+
auto* instance = self();
3406+
Export* memoryExport = nullptr;
3407+
for (auto* memory = instance->wasm.getMemory(name); memory->imported();
3408+
memory = instance->wasm.getMemory(*memoryExport->getInternalName())) {
3409+
instance = instance->linkedInstances.at(memory->module).get();
3410+
memoryExport = instance->wasm.getExport(memory->base);
3411+
}
3412+
3413+
if (memoryExport) {
3414+
return instance->getMemoryInstanceInfo(*memoryExport->getInternalName());
33353415
}
33363416

33373417
return MemoryInstanceInfo{self(), name};
@@ -3385,6 +3465,11 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
33853465
}
33863466

33873467
Address getMemorySize(Name memory) {
3468+
auto info = getMemoryInstanceInfo(memory);
3469+
if (info.instance != self()) {
3470+
return info.instance->getMemorySize(info.name);
3471+
}
3472+
33883473
auto iter = memorySizes.find(memory);
33893474
if (iter == memorySizes.end()) {
33903475
externalInterface->trap("getMemorySize called on non-existing memory");
@@ -3397,7 +3482,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
33973482
if (iter == memorySizes.end()) {
33983483
externalInterface->trap("setMemorySize called on non-existing memory");
33993484
}
3400-
memorySizes[memory] = size;
3485+
iter->second = size;
34013486
}
34023487

34033488
public:
@@ -4682,12 +4767,14 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
46824767
}
46834768
Flow visitStackSwitch(StackSwitch* curr) { return Flow(NONCONSTANT_FLOW); }
46844769

4685-
void trap(const char* why) override {
4770+
void trap(std::string_view why) override {
46864771
// Traps break all current continuations - they will never be resumable.
46874772
self()->clearContinuationStore();
46884773
externalInterface->trap(why);
46894774
}
46904775

4776+
void trap(const char* why) override { trap(std::string_view(why)); }
4777+
46914778
void hostLimit(const char* why) override {
46924779
self()->clearContinuationStore();
46934780
externalInterface->hostLimit(why);

‎src/wasm.h‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2435,6 +2435,28 @@ class Table : public Importable {
24352435
initial = 0;
24362436
max = kMaxSize;
24372437
}
2438+
2439+
static bool isSubType(const Table& a, const Table& b) {
2440+
if (a.addressType != b.addressType) {
2441+
return false;
2442+
}
2443+
2444+
if (!Type::isSubType(a.type, b.type)) {
2445+
return false;
2446+
}
2447+
2448+
if (a.initial > b.initial) {
2449+
return false;
2450+
}
2451+
2452+
if (a.max < b.max) {
2453+
return false;
2454+
}
2455+
2456+
return true;
2457+
}
2458+
2459+
bool isSubType(const Table& other) { return Table::isSubType(*this, other); }
24382460
};
24392461

24402462
class DataSegment : public Named {
@@ -2470,6 +2492,15 @@ class Memory : public Importable {
24702492
shared = false;
24712493
addressType = Type::i32;
24722494
}
2495+
2496+
static bool isSubType(const Memory& a, const Memory& b) {
2497+
return a.shared == b.shared && a.addressType == b.addressType &&
2498+
a.initial >= b.initial && a.max <= b.max;
2499+
}
2500+
2501+
bool isSubType(const Memory& other) {
2502+
return Memory::isSubType(*this, other);
2503+
}
24732504
};
24742505

24752506
class Global : public Importable {

‎test/spec/exact-func-import.wast‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,25 @@
1-
;; TODO: Use the upstream version from the custom descriptors proposal.
2-
3-
(module
1+
(module definition
42
(type $f (func))
53
(import "" "" (func $1 (exact (type 0))))
6-
(import "" "" (func $2 (exact (type $f) (param) (result))))
7-
(import "" "" (func $3 (exact (type $f))))
8-
(import "" "" (func $4 (exact (type 1)))) ;; Implicitly defined next
9-
(import "" "" (func $5 (exact (param i32) (result i64))))
4+
;; (import "" "" (func $2 (exact (type $f) (param) (result)))) ;; TODO: parser support
5+
(import "" "" (func $2 (exact (type $f))))
6+
(import "" "" (func $3 (exact (type 1)))) ;; Implicitly defined next
7+
(import "" "" (func $4 (exact (param i32) (result i64))))
108

11-
(func $6 (import "" "") (exact (type 0)))
12-
(func $7 (import "" "") (exact (type $f) (param) (result)))
13-
(func $8 (import "" "") (exact (type $f)))
14-
(func $9 (import "" "") (exact (type 2))) ;; Implicitly defined next
15-
(func $10 (import "" "") (exact (param i64) (result i32)))
9+
(func $5 (import "" "") (exact (type 0)))
10+
;; (func $6 (import "" "") (exact (type $f) (param) (result))) ;; TODO: parser support
11+
(func $6 (import "" "") (exact (type $f)))
12+
;; (func $7 (import "" "") (exact (type 2))) ;; Implicitly defined next
13+
;; (func $8 (import "" "") (exact (param i64) (result i32))) ;; TODO: parser support
1614

1715
(global (ref (exact $f)) (ref.func $1))
1816
(global (ref (exact $f)) (ref.func $2))
19-
(global (ref (exact $f)) (ref.func $3))
17+
(global (ref (exact 1)) (ref.func $3))
2018
(global (ref (exact 1)) (ref.func $4))
21-
(global (ref (exact 1)) (ref.func $5))
19+
(global (ref (exact $f)) (ref.func $5))
2220
(global (ref (exact $f)) (ref.func $6))
23-
(global (ref (exact $f)) (ref.func $7))
24-
(global (ref (exact $f)) (ref.func $8))
25-
(global (ref (exact 2)) (ref.func $9))
26-
(global (ref (exact 2)) (ref.func $10))
21+
;; (global (ref (exact 2)) (ref.func $7))
22+
;; (global (ref (exact 2)) (ref.func $8))
2723
)
2824

2925
;; References to inexact imports are not exact.
@@ -51,7 +47,7 @@
5147

5248
;; Inexact imports can still be referenced inexactly, though.
5349

54-
(module
50+
(module definition
5551
(type $f (func))
5652
(import "" "" (func $1 (type $f)))
5753
(global (ref $f) (ref.func $1))
@@ -70,7 +66,9 @@
7066
;; Import and re-export inexactly.
7167
(module $B
7268
(type $f (func))
73-
(func (export "f") (import "A" "f") (type $f))
69+
;; (func (import "A" "f") (export "f") (type $f))
70+
(func (import "A" "f") (type $f))
71+
(export "f" (func 0))
7472
)
7573
(register "B")
7674

@@ -220,7 +218,7 @@
220218
;; Test the binary format
221219

222220
;; Exact function imports use 0x20.
223-
(module binary
221+
(module definition binary
224222
"\00asm" "\01\00\00\00"
225223
"\01" ;; Type section id
226224
"\04" ;; Type section length
@@ -265,4 +263,4 @@
265263
"\0b" ;; End
266264
)
267265
"malformed export kind"
268-
)
266+
)

‎test/spec/ref_func.wast‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
(module
22
(func (export "f") (param $x i32) (result i32) (local.get $x))
33
)
4+
(register "M")
45
(module
56
(func $f (import "M" "f") (param i32) (result i32))
67
(func $g (param $x i32) (result i32) (i32.add (local.get $x) (i32.const 1)))

‎test/spec/tags.wast‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
;; Test tags
22

3+
(module
4+
(tag (export "im0") (param i32))
5+
(tag (export "im1") (param i32 f32))
6+
)
7+
(register "env")
8+
39
(module
410
(tag $e-import (import "env" "im0") (param i32))
511
(import "env" "im1" (tag (param i32 f32)))

0 commit comments

Comments
 (0)