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

Segfault with published wasm version of libzim #751

Closed
mgautierfr opened this issue Dec 5, 2022 · 10 comments · Fixed by kiwix/kiwix-build#555
Closed

Segfault with published wasm version of libzim #751

mgautierfr opened this issue Dec 5, 2022 · 10 comments · Fixed by kiwix/kiwix-build#555
Assignees
Milestone

Comments

@mgautierfr
Copy link
Collaborator

From @Jaifroid kiwix/kiwix-build#552 (comment):

@mgautierfr I compiled an Emscripten WASM from the libzim.a and dependencies in /data/openzim/release/libzim/libzim_wasm-emscripten-8.1.0.tar.gz (date-stamp: 01/12/22 21:42) from master.download.kiwix.org). I compiled by making a clean build directory at /src/build/ and placing the lib and include directories from the package in it. I moved the contents of lib/x86_64-linux-gnu so that it is directly under lib, because that is how the libs are referenced in the em++ command below. I then compiled with this command, which is the command that produces a working WASM and wrapper when I compile everything from source on kiwix/javascript-libzim:

em++ -o libzim-wasm.js --bind libzim_bindings.cpp -I/src/build/include -L/src/build/lib -lzim -llzma -lzstd -lxapian -lz -licui18n -licuuc -licudata -lpthread -lm -fdiagnostics-color=always -pipe -Wall -Winvalid-pch -Wnon-virtual-dtor -std=c++11 -O0 -g -Werror --pre-js prejs_file_api.js --post-js postjs_file_api.js -s WASM=1 -s DISABLE_EXCEPTION_CATCHING=0 -s "EXPORTED_RUNTIME_METHODS=['ALLOC_NORMAL','printErr','ALLOC_STACK','print']" -s DEMANGLE_SUPPORT=1 -s TOTAL_MEMORY=83886080 -s ALLOW_MEMORY_GROWTH=1 -lworkerfs.js

The good news is that this appears to compile the WASM and wrapper without complaining (they are both produced, and no error is shown). The not-so-good news is that this WASM is producing fatal exceptions when used within the test implementation on javascript-libzim. It initializes but throws an obscure exception after initialization. Then, attempting to load an article or do a full-text search produces a more informative exception chain, which is a mixture of JS and WebAssembly mimicking C++.The errors are clearly coming from the WASM, not the wrapper (text in white in the console below the red JS errors):

image


I'm afraid I haven't got much of a clue about how to fix this. I suspect that the dependencies are not being compiled with the right switches or configuration overrides for Emscripten, but I really don't know much more about all this than what appears in the Makefile on openzim/javascript-libzim. I sure hope you have more of a clue than I do... But it seems possible that the issue is in the precompiled binaries. What Emscripten configuration are you doing to compile these binaries?

I've had to learn a lot just to get to the stage of being able to test-compile this, but I really don't think I can fix this by myself. I can commit the built files and modified demo to a branch on the repo if that would be useful. Note I built this using the branch in my workflow PR: openzim/javascript-libzim#14, so the compiled WASM and wrapper are now named libzim-wasm.* (and are correctly linked in the demo), rather than a.out.*.

Feel free to move this comment to a new issue if you prefer, or to re-open this one if you agree that the issue is with the binaries and not with the binding.

@kelson42
Copy link
Contributor

kelson42 commented Dec 6, 2022

@mgautierfr @Jaifroid If I understand properly, this ticket seems to show that the work done so far to port/compule/distribute libzim wasm "binaries" is sofar useless/wortless for Kiwix JS. You confirm?

@Jaifroid
Copy link

Jaifroid commented Dec 6, 2022

@kelson42 I think "worthless" is harsh. It needs debugging, that's all. The fact that Emscripten can compile all the dependencies into a working prototype in openzim/javascript-libzim means that it can also be done on kiwix-build with some tweaking of switches/options when compiling the binary objects.

@kelson42
Copy link
Contributor

kelson42 commented Dec 6, 2022

@mgautierfr @Jaifroid OK, this bug is a blocker for moving forward with wasm. It's urgent to fix it.

@mgautierfr
Copy link
Collaborator Author

mgautierfr commented Dec 6, 2022

Should be fixed with kiwix/kiwix-build#555 (at least it is on my side)
Can you test with http://tmp.kiwix.org/ci/fix_wasm_build/libzim_wasm-emscripten-2022-12-06.tar.gz please ?

@Jaifroid
Copy link

Jaifroid commented Dec 6, 2022

That's good news, thank you! I'll test and report back here.

@Jaifroid
Copy link

Jaifroid commented Dec 6, 2022

On a quick compile of the dev versions using these binaries, the demo is working!

image

😊

@Jaifroid
Copy link

Jaifroid commented Dec 6, 2022

Full text search with optimized version also working in KJSWL:

image

@Jaifroid
Copy link

Jaifroid commented Dec 6, 2022

Also the ASM version (pure JavaScript, no WebAssembly).

@Jaifroid
Copy link

Jaifroid commented Dec 7, 2022

I've now done a completely clean build with the prebuilt binaries. All good.

My only query is very minor -- the organization of the gzipped archive. Why do we have a CPU architecture x86_64-linux-gnu inside the lib directory, when wasm is its own "architecture", and the wasm should work on any CPU? Wouldn't it make more sense for it to be called wasm, or not to have a subdirectory and merely include all the binaries inside lib? It seems to me that the designation x86_64 is a bit misleading.

image

That aside, as far as I'm concerned, this issue can be closed. Once we have nightly archives, and I know that the folder structure inside the archive is finalized, I can complete openzim/javascript-libzim#12 easily.

@mgautierfr
Copy link
Collaborator Author

Good news !!

I've merged kiwix/kiwix-build#555 so nightly should be available tomorrow.

About the library prefix it can be changed yes. I've used the one of the local machine to be sure they is no conflict and script trying to access file in the wrong prefix. But it is a issue for kiwix-build.

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

Successfully merging a pull request may close this issue.

3 participants