Linker error when building a proc-macro crate that uses `wasm-bindgen`

fjarri commented 1 year ago

Describe the Bug

Linker error when building a proc-macro crate that uses wasm-bindgen.

Steps to Reproduce

Cargo.toml:

[package]
name = "wasm-bindgen-test"
version = "0.1.0"
edition = "2021"

[lib]
proc-macro = true

[dependencies]
wasm-bindgen = "0.2.86"

src/lib.rs:

use wasm_bindgen::prelude::*;

Expected Behavior

cargo build succeeds.

Actual Behavior

Linker error on cargo build: Running cargo build produces:

error: linking with `cc` failed: exit status: 1
  |
  = note: LC_ALL="C" PATH="/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin:/root/.pyenv/shims:/root/.pyenv/bin:/root/.cargo/bin:/root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin" VSLANG="1033" "cc" "-Wl,--version-script=/tmp/rustcrllIR3/list" "-Wl,--no-undefined-version" "-m64" "/tmp/rustcrllIR3/symbols.o" "/root/wasm-bindgen-test/target/debug/deps/wasm_bindgen_test-af27ef030bf52524.1m1x0ambykx9eboc.rcgu.o" "/root/wasm-bindgen-test/target/debug/deps/wasm_bindgen_test-af27ef030bf52524.4ktes0bdj0fhuk3d.rcgu.rmeta" "/root/wasm-bindgen-test/target/debug/deps/wasm_bindgen_test-af27ef030bf52524.40vojdb1hc1dke77.rcgu.o" "-Wl,--as-needed" "-L" "/root/wasm-bindgen-test/target/debug/deps" "-L" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libproc_macro-1e0c4dcd95dfced1.rlib" "/root/wasm-bindgen-test/target/debug/deps/libwasm_bindgen-e447d81c61af4920.rlib" "/root/wasm-bindgen-test/target/debug/deps/libcfg_if-c1740dee434d7871.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-8389830094602f5a.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-41c1085b8c701d6f.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libobject-f733fcc57ce38b99.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libmemchr-6495ec9d4ce4f37d.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libaddr2line-1e3796360cca5b49.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libgimli-2e7f329b154436e1.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_demangle-1e1f5b8a84008aa8.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd_detect-cbcb223c64b13cf3.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libhashbrown-b40bc72e060a8196.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libminiz_oxide-1eb33ae9877d3c0f.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libadler-0335d894dd05bed7.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_alloc-076a893ead7e7ab5.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-2e924dd85b2e9d95.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcfg_if-7975ffb5e62386c4.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-285425b7cea12024.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-38694d775e998991.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-914eb40be05d8663.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-27094fcca7e14863.rlib" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-919e055b306699ae.rlib" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/root/wasm-bindgen-test/target/debug/deps/libwasm_bindgen_test-af27ef030bf52524.so" "-Wl,--gc-sections" "-shared" "-Wl,-z,relro,-z,now" "-nodefaultlibs"
  = note: /usr/bin/ld: __wbindgen_realloc: undefined version: 
          /usr/bin/ld: __wbindgen_malloc: undefined version: 
          /usr/bin/ld: __wbindgen_free: undefined version: 
          /usr/bin/ld: __wbindgen_exn_store: undefined version: 
          /usr/bin/ld: __externref_table_dealloc: undefined version: 
          /usr/bin/ld: __externref_table_alloc: undefined version: 
          /usr/bin/ld: __externref_heap_live_count: undefined version: 
          /usr/bin/ld: __externref_drop_slice: undefined version: 
          /usr/bin/ld: failed to set dynamic section sizes: bad value
          collect2: error: ld returned 1 exit status

Additional Context

Reproduces on: Ubuntu 20.04.3 LTS x86_64, rustc 1.70.0 (90c541806 2023-05-31) Works on MacOS 13.3.1, same rustc Works on the Ubuntu machine when downgrading to Rust 1.69:

rustup toolchain install 1.69
cargo +1.69 build

daxpedda commented 1 year ago

