Skip to content
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

Jump to bottom

Stop using re-assignment thunks when WASM_ASYNC_COMPILATION is enabled. #23339

Open
sbc100 opened this issue Jan 8, 2025 · 2 comments · May be fixed by #23411
Open

Stop using re-assignment thunks when WASM_ASYNC_COMPILATION is enabled. #23339

sbc100 opened this issue Jan 8, 2025 · 2 comments · May be fixed by #23411

Comments

@sbc100
Copy link
Collaborator

sbc100 commented Jan 8, 2025

In release mode, when WASM_ASYNC_COMPILATION is used (i.e the default) we generate little thunks for all wasm exports. They looks like this:

var _foo = Module['_foo'] = () => (_foo = Module['_cfoo'] = wasmExports['foo'])();

The idea here is that these would then get re-assigned on first use. However, there are several problem with this:

  1. If somebody takes a reference early they will get a reference to wrapper that will not get re-assigned and will cause perpetual re-assignment:
// This runs early on
var myfunc = Module['foo'];

// Now my func will forever point to the wrapper:

myfunc();  // always goes via wrapper and always re-assigns.. strange
Module['foo'](); // now goes direct
  1. The identity of these function changes which can be observable (since we don't this re-assignment thing in debug builds):
var myfunc = Module['foo'];
// later after instantiation and usage
if (myfunc == Module['foo']) // would always be true in debug builds, but can be false in release builds

See #23337
@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 8, 2025

(1) actually seems fairly bad since it perpetually mutates the Module and the globalThis.

@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 8, 2025

One possible solution would be to simply delay the assignments and just generate initially-undefined variables:

var _foo;  // Will simply be undefined until the instantiation is done.  As will `Module['_foo']`

In debug builds we could generate wrappers that simply fail if they are ever called:

var _foo = Module['_foo'] = reportModuleNotReady('_foo');

The major breakage here would be for folks that are currently taking references to _foo or Module['_foo'] before the module is ready.

sbc100 added a commit to sbc100/emscripten that referenced this issue Jan 15, 2025
@sbc100
Avoid creating wrapper thunks or wasm exports
0ee6b6f 
In debug builds nothing really changes here since we exported functions
still get statically assigned to debug wrappers/thunks.

However, after this change, in release builds, if you attempt to access
`_foo` (internally) or `Module['_foo']` externally before the module
is loaded you will now get `undefined`.

Since it was always the case that calling these function would crash
anyway, the only effected folks would be folks that take the reference
early and then call later. e.g.

```
var myfunc = Module['_foo'];  // Run before module is loaded
// wait until loaded
myfunc();  // This will no longer work.
```

Fixes: emscripten-core#23339
@sbc100 sbc100 linked a pull request Jan 15, 2025 that will close this issue
Draft
sbc100 added a commit to sbc100/emscripten that referenced this issue Jan 15, 2025
@sbc100
Avoid creating wrapper thunks or wasm exports
5ca2258 
In debug builds nothing really changes here since we exported functions
still get statically assigned to debug wrappers/thunks.

However, after this change, in release builds, if you attempt to access
`_foo` (internally) or `Module['_foo']` externally before the module
is loaded you will now get `undefined`.

Since it was always the case that calling these function would crash
anyway, the only effected folks would be folks that take the reference
early and then call later. e.g.

```
var myfunc = Module['_foo'];  // Run before module is loaded
// wait until loaded
myfunc();  // This will no longer work.
```

Fixes: emscripten-core#23339
sbc100 added a commit to sbc100/emscripten that referenced this issue Jan 15, 2025
@sbc100
Avoid creating wrapper thunks or wasm exports
078ea97 
In debug builds nothing really changes here since we exported functions
still get statically assigned to debug wrappers/thunks.

However, after this change, in release builds, if you attempt to access
`_foo` (internally) or `Module['_foo']` externally before the module
is loaded you will now get `undefined`.

Since it was always the case that calling these function would crash
anyway, the only effected folks would be folks that take the reference
early and then call later. e.g.

```
var myfunc = Module['_foo'];  // Run before module is loaded
// wait until loaded
myfunc();  // This will no longer work.
```

Fixes: emscripten-core#23339
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WIP] Avoid creating wrapper thunks or wasm exports sbc100/emscripten
1 participant
@sbc100