[js-api] Share import reading between the JS API and ESM integration - #126
Open
guybedford wants to merge 3 commits into
Open
guybedford wants to merge 3 commits into
guybedford wants to merge 3 commits into
Conversation
ExecuteModule duplicated the per-import value coercion of read the imports and had drifted from it: TypeError from ToWebAssemblyValue was not converted to LinkError, the i64 / number kind checks were missing, and there was no tag case at all. It also created a fresh builtin function address per import, whereas read the imports instantiates each builtin set once, so importing the same builtin twice gave distinct Exported Function identities under ESM only. Factor read the imports into two helpers, instantiate the builtin and string imports and read an import value, and use them from ExecuteModule so both instantiation paths share the same semantics. Only the Wasm to Wasm instance_export linking remains ESM specific.
chicoxyzzy
reviewed
Sep 25, 2026
Comment on lines
+560
to
+572
| 1. If |externtype| is of the form [=external-type/global=] <var ignore>mut</var> |valtype|, | ||
| 1. If |v| [=implements=] {{Global}}, | ||
| 1. Let |globaladdr| be |v|.\[[Global]]. | ||
| 1. Otherwise, | ||
| 1. If |valtype| is [=i64=] and |v| [=is not a BigInt=], | ||
| 1. Throw a {{LinkError}} exception. | ||
| 1. If |valtype| is one of [=i32=], [=f32=] or [=f64=] and |v| [=is not a Number=], | ||
| 1. Throw a {{LinkError}} exception. | ||
| 1. If |valtype| is [=v128=], | ||
| 1. Throw a {{LinkError}} exception. | ||
| 1. Let |value| be [=ToWebAssemblyValue=](|v|, |valtype|). If this operation throws a {{TypeError}}, catch it, and throw a {{LinkError}} exception. | ||
| 1. Let |store| be the [=surrounding agent=]'s [=associated store=]. | ||
| 1. Let (|store|, |globaladdr|) be [=global_alloc=](|store|, [=const=] |valtype|, |value|). |
Member
There was a problem hiding this comment.
read an import value drops mut and always does global_alloc of a const global.
ExecuteModule used to throw a LinkError before any allocation. One case is a var import that is not already a Global. The other is a const import of a var Global. Both still fail in module_instantiate. The var case now leaves that const global in the store on the way there.
Collaborator
Author
There was a problem hiding this comment.
This one is actually pre-existing in the WebAssembly.instantiate spec itself, which we are now following.
Therefore if we want to address this I think it would need to be an upstream PR.
parse a WebAssembly module constructed the Module object without running validate builtins and imported string, unlike compile, validate and the Module constructor, so a string constant import with a non extern type or a builtin import with the wrong signature was accepted at parse time and only failed during execution.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This refactors
read the importsso thatExecuteModulecan share it.Two helpers are extracted from
read the imports, keeping the internal implementation identical:instantiate the builtin and string importsreturns themoduleName -> exportsObjectmapread an import valuecoerces a JS value of a given extern type and appends the externvalIn the process this resolves some drift between the two paths including:
ToWebAssemblyValuefailures propagated asTypeErrorrather than being converted toLinkErrori64/ BigInt and numeric kind checks were missingtagcase, so a JS module exporting aWebAssembly.Tagto a Wasm import appended nothing and misaligned the externval listread the importsinstantiates each builtin set once, so importing the same builtin twice gave distinctExported Functionidentities under ESM only//cc @chicoxyzzy for review