Skip to content

Add type validations for function imports #8478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stevenfontanella merged 2 commits into from
Mar 18, 2026
+16 −6
Merged

Add type validations for function imports #8478

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2afcbf
Add validations for function imports
stevenfontanella Mar 17, 2026
4619bfa
Update error message
stevenfontanella Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions scripts/test/shared.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ def get_tests(test_dir, extensions=[], recursive=False):
'if.wast', # Requires more precise unreachable validation
'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
'linking.wast', # Missing global type validation on instantiation
'proposals/threads/memory.wast', # Missing memory type validation on instantiation
'annotations.wast', # String annotations IDs should be allowed
'instance.wast', # Requires support for table default elements
Expand All @@ -445,8 +445,6 @@ def get_tests(test_dir, extensions=[], recursive=False):
'ref_cast.wast', # Requires host references to not be externalized i31refs
'ref_test.wast', # Requires host references to not be externalized i31refs
'struct.wast', # Fails to roundtrip unnamed types e.g. `(ref 0)`
'type-rec.wast', # Missing function type validation on instantiation
'type-subtyping.wast', # ShellExternalInterface::callTable does not handle subtyping
'memory64.wast', # Requires validations on the max memory size
'imports3.wast', # Requires better checking of exports from the special "spectest" module
'relaxed_dot_product.wast', # i16x8.relaxed_dot_i8x16_i7x16_s instruction not supported
Expand Down
16 changes: 13 additions & 3 deletions src/wasm-interpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3450,9 +3450,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
if (!MemoryUtils::isSubType(exportedMemory, **memory)) {
trap("Imported memory isn't compatible.");
}
}

if (auto** tableDecl = std::get_if<Table*>(&import)) {
} else if (auto** tableDecl = std::get_if<Table*>(&import)) {
Copy link
Member

@tlively tlively Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow-up, it would be nice to give Importable a getKind method so we could use a switch here.

Copy link
Member Author

@stevenfontanella stevenfontanella Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use std::visit or switch on import.index() if that sounds good? I didn't use visit originally since there are a lot of variables that we'd have to capture from the outside. Is the idea to make the cases exhaustive?

Copy link
Member

@tlively tlively Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to make it exhaustive and to make it easier to read (although that's subjective). I guess we would still need to cast to the relevant subtype, though, so maybe it's not worth it.

auto* importedTable = importResolver->getTableOrNull(
importable->importNames(), **tableDecl);
if (!importedTable) {
Expand All @@ -3468,7 +3466,19 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
<< " isn't compatible with import declaration: " << **tableDecl)
.str());
}
} else if (auto** function = std::get_if<Function*>(&import)) {
auto exportedFunc = getFunction((*function)->name);
if (!Type::isSubType(exportedFunc.type, (*function)->type)) {
trap((std::stringstream()
<< "Imported function with type "
<< exportedFunc.type.getHeapType().getSignature().toString()
<< " isn't compatible with import declaration: "
<< (*function)->type.getHeapType().getSignature().toString())
.str());
}
}

// TODO: remaining cases e.g. globals and tags.
});
}

Expand Down
Loading