I think this is an issue with having to detect that this is a proc macro and not defining any FFI points. Though I'm surprised it doesn't do that already, as it should detect the target as not being wasm32-unknown-unknown already.

May I ask what the use-case is for this anyway?

fjarri commented 1 year ago

I think this is an issue with having to detect that this is a proc macro and not defining any FFI points.

I wonder then what changed in Rust 1.70 so that it stopped working? Also it seems that in some dependent crates it works if one sets opt-level=1 in the build profile, but I couldn't reproduce that in the MRE.

May I ask what the use-case is for this anyway?

The crate https://docs.rs/wasm-bindgen-derive which adds some missing functionality (handling Vec and Option arguments).

daxpedda commented 1 year ago

I wonder then what changed in Rust 1.70 so that it stopped working? Also it seems that in some dependent crates it works if one sets opt-level=1 in the build profile, but I couldn't reproduce that in the MRE.

I have no clue, pretty strange.

The crate https://docs.rs/wasm-bindgen-derive which adds some missing functionality (handling Vec and Option arguments).

Why would you depend on wasm-bindgen in the proc-macro though? For example right now wasm-bindgen-derive has two unused dependencies: wasm-bindgen and js-sys.

fjarri commented 1 year ago

Why would you depend on wasm-bindgen in the proc-macro though?

Good point :) That was my first proc-macro, and I guess I misunderstood the details of the build process. I would still like to restrict the wasm-bindgen version to >=0.2.85 for anyone who uses this crate, because it relies on some undocumented details, but I guess I can live without it. I'll do some tests with downstream crates later today, but most probably that will resolve the problems.

fjarri commented 1 year ago

On a second thought, while this does serve as a workaround, it is still very useful to have explicit dependencies in a proc-macro crate, even if they are technically "unused". The user of the proc-macro will have to have those, so it is much better to enforce it right away rather than rely on the user to explicitly add them. Plus, of course, there's an issue of the proper version bounds - even without the undocumented stuff, how do I communicate to the user "this proc-macro requires wasm-bindgen=0.2.*"? Much easier to do that through the dependencies.

daxpedda commented 1 year ago

The user of the proc-macro will have to have those, so it is much better to enforce it right away rather than rely on the user to explicitly add them.

You can't re-export the dependencies from your proc-macro crate, so it's pointless. The user will still have to explicitly add them.

Plus, of course, there's an issue of the proper version bounds - even without the undocumented stuff, how do I communicate to the user "this proc-macro requires wasm-bindgen=0.2.*"?

There is a disadvantage to "documenting by adding as a dependency", the user will have increased compile times. Proc-macros are compiled for the target the compiler is being run on, but dependencies that are for the application itself have to be compiled for the application target.

So in this case wasm-bindgen will have to be compiled for e.g. x86_64-unknown-linux-gnu and for wasm32-unknown-unknown, but only wasm32-unknown-unknown was actually needed, increasing the compile time.

In any case, this should be fixed nevertheless.

fjarri commented 1 year ago

I would personally take safety over compilation times, but I see your point. It seems that this problem is generally solved by somecrate depending on somecrate-derive (and re-exporting it gated by a feature), not the other way around as it is in my case. Any chance you could consider that?

Even better, of course, if those two issues (#2370 and #111) could be resolved, then the whole wasm-bindgen-derive would be unnecessary. I would try that, but I am not even sure where to start.

daxpedda commented 1 year ago

I would personally take safety over compilation times, [..]

What do you mean with "safety"? How do you gain safety by this approach?

It seems that this problem is generally solved by somecrate depending on somecrate-derive (and re-exporting it gated by a feature), not the other way around as it is in my case. Any chance you could consider that?

Do you mean gate wasm-bindgen-macro behind a crate feature in wasm-bindgen? That might actually be possible, but not right now because that would be a breaking change. I'm still not sure how that would help you.

fjarri commented 1 year ago

What do you mean with "safety"? How do you gain safety by this approach?

Suppose a crate that uses wasm-bindgen-derive has a dependency wasm-bindgen=0.2.84. If wasm-bindgen-derive has a dependency wasm-bindgen=0.2.85 (which it requires for the generated code to work correctly), then it will be resolved to wasm-bindgen>=0.2.85, and everything will work. If wasm-bindgen-derive does not specify a dependency, it may be resolved to wasm-bindgen=0.2.84, which will lead to strange-looking errors in runtime.

Do you mean gate wasm-bindgen-macro behind a crate feature in wasm-bindgen? That might actually be possible, but not right now because that would be a breaking change.

I don't think adding a feature would be considered a breaking change.

I'm still not sure how that would help you.

It will ensure that the code wasm-bindgen-derive generates will stay in sync with the API of wasm-bindgen without me needing to explicitly write in the docs "please use this version range for wasm-bindgen", which is highly unusual for Rust ecosystem, so nobody will bother looking there, and then their build will just randomly break at some point in the future.

daxpedda commented 1 year ago

Suppose a crate that uses wasm-bindgen-derive has a dependency wasm-bindgen=0.2.84. If wasm-bindgen-derive has a dependency wasm-bindgen=0.2.85 (which it requires for the generated code to work correctly), then it will be resolved to wasm-bindgen>=0.2.85, and everything will work. If wasm-bindgen-derive does not specify a dependency, it may be resolved to wasm-bindgen=0.2.84, which will lead to strange-looking errors in runtime.

Ah, yes! Got confused by the word "safety".

I don't think adding a feature would be considered a breaking change.

It is, because users are currently relying on this feature always being available, if it's gated behind a crate feature users will have to opt-in, which they currently can't, because the crate feature doesn't exist yet. So that's pretty breaking.

I think what you could do for wasm-bindgen-derive is what you explained before - split your crate in two:

wasm-bindgen-derive-macro: proc-macro crate.
wasm-bindgen-derive: reexport proc-macro and add wasm-bindgen as a dependency.

That way you make sure that users have the correct version of wasm-bindgen and you don't incur the compile-time penalty. syn is a huge dependency that adds a significant amount to users compile time, definitely worth avoiding it.

Just to clarify: wasm-bindgen not being able to compile as a dependency of a proc-macro crate is still a bug and should be fixed.

fjarri commented 1 year ago

because users are currently relying on this feature always being available

Uh, what feature? Neither derive(TryFromJsValue), nor using Vec or Option as arguments is currently available in wasm-bindgen.

daxpedda commented 1 year ago

It seems that this problem is generally solved by somecrate depending on somecrate-derive (and re-exporting it gated by a feature), not the other way around as it is in my case. Any chance you could consider that?

Do you mean gate wasm-bindgen-macro behind a crate feature in wasm-bindgen? That might actually be possible, but not right now because that would be a breaking change. I'm still not sure how that would help you.

I don't think adding a feature would be considered a breaking change.

It is, because users are currently relying on this feature always being available, if it's gated behind a crate feature users will have to opt-in, which they currently can't, because the crate feature doesn't exist yet. So that's pretty breaking.

Uh, what feature? Neither derive(TryFromJsValue), nor using Vec or Option as arguments is currently available in wasm-bindgen.

What? I'm confused :rofl:. I thought we are talking about adding a crate feature to wasm-bindgen gating the wasm-bindgen-macro dependency. This is most definitely a breaking change, it supplies the wasm_bindgen proc-macro, which many rely on.

fjarri commented 1 year ago

No, I meant adding a feature that would supply derive(TryFromJsValue)

daxpedda commented 1 year ago

You mean into wasm-bindgen? I assume with "feature" you don't mean crate feature, because that wouldn't be necessary.

I'm not sure exactly what it does, but if what it does is address #2370 and #111, then I would say no. I would rather fix the wasm_bindgen proc-macro instead of introducing a new derive macro.

fjarri commented 1 year ago

I would rather fix the wasm_bindgen proc-macro instead of introducing a new derive macro.

That would indeed be ideal. Also sorry for dragging on this thread :)

daxpedda commented 1 year ago

No problem at all! GitHub is probably not the right medium for something like this though. Please join us on Discord: https://discord.com/channels/442252698964721669/1110955325814747206.

rustwasm / wasm-bindgen