Skip to content

Runtime compilation: fail if importing from unexpected modules #1931

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.

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
merged 1 commit into from
Apr 11, 2025

Conversation

vouillon
Copy link
Member

No description provided.

@hhugo
Copy link
Member

hhugo commented Apr 10, 2025

I was thinking about a check that verify that import from known modules have a matching "export".
A typo in the name of an imported function will result in a runtime error that is not even mentioning the problematic name.

Step to reproduce:

  • Apply the following diff to obj.wat
 -  (import "effect" "caml_trampoline"
 +  (import "effect" "caml_trampoline23"
  • Run some wasm code
    WASM_OF_OCAML=true dune build @compiler/tests-ocaml/runtest-wasm --profile=with-effects

  • Errors

Command [47] exited with code 1:
$ (cd _build/default/compiler/tests-ocaml/lib-option && ../../../.bin/node test.bc.wasm.js) > _build/default/compiler/tests-ocaml/lib-option/test.bc.wasm.js.output
node:internal/process/promises:394
    triggerUncaughtException(err, true /* fromPromise */);
    ^

[TypeError: WebAssembly.instantiate(): Import #110 "effect": module is not an object or function]

Node.js v23.10.0

@vouillon
Copy link
Member Author

With this PR, you get the following error message:

The runtime contains unknown imports:
  effect caml_trampoline23

@hhugo
Copy link
Member

hhugo commented Apr 11, 2025

With this PR, you get the following error message:

The runtime contains unknown imports:
  effect caml_trampoline23

Cool, I was testing with an older commit which didn't exit 2 in case of errors (1c29882)

@hhugo hhugo merged commit 18cc926 into master Apr 11, 2025
22 of 26 checks passed
@hhugo hhugo deleted the runtime-checks branch April 11, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants