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

Embind problems with val::as<T>() in wasm32 with MAXIMUM_MEMORY=4GB #19968

Closed
pavshen opened this issue Aug 3, 2023 · 12 comments · Fixed by #20071
Closed

Embind problems with val::as<T>() in wasm32 with MAXIMUM_MEMORY=4GB #19968

pavshen opened this issue Aug 3, 2023 · 12 comments · Fixed by #20071
Labels

Comments

@pavshen
Copy link

pavshen commented Aug 3, 2023

There is a problem with raw pointers previously returned from embind-wrapped C++ functions by val(ptr) and structs embind-registered as value_object used later as in-parameters of type val of embind-wrapped C++ functions. Their conversion from val to native types by val::as<T>() inside the C++ function produces garbage in case of memory locations above 2GB (in raw pointer when its $$.ptr is negative; as for value_object, I don't know when). On the other hand, when they are used in functions with parameters of their native types without the usage of val they are marshaled OK. The same is with JS string literals passed as val and converted with val::as<std::string>().

(I need to pass them as val in order to support variable-type and nullable parameters).

I thought #19755 would fix this, but the problem still exists.

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.44 (bec42dac7873903d09d713963e34020c22a8bd2d)
clang version 17.0.0 (https://github.com/llvm/llvm-project a8cbd27d1f238e104a5d5ca345d93bc1f4d4ab1f)
Target: wasm32-unknown-emscripten
Thread model: posix
@kripken kripken added the embind label Aug 3, 2023
sbc100 added a commit that referenced this issue Aug 5, 2023
@sbc100
Copy link
Collaborator

sbc100 commented Aug 5, 2023

I have fix pending in 5fbde92

@pavshen
Copy link
Author

pavshen commented Aug 6, 2023

I merged 5fbde92 into my compiled version of tag 3.1.44 (I also replaced several additional relevant files that were changed after 3.1.44), It worked below 2GB, but above 2GB all three mentioned cases didn't work. I saw that $$.ptr's were positive for raw pointers above 2GB, but as<T>() still produced garbage.

@pavshen
Copy link
Author

pavshen commented Aug 15, 2023

I have fix pending in 5fbde92

@sbc100, any ideas why the fix doesn't work (see above)?

@sbc100
Copy link
Collaborator

sbc100 commented Aug 15, 2023

I am working on a bunch of >4gb changes right now. I imagine once they all land this issue should be resolved.

@pavshen
Copy link
Author

pavshen commented Aug 16, 2023

Do those changes include fixes in WebGL? #18744 fixes WebGL1 problems beyond 2GB, WebGL2 still crashes in many functions.

sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
sbc100 added a commit that referenced this issue Aug 18, 2023
@sbc100
Copy link
Collaborator

sbc100 commented Aug 18, 2023

The embind fixes have now landed and all the tests pass. If you example still fails perhaps you could come up with a small example of the failure code and we can turn that into a new test and fix it?

@sbc100 sbc100 reopened this Aug 18, 2023
@pavshen
Copy link
Author

pavshen commented Aug 21, 2023

@sbc100, I can try to test it tomorrow, but I saw #20077 and #20079. Should I either test without them or try to take them as well or wait until they have been merged?

Are you planning to fix WebGL, too? #18744 is old (for 3.1.38) and it fixes only WebGL1 problems beyond 2GB, WebGL2 still crashes in many functions.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 21, 2023

Yes, I would wait until both #20077 and #20079.

And yes, the plan to get everything working with wasm64 >4gb and with wasm32 >2gb. We are making good progress towards that.

@pavshen
Copy link
Author

pavshen commented Aug 21, 2023

Let me know when it is a good time to test embind and/or WebGL with wasm32 >2GB and I will update you about the results.

I'll be very glad to test wasm64 >4GB when it is ready. But, TypedArrays with long indices are not supported yet by browsers, are they?

@sbc100
Copy link
Collaborator

sbc100 commented Aug 21, 2023

For large typed arrays we now have a polyfil that allows us to get stuff running: #19737

@pavshen
Copy link
Author

pavshen commented Oct 16, 2023

My example still fails with 3.1.47. I will check which cases of the above-mentioned fail and will try to prepare small examples.

@pavshen
Copy link
Author

pavshen commented May 23, 2024

I checked again my example with 3.1.57 and 3.1.60. Now it works! I think the issue can be closed.

@sbc100 sbc100 closed this as completed May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants