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.

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

EM_JS functions can only be called from the files in which they're defined #18927

Closed
curiousdannii opened this issue Mar 9, 2023 · 3 comments · Fixed by #18928
Closed

EM_JS functions can only be called from the files in which they're defined #18927

curiousdannii opened this issue Mar 9, 2023 · 3 comments · Fixed by #18928

Comments

@curiousdannii
Copy link
Contributor

curiousdannii commented Mar 9, 2023

Just posting as a new issue so it doesn't get forgotten...

Originally posted by @sbc100 in #16003 (comment)

Sorry to raise an error just after you concluded there were no issues stuck_out_tongue But you know, sometimes it takes a while to try updating dependencies.

I tried updating to 3.1.28+ and I now get errors for undefined symbols when an EM_JS function is defined in a library.

wasm-ld: error: libremglk.a(gi_dispa.c.o): undefined symbol: glk_buffer_to_title_case_uni

I think the reason for this is that the EM_JS macro basically expands to a declaration of the EM_JS function as external with an __attribute__((import_name)) attached to it.

However, when you use it outside in other compilation units they don't see that declaration. Actually that sounds odd now that I say its since those attributes should probably attached to the final symbol.

In any case, I think the soltution that others have uses it to only call the EM_JS function within the file where they are defined, and then define small (difined) wrappers for them for use outside the compilation unit.

e.g. declare glk_buffer_to_title_case_uni_js as the EM_JS function then define a local glk_buffer_to_title_case_uni which then just calls it.

(I will investigate further to see if we can actually this fix with you having to change any code).

There seems to be two options:

  1. A way is found so that EM_JS functions can be called from other files without users needing to wrap them.
  2. The docs are modified to note that this is a limitation and wrapping is needed.
@sbc100
Copy link
Collaborator

sbc100 commented Mar 9, 2023

Thanks for filing this. I agree we need to figure this one out. I think we can probably find a way to make (1) work.

@sbc100
Copy link
Collaborator

sbc100 commented Mar 9, 2023

Investigating this I noticed that this only happens if there are no references to the EM_JS function withing the same file in which it is declared. As long as there is at least one file-local caller than all the other call sites in the program work just fine.

sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

Fixes: #18927
@sbc100
Copy link
Collaborator

sbc100 commented Mar 9, 2023

I founds a fairly straight forward fix: #18928

sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

Fixes: #18927
sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

Fixes: #18927
sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

Fixes: #18927
sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

This use case was broken by the switch to LLD_REPORT_UNDEFINED in #16003.

Fixes: #18927
sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

This use case was broken by the switch to LLD_REPORT_UNDEFINED in #16003.

Fixes: #18927
sbc100 added a commit that referenced this issue Mar 9, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

This use case was broken by the switch to LLD_REPORT_UNDEFINED in #16003.

Fixes: #18927
impact-maker pushed a commit to impact-maker/emscripten that referenced this issue Mar 17, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

This use case was broken by the switch to LLD_REPORT_UNDEFINED in emscripten-core#16003.

Fixes: emscripten-core#18927
impact-maker pushed a commit to impact-maker/emscripten that referenced this issue Mar 17, 2023
When no local usage of an EM_JS function is present the compiler was
completely removing the import of the function (and its import_name).

With this change we force at least one reference to the function by
taking its address in an otherwise unused (and GC-able) pointer.

This use case was broken by the switch to LLD_REPORT_UNDEFINED in emscripten-core#16003.

Fixes: emscripten-core#18927
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 a pull request may close this issue.

2 participants