Disable Wasm unstable target features according to Rust

[daxpedda]

As discussed in rust-lang#127513 (comment).
I decided to use negative target features instead of switching to mvp because that would still allow users to use mvp while expecting no target features to be active. Though this breaks bleeding-edge, which probably is irrelevant for Rust.

I also went ahead and removed +mutable-globals and +sign-ext, as these are enabled by default now.

[daxpedda]

Cc @alexcrichton if you don't mind double-checking this as well.

[alexcrichton]

I think we'll instead want to keep alignment with Clang's defaults by keeping these features enabled by default. I don't believe that leaving them enabled by default breaks anything though, but do you know of something that's breaking?

I'm under the impression that reference-types doesn't do anything unless you explicitly do "interesting things" with LLVM IR which rustc doesn't. For multi-value I'm under the impression that Clang has changed to only looking at it if experimental-mv is the target abi, which rustc also doesn't set, so I think that both are noops effectively.

[daxpedda]

I'm under the impression that reference-types doesn't do anything unless you explicitly do "interesting things" with LLVM IR which rustc doesn't.

I've confirmed before that this is correct.

For multi-value I'm under the impression that Clang has changed to only looking at it if experimental-mv is the target abi, which rustc also doesn't set, so I think that both are noops effectively.

This does change things without experimental-mv actually, non-C ABI functions are able to use multivalue returns. This affects Rust functions used by the Wasm module internally.

While I think this isn't an issue if we go ahead and enable it by default, I don't know what our plans are with +multivalue in the future, we might want to enable experimental-mvby default at some point when using +multivalue (not sure what LLVMs plans are here exactly) , which would be a breaking change in the C ABI.

If this is unlikely to ever happen because we would like to keep the C ABI without multivalue returns and re-introduce a Wasm ABI as before for multivalue returns, then I agree this is all probably moot.

[alexcrichton]

Oh so the internal ABI of LLVM changes things with +multivalue? Do you have an example I could poke at?

As for the long-term story, the high-order bit is to match Clang/wasi-libc. Maintainers for those all understand that changing the ABI is not feasible at this time and it probably won't happen any time soon. Exactly how it might happen I don't think anyone reall knows at this point.

My guess is that a new ABI will be added for multi-return and it will be usable on an opt-in basis. What exactly it's called (e.g. maybe "wasm" maybe not, depends on coordinating with Clang) I'm not sure but for the time being no one wants to change the current C ABI by default.

[daxpedda]

Oh so the internal ABI of LLVM changes things with +multivalue? Do you have an example I could poke at?

I don't know about an internal ABI, I meant the Rust ABI used when not specifying one.

extern "C" {
    fn test_1() -> (i32, i32);
}

#[no_mangle]
extern "C" fn test_2() -> (i32, i32) {
    internal()
}

#[inline(never)]
fn internal() -> (i32, i32) {
    unsafe { test_1() }
}

Simplified:

  (func $test_1 (import "env" "test_1") (param i32))
  (func $test_2 (param i32)
    ...
    call $internal
    ...
  )
  (func $internal (result i32 i32)
    ...
    call $test_1
    ...
  )

My guess is that a new ABI will be added for multi-return and it will be usable on an opt-in basis. What exactly it's called (e.g. maybe "wasm" maybe not, depends on coordinating with Clang) I'm not sure but for the time being no one wants to change the current C ABI by default.

Sounds good!
Gonna go ahead and close the PR then.

EDIT: Compiled with -Zwasm-c-abi=spec of course.

---

@nikic same as last time we shouldn't forget to mention that these features are enabled by default now in the changelog. E.g. even if users don't use the reference types proposal because Rust doesn't support it yet, the Wasm module is marked as such and will be declined by Wasm engines without support for it.

[alexcrichton]

I meant the Rust ABI used when not specifying one.

Aha makes sense!

even if users don't use the reference types proposal because Rust doesn't support it yet, the Wasm module is marked as such and will be declined by Wasm engines without support for it.

Is this true though? I don't think anything in the module will be using anything from the reference-types proposal by default?

(it'll definitely use multi-value as your changes show, though)

[daxpedda]

even if users don't use the reference types proposal because Rust doesn't support it yet, the Wasm module is marked as such and will be declined by Wasm engines without support for it.

Is this true though? I don't think anything in the module will be using anything from the reference-types proposal by default?

Yes, even if nothing uses the reference types proposal, the Wasm module will be marked as requiring it nevertheless.

You can test it with wasm-objdump foo.wasm -x -j target_features:

Custom:
 - name: "target_features"
  - [+] mutable-globals
  - [+] reference-types
  - [+] sign-ext

[alexcrichton]

Ah ok that makes sense, but that doesn't actually affect engines since it's just a custom section. Engines typically validate based on contents other than that.

[daxpedda]

Ah ok that makes sense, but that doesn't actually affect engines since it's just a custom section. Engines typically validate based on contents other than that.

This custom section has been well defined by the Wasm tool conventions. I can't speak for Wasmtime, but Chrome and Firefox reject Wasm modules depending on the content of the target_features custom section.

[alexcrichton]

Oh? Do you know of the source code for that? That would be surprising news to me, and for Firefox I don't see anything in wasm reading anything related to target_features

[daxpedda]

Ah ok that makes sense, but that doesn't actually affect engines since it's just a custom section. Engines typically validate based on contents other than that.

This custom section has been well defined by the Wasm tool conventions. I can't speak for Wasmtime, but Chrome and Firefox reject Wasm modules depending on the content of the target_features custom section.

Whoops, just tested it again and it works, probably just remembered it wrongly?
I know that other things read the target_features custom section, e.g. wasm-opt does for example.

EDIT: Binaryen source. E.g. wasm-opt fails if these aren't set correctly, which lead to rustwasm/wasm-bindgen#3967.

[CryZe]

In the other PR @alexcrichton wrote The WebAssembly target features multivalue [..] enabled by default, but generated code is not affected by default, but didn't we conclude that the Rust ABI will still make use of multivalue? That's definitely how I'm seeing it working on the current Rust compiler, but I'm not sure it still behaves like that with LLVM 19, not sure if the testing @daxpedda did was with LLVM 18 or 19.

[alexcrichton]

Ah in clarifying with @daxpedda that was using Nightly rust today, so LLVM 18. With LLVM 19 the extern "Rust" ABI, nor any other, will be affected.

[CryZe]

Ah alright then. Fairly unfortunate, but I guess that's just how it is then for now.