emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.92k stars 3.32k forks source link

Calling a JS function via the val API fails if compiled with -pthreads and linked with -O3 #22989

Closed bruno-j-nicoletti closed 1 week ago

bruno-j-nicoletti commented 1 week ago

I'm trying to call a JS function from C++ via the val API. If I compile with -pthread and link with any -O1, -O2 or -O3 it will fail as the function caller won't be registered on the JS side.

I get errors along the line of...

TypeError: caller is not a function
    at __emval_call_method (/Users/bruno/Work/webbind/build/webBind.js:1089:24)
    at wasm://wasm/00091682:wasm-function[508]:0x213b0
    at Module._main (/Users/bruno/Work/webbind/build/webBind.js:1389:99)
    at /Users/bruno/Work/webbind/build/webBind.js:1485:13

After some experimenting it looks like the function signature registration is not happening correctly. _emval_get_method_caller is being called on the C++ side, but apparently not being invoked on the JS side.

I have a small project that replicates the issue. Uncomment the line in CMakeLists.txt with add_compile_options(-pthread), rebuild and it will fail.

https://github.com/bruno-j-nicoletti/webbind

Possibly related to this older issue?

https://github.com/emscripten-core/emscripten/issues/15557

Any help appreciated.

brendandahl commented 1 week ago

I think you're missing the -pthread link flag when adding the compile flag.

brendandahl commented 1 week ago

@sbc100 Do you know off hand if there should be a linking error if you compile with pthreads enabled, but link without them?

sbc100 commented 1 week ago

I think we allow that. It shouldn't cause any issues on modern browsers.

What is the cause of this issue?

brendandahl commented 1 week ago

I didn't dig too deep, but it seems this thread_local is not working correctly.

sbc100 commented 1 week ago

There was a recent fix for TLS in non-threaded builds: https://github.com/llvm/llvm-project/pull/116136.

I wonder if that made it into an emscripten release yet? Are you able to reproduce on tot?

brendandahl commented 1 week ago

Works with emsdk tot!

sbc100 commented 1 week ago

Awesome I think we can close this then. The fix is in emsdk tot already and will be in the next release (3.1.73)

bruno-j-nicoletti commented 1 day ago

Thank you all. Just updated to 3.1.73 and that fixed it.