Skip to content

Add adjustWasmImports incoming Module setting #23794

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hoodmane
Copy link
Collaborator

This provides a way to add symbols to the wasmImports dynamically. This doesn't allow anything new compared to instantiateWasm except that it doesn't interfere with sourcemaps or ASAN and it doesn't require reproducing the somewhat tricky behavior in the standard instantiateWasm function.

As an alternative, we could expose something like instantiateWasmDefault to the user, and then they could do this like:

Module['instantiateWasm'] = (imports, successCallback) => {
   adjustWasmImports(imports);
   instantiateWasmDefault(imports, successCallback);
}

This provides a way to add symbols to the wasmImports dynamically.
This doesn't allow anything new compared to `instantiateWasm` except
that it doesn't interfere with sourcemaps or ASAN and it doesn't require
reproducing the somewhat tricky behavior in the standard `instantiateWasm`
function.

As an alternative, we could expose something like `instantiateWasmDefault`
to the user, and then they could do this like:
```js
Module['instantiateWasm'] = (imports, successCallback) => {
   adjustWasmImports(imports);
   instantiateWasmDefault(imports, successCallback);
}
```
@sbc100
Copy link
Collaborator

sbc100 commented Feb 28, 2025

Can you say more about why you want to be able to do this?

e.g. Are you looking to wrap/modify the existing imports, or simply add new ones?

@hoodmane
Copy link
Collaborator Author

I originally added this in order to make JSPI work with emscripten js exceptions by replacing the invoke_ functions with runtime generated wasm trampolines that use wasm try/catch like in #23619. But just a few days ago I ran into someone else who needed to dynamically inject functions for some of their imports and wanted to know how to do it. It could also be helpful to wrap certain imports for debugging purposes. I think there are a lot of possible use cases.

I would say that it doesn't make much sense to restrict it to adding new imports because instantiateWasm doesn't have that restriction and the point of this is to make it possible to do the same things you can do with instantiateWasm but without having to duplicate the logic for getting the actual wasm module.

@emscripten-core emscripten-core locked and limited conversation to collaborators Feb 28, 2025
@emscripten-core emscripten-core unlocked this conversation Feb 28, 2025
@sbc100
Copy link
Collaborator

sbc100 commented Feb 28, 2025

The main problem I have adding new hooks like this it add yet more combinations of settings that we need to test and maintains.

I wonder if we could find some other possible ways to solve these problems. Its kind of a shame --pre-js comes before wasmImports is created and --post-js comes after its is used. Perhaps we could consider moving --post-js to between wasmImports creation and usage? Then you could do what you need here simply with --post-js I think

@hoodmane
Copy link
Collaborator Author

What about my idea of exposing instantiateWasmDefault()?

@sbc100
Copy link
Collaborator

sbc100 commented Feb 28, 2025

What about my idea of exposing instantiateWasmDefault()?

If we could do it at no codesize code that seems reasonable. I suppose we would need to pass it as an augment to instantiateWasm somehow? (Because the instantiateWasm might be declared out side the scope of the generated code).

@hoodmane
Copy link
Collaborator Author

Yeah passing it as an argument would make sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants