Add some option to disable the wasm-shim

[juntyr]

Thank you for your great library! I am compiling zstd-sys for wasm32-wasi in an environment with wasi-sdk, which exposes the WASI stdlib to C and C++. I am also trying to not have any additional WASM imports in the resulting module. Unfortunately, building with the current wasm-shim results in several symbols needing to be imported, e.g. env::rust_zstd_wasm_shim_malloc. If I instead remove both the C and Rust sides of the wasm-shim, everything compiles (with wasi-sdk) and runs without issues. Could there perhaps be some way to disable the wasm-shim / to fix the underlying issue that zstd-sys ends up with several symbols needing to be imported? Thanks for your help!

[gyscos]

Hi, and thanks for the report!

I can look into adding a no-wasm-shim feature, but I'm curious why it's not working for you. rust_zstd_wasm_shim_malloc should be provided by the zstd-sys crate itself in the wasm_shim module, it shouldn't be a required import. :confused:

[gyscos]

Added a no_wasm_shim feature to zstd-sys.

$ export CC="${WASI_SDK_PATH}/bin/clang --sysroot=${WASI_SDK_PATH}/share/wasi-sysroot"
$ # Building without the shims. There are some warnings re: the usage of clock(), which can be ignored (the clock is only used for reporting).
$ cargo build --target wasm32-wasi --features no_wasm_shim
warning: zstd/lib/dictBuilder/cover.c:728:5: warning: 'clock' is deprecated: WASI lacks process-associated clocks; to enable emulation of the `clock` function using the wall clock, which isn't sensitive to whether the program is running or suspended, compile with -D_WASI_EMULATED_PROCESS_CLOCKS and link with -lwasi-emulated-process-clocks [-Wdeprecated-declarations]
warning:     DISPLAYUPDATE(
warning:     ^
warning: zstd/lib/dictBuilder/cover.c:87:31: note: expanded from macro 'DISPLAYUPDATE'
warning: #define DISPLAYUPDATE(l, ...) LOCALDISPLAYUPDATE(g_displayLevel, l, __VA_ARGS__)
warning:                               ^
warning: zstd/lib/dictBuilder/cover.c:81:10: note: expanded from macro 'LOCALDISPLAYUPDATE'
warning:     if ((clock() - g_time > g_refreshRate) || (displayLevel >= 4)) {             \
warning:          ^
warning: /opt/wasi-sdk/share/wasi-sysroot/include/time.h:66:16: note: 'clock' has been explicitly marked deprecated here
warning: __attribute__((__deprecated__(
warning:                ^
...
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
$ # Though note that this works too:
$ cargo build --target wasm32-wasi

[juntyr]

Thank you for investigating as well! If you look at your wasm32 output module with wasm2wat, do you still see the imports? I also agree solving that problem would be preferable, but I greatly appreciate the new feature as a fix!

[gyscos]

(Disclaimer: I have zero experience building stuff for wasm.)

There is a it_work example in zstd_sys. I can try building it with cargo wasi build without any feature (so it should use the shims).

I then see 3 .wasm files in the target folder: it_work.wasm, it_work.rustc.wasm, and it_work.wasi.wasm. I checked them all and none of them include extra imports. Here's the example for one of them:

$ cargo wasi build --example it_work
$ wasm2wat target/wasm32-wasi/debug/examples/it_work.wasi.wasm | rg import
  (import "wasi_snapshot_preview1" "fd_write" (func $wasi::lib_generated::wasi_snapshot_preview1::fd_write::h4af746c5c9249244 (type 11)))
  (import "wasi_snapshot_preview1" "environ_get" (func $__imported_wasi_snapshot_preview1_environ_get (type 6)))
  (import "wasi_snapshot_preview1" "environ_sizes_get" (func $__imported_wasi_snapshot_preview1_environ_sizes_get (type 6)))
  (import "wasi_snapshot_preview1" "proc_exit" (func $__imported_wasi_snapshot_preview1_proc_exit (type 2)))
    call $__imported_wasi_snapshot_preview1_environ_get
    call $__imported_wasi_snapshot_preview1_environ_sizes_get
    call $__imported_wasi_snapshot_preview1_proc_exit

Using cargo build --example it_work --target wasm32-wasi instead of cargo wasi build does not result in the .rustc.wasm or .wasi.wasm, but it_work.wasm is still there and also has no extra import.

[juntyr]

This issue is getting weirder. I have two different WASM modules that both depend on zstd-sys:

• the first one depends on zstd-rs and does not experience any issues, as all imports are resolved
• the second one depends on sz3-rs, which depends on zstd-sys, and this one does experience the import problems

Both set default-features = false. I also tried downgrading a few versions, and zstd-sys 2.0.7-2.0.9 all experience this issue. I have no idea what to make of this ...

[juntyr]

Perhaps this discrepancy could be since I use the stream API from zstd but perhaps sz3 uses some code path that does need to allocate?

[juntyr]

Looking at both output modules again, I can see that the first does export all symbols from the wasm-shim, while they are missing from the second. I might have an idea, but will see if it's the right one.

[juntyr]

Ok, this seems to be an issue in Rust itself (https://github.com/rust-lang/rust/issues/50007 is the closest I could find, even though that one says it is fixed). The issue can be fixed by re-exporting the wasm-shim symbols into the top-level shared library.

Thank you so much for your help investigating this issue! For now, I would recommend to change the following:

https://github.com/gyscos/zstd-rs/blob/92f87dca4987555fe8a87504dea68b37a29d62b9/zstd-safe/zstd-sys/src/lib.rs#L12-L13

to:

#[cfg(all(not(feature = "no_wasm_shim"), target_arch = "wasm32"))]
#[doc(hidden)]
// If you come across an error similar to 
// "cannot find definition for import env::rust_zstd_wasm_shim_malloc"
// when loading a wasm module with zstd-sys, try re-exporting all symbols
// in this module in your top-level WASM shared library module.
pub mod wasm_shim;

[juntyr]

I've been thinking about it some more and here is my best understanding. zstd-sys provides the compiled C library libzstd.a, the Rust bindings, and the WASM shim. When using it through zstd or zstd-safe, Rust sees that we're using both the Rust library and the linked C library, and so the wasm-shim is compiled in and everything works.

From my understanding of sz3-rs's build script, it depends on zstd-sys but then only links to the libzstd.a C library in its own C library compilation process, but never touches the Rust bindings for Zstd. My gut feeling is that Rust therefore thinks that the Rust bindings aren't used, doesn't compile them in, and thus the wasm-shim symbols are suddenly unresolved.

As I am currently using my own for of sz3-rs, I can handle this case as long as I can re-export the wasm-shim symbols in some Rust code that is actually used. However, I wonder if this is something that could be fixed on the zstd-sys side. We would need to find some way to convince rustc to include the Rust-side wasm-shim even when the Rust bindings in zstd-sys are seemingly never used.

[juntyr]

I stumbled across https://github.com/rust-lang/rust/issues/94348#issuecomment-1079770043, which seems to work for me - just add extern crate zstd_sys; somewhere in the import hierarchy, which seemingly forces rustc to link the crate even even when I'm not using any of its Rust bindings inside Rust code.

With this, my new suggestion would be to change my above suggestion to

#[cfg(all(not(feature = "no_wasm_shim"), target_arch = "wasm32"))]
// If you come across an error similar to 
// "cannot find definition for import env::rust_zstd_wasm_shim_malloc"
// when loading a wasm module with zstd-sys, try adding an
// `extern crate zstd_sys` to your crate.
mod wasm_shim;