Mismatched target feature detection when compiling C dependencies

[Spxg]

This commit, https://github.com/wasm-bindgen/wasm-bindgen/pull/4642, changes the externref symbol to only exists when the reference-types target-feature is enabled.

Given that wasm-bindgen determines whether reference-types is enabled in the target_feature section, this means that if a .o file has it enabled, the final product will also have it enabled, causing wasm-bindgen to fail:

error: failed to find intrinsics to enable `clone_ref` function
error: failed to find the __wbindgen_externref_table_alloc function

This issue will be reproduced if you use ring, zstd, and other crates that require compiled C code, and use a rustc version prior to 1.82:

use wasm_bindgen::prelude::wasm_bindgen;

#[wasm_bindgen]
pub fn issue() -> u32 {
    unsafe { zstd_sys::ZDICT_isError(0) }
}

The simple solution is to add CFLAGS to disable:

export CFLAGS_wasm32_unknown_unknown="-mno-reference-types"

Perhaps this compatibility needs to be guaranteed by crate developers or cc-rs, but wasm-bindgen 0.2.102 does introduce a "breaking change" for this scenario.

[daxpedda]

Hm, I think this is an acceptable breaking change?

Basically you can't simply mix-and-match target features between compilation files if those target features change the ABI. In this case, the better solution might be to make sure that if you are using an old version of Rust, you also have to use an old version of Clang. The wasm32-unknown-unknown target between those too has to actually match up.

[Spxg]

is an acceptable breaking change

Probably not, especially considering the widely use of crates such as ring and zstd-sys that require compiled C code.

In this case, the better solution might be to make sure that if you are using an old version of Rust, you also have to use an old version of Clang

Most users will hardly notice this, for example, which clang version has "reference-types" disable by default? Maybe there is a similar tool that can do this, I know nothing about clang version management.

[daxpedda]

is an acceptable breaking change

Probably not, especially considering the widely use of crates such as ring and zstd-sys that require compiled C code.

Well, this doesn't outright break any compatibility with ring or other compiled C code. Just on target mismatch.

In this case, the better solution might be to make sure that if you are using an old version of Rust, you also have to use an old version of Clang

Most users will hardly notice this, for example, which clang version has "reference-types" disable by default? Maybe there is a similar tool that can do this, I know nothing about clang version management.

They have to match up LLVM versions, basically. You can look up which LLVM version is used by which Rust version and match up the Clang version. See the compatibility table for linker-plugin-based LTO.

---

The thing is, that we should actually treat this as a bug. We shouldn't allow target feature mismatch between compilation files to begin with. Unfortunately this is out of our control. Its definitely fine with some features, e.g. SIMD, but its not fine with others, e.g. reference types. Famously, this is a big problem with multivalue.

[Spxg]

They have to match up LLVM versions, basically. You can look up with LLVM version is used by which Rust version and match up the Clang version.

Well, rustc --version --verbose shows the llvm version.

Rust 1.82 uses LLVM 19, which is also the version of Rust that enables reference-types by default, I think the aligned-version can be a solution.

[daxpedda]

While this is otherwise non-actionable by us, we should at least document this somewhere.

We could actually introduce a proper error message here: we could encode which target features are enabled by Rust in wasm-bindgen. If the post-processor finds additional target features in the Wasm module output, we can return an error explaining the issue and how users might proceed.

[RReverser]

I agree with everything @daxpedda said, just want to add that cc-rs already does a lot of translation of Rust flags to C flags, although Wasm logic in particular is rather limited.

In theory we could extend it to combine -mcpu=mvp from the wasm32v1-none support - https://github.com/rust-lang/cc-rs/blob/d740f9b1f5d65b09ccac41cac2e40caa8958e348/src/lib.rs#L2484 - with reading CARGO_CFG_TARGET_FEATURES like it already does in bunch of other places, and enabling post-MVP features for Clang only if they are in Cargo.

This still wouldn't help with more significant ABI mismatch, like the one we had until Rust 1.81, but it should make matching features easier for future versions.

Alternatively, we could only do this for the few target features we know to be problematic when mismatched (reference types, multivalue, probably exceptions) but that would be more error prone long-term.

[Spxg]

This commit, #4642, changes the externref symbol to only exists when the reference-types target-feature is enabled.

I found another issue: under Rust 1.82, cfg!(target_features = "reference-types") is false, but the final product had the reference-types target_feature.

This causes wasm-bindgen to fail unconditionally in 1.82, regardless of whether or not a C library is present.

I suspect this may be because some of the precompiled Rust products have reference-types enabled.

[RReverser]

Hmm that does sound bad. I'm happy to revert but we'd need a repro for CI.

[mohe2015]

This seems to also break dioxus hotpatching.

[daxpedda]

I found another issue: under Rust 1.82, cfg!(target_features = "reference-types") is false, but the final product had the reference-types target_feature.

So v1.82 is where reference-types is enabled by default. But its only stabilized at v1.84. Just confirmed that we can't actually detect it via cfg(target_feature = "reference-types") before its stabilized.

@RReverser I believe we should revert at this point.

[RReverser]

@RReverser I believe we should revert at this point.

Yup, already agreed above, I'd just like to have a test but we can revert asap anyway.

Not being able to rely on target_feature overall sounds pretty bad long-term though. E.g. we also rely on it for atomics, but I assume we don't see similar failures because it's unstable anyway.

[daxpedda]

[..], I'd just like to have a test but we can revert asap anyway.

I think the test is just running it with Rust v1.82.

Not being able to rely on target_feature overall sounds pretty bad long-term though. E.g. we also rely on it for atomics, but I assume we don't see similar failures because it's unstable anyway.

I think in the future we should be more careful in Rust to stabilize target features when they are enabled by default and not afterwards. E.g. in the case of reference-types nothing was stopping us from stabilizing them, we just didn't know what kind of problems this would cause.

[RReverser]

I think the test is just running it with Rust v1.82.

Yup, that's perfectly fine to add.

I think in the future we should be more careful in Rust to stabilize target features when they are enabled by default and not afterwards.

Do you mean in Rust itself (upstream)?

[daxpedda]

I think in the future we should be more careful in Rust to stabilize target features when they are enabled by default and not afterwards.

Do you mean in Rust itself (upstream)?

Yep.

[tyilo]

I got a similar error after updating wasm-bindgen:

error: failed to find the `__wbindgen_externref_table_dealloc` function

It seems to trigger when RUSTFLAGS="-C target-cpu=native" is used (which I have set globally).

[RReverser]

I got a similar error after updating wasm-bindgen:

error: failed to find the `__wbindgen_externref_table_dealloc` function

It seems to trigger when RUSTFLAGS="-C target-cpu=native" is used (which I have set globally).

That's definitely problematic for any kind of cross-compilation. You should instead set something like

[target.x86_64-unknown-linux-gnu]
rustflags = ["-Ctarget-cpu=native"]

in your ~/.cargo/config.toml, that's what I'm doing.