-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Generated WebAssembly unexpectedly requires reference types #128475
Comments
cc @alexcrichton @daxpedda Looks like this causes issues in practice and we might want to do nikic#1 after all? |
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-high |
I was unable to confirm the presence of any reference type instructions in the generated Wasm, even though I got the same error pointing towards a Interestingly enough, doing the back and forth conversion with I'm assuming that this is another example of a tool making use of the target feature section, so this was already anticipated. Unfortunately (?) this isn't the first time LLVM has enabled features by default, in the past they even did affect code generation. From what I've read in the various WebAssembly specification groups, this was always intended. In conclusion, I agree this is a form of breakage, it seems to be intended, I believe if Rust wants to do anything about it the solution needs to a bit more comprehensive. E.g. actually disable all target features by default (I don't know if this is feasible with LLVM, would need research). |
I just spend some time digging into this error message that The location pointed out by the error message does indeed contain a call to I also checked if So maybe there is some sort of codegen difference, but I am unable to actually find it. |
Could it be the encoding difference discussed in the description of this PR? llvm/llvm-project#90792
|
Yes, I just confirmed that's the issue. |
Yes the problem is the LEB-encoding of the table index immediate. Using
which shows that
I believe this is due to the the binary encoding of the LEB. This isn't reflected in the text format and most other tools don't handle this well, so it's a subtle error. That's also why convert-to-text-and-back produces a working binary.
To clarify, I don't believe that this is the case. I believe modifications to the target feature section are unrelated. Ok so the question now is what to do. I see a few possible routes:
There's not really a governing body per se around the Rust WebAssembly targets so there's not really a group of people we can point to and say "ok please tell us what to do". In lieu of that I would personally be tempted to go with route (1) because that keeps LLVM/C/C++ aligned with Rust and prevents divergence in the wasm ecosystem. Given the age of the So, concretely, I would propose:
|
Discussed in the compiler team triage meeting. We think @alexcrichton's plan is reasonable at present but if there are additional issues or complications (such as it being more difficult to create an MVP binary than just using |
Looks like there may be complications. I am endeavouring to add a test to CI which asserts that a particular compiler invocation will generate MVP features. So far roadblocks are:
Trying out the There's... a lot going on here apparently. Given the above I don't think that (2) I described above is a silver bullet by any means, even that seems doubtful to work. I need to investigate more what's going on here with all these features slinging all over the place. |
Testing shows that |
I've proposed #128511 to close this issue as "this change is expected". |
…jieyouxu Document WebAssembly target feature expectations This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation. The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo. Closes rust-lang#109807 Closes rust-lang#119811 Closes rust-lang#128475
…jieyouxu Document WebAssembly target feature expectations This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation. The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo. Closes rust-lang#109807 Closes rust-lang#119811 Closes rust-lang#128475
…jieyouxu Document WebAssembly target feature expectations This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation. The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo. Closes rust-lang#109807 Closes rust-lang#119811 Closes rust-lang#128475
…jieyouxu Document WebAssembly target feature expectations This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation. The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo. Closes rust-lang#109807 Closes rust-lang#119811 Closes rust-lang#128475
…jieyouxu Document WebAssembly target feature expectations This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation. The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo. Closes rust-lang#109807 Closes rust-lang#119811 Closes rust-lang#128475
Rollup merge of rust-lang#128511 - alexcrichton:doc-wasm-features, r=jieyouxu Document WebAssembly target feature expectations This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation. The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo. Closes rust-lang#109807 Closes rust-lang#119811 Closes rust-lang#128475
Code
This can be replicated with a minimal
test.rs
:Compile it to WASM with
rustc test.rs -o test.wasm --target wasm32-unknown-unknown
Then, check if the generated WASM uses reference types:
Version it worked on
It most recently worked on:
nightly-2024-07-30
(and previous stable rust versions)On those versions, compiling the above
test.rs
and then runningwasm-validate
with--disable-reference-types
doesn't fail.Version with regression
rustc --version --verbose
:@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged
This seems to have been caused by #127513
The compatibility note there says:
However, the generated code seems to be indeed affected by default. Interestingly, reference types seem to be used unnecessarily, as using
wasm2wat
to turn the generatedtest.wasm
into wat and thenwat2wasm
to turn it back into wasm removes the reference types.As-is, this breaks compatibility with the WebAssembly 1.0 spec.
The text was updated successfully, but these errors were encountered: