Skip to content
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

rust 1.82.0 wasm32-unknown-unknown triggers issues with wasm-bindgen >= 0.2.94 #132620

Closed
eric-seppanen opened this issue Nov 5, 2024 · 8 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eric-seppanen
Copy link

eric-seppanen commented Nov 5, 2024

I found that a wasm project that builds under Trunk with Rust 1.81.0, no longer builds with Rust 1.82.0. This seems to be because rustc now emits wasm "bulk memory" instructions that Trunk's bundled wasm-opt can't deal with. I haven't found any documented reason why this changed between 1.81.0 and 1.82.0, so I fear it may have been accidentally introduced by the switch to LLVM 19, which was documented to enable two other target-features (but not "bulk memory").

Code

I minimized an affected project, which can be found here: https://github.com/eric-seppanen/wasm_test_2024
It's not very well minimized, because the problem appears when using the gloo crate, which has a large dependency tree.

I expected to see this happen: project builds under rust 1.82.0, just like it does under 1.81.0, and the emitted code is compatible with Trunk and wasm-opt v116 that is bundled with Trunk.

Instead, this happened: trunk fails to build because:

  • rustc emitted wasm code containing "bulk memory" instructions
  • trunk's bundled wasm-opt binary does not understand "bulk memory" instructions.

The rustc book contains a list of wasm target features that are enabled by default. The bulk-memory feature is not listed among them.

Version it worked on

It most recently worked on: 1.81.0

Version with regression

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: x86_64-unknown-linux-gnu
release: 1.82.0
LLVM version: 19.1.1

I also filed an issue with Trunk.

@eric-seppanen eric-seppanen added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Nov 5, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 5, 2024
@eric-seppanen
Copy link
Author

If I use RUSTFLAGS='-C target-feature=-bulk-memory' I don't see any improvement, so I guess this means that the offending instructions are coming from the standard library?

I can't tell for sure, but this comment may be confirming this.

@jieyouxu jieyouxu added the O-wasm Target: WASM (WebAssembly), http://webassembly.org/ label Nov 5, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Nov 5, 2024

cc @alexcrichton , though I kinda expect this to be one of the wasm proposals that has become enabled-by-default by llvm

@CryZe
Copy link
Contributor

CryZe commented Nov 5, 2024

LLVM has enabled it on October 25th llvm/llvm-project#112049 but that's after the compiler's commit date, so that can't have been it. Without having seen the wasm / wat file (would be great if you put that in the repository as well), I'd say it's very likely to just be the expected fallout from https://blog.rust-lang.org/2024/09/24/webassembly-targets-change-in-default-target-features.html#enabling-reference-types-by-default caused by an outdated wasm-opt.

@nikic
Copy link
Contributor

nikic commented Nov 5, 2024

Yeah, bulk-memory is supposed to be enabled in LLVM 20 only, not in LLVM 19.

@jieyouxu jieyouxu added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. labels Nov 5, 2024
@alexcrichton
Copy link
Member

@eric-seppanen the error message in the linked issue is:

[parse exception: invalid code after misc prefix: 17 (at 0:20646)]

where here "misc prefix" is 0xfc and 17 as the opcode means that this is a table.fill instruction. I don't believe LLVM ever emits the table.fill instruction right now, so are you perhaps running into something related to wasm-bindgen instead? I believe that wasm-bindgen auto-enabled reference-types/bulk-memory according to rustwasm/wasm-bindgen#4213 and are you perhaps running into that?

If you think you're not running into this I think it would be best to get a reproduction which involves running cargo/rustc directly and then inspecting the output binary to ensure there aren't any other tools in play here.

@eric-seppanen
Copy link
Author

@alexcrichton thanks for the help. My build process certainly invokes wasm-bindgen (both with and without Trunk).

I tried downgrading the wasm-bindgen library dependency and the wasm-bindgen-cli binary to 0.2.93 and that does allow the project to build with rust 1.82.0.

If I update wasm-bindgen and wasm-bindgen-cli to 0.2.95 (0.2.94 was yanked) then the project builds under rust 1.81.0 but not 1.82.0.

If I understand correctly, this means there's something about the code emitted by rust 1.82.0 that triggers the problem in wasm-bindgen 0.2.95. If that's the case, then I can try to move the discussion over there.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 5, 2024
@alexcrichton
Copy link
Member

Yes Rust 1.82 would emit +reference-types in the target-features custom section of the output wasm (that section has always been there and it's just more features being added there). I believe that wasm-bindgen used that as an indication to automatically use other transforms that enables, for example, table.fill. That would be why wasm-bindgen first started "breaking" with Rust 1.82.

AFAIK this behavior is being disabled in wasm-bindgen, but you can also confirm with maintainers that this is the case.

@eric-seppanen eric-seppanen changed the title rust 1.82.0 wasm32-unknown-unknown seems to have enabled the bulk-memory target-feature rust 1.82.0 wasm32-unknown-unknown triggers issues with wasm-bindgen >= 0.2.94 Nov 5, 2024
@eric-seppanen
Copy link
Author

Since this will be resolved in the next release (0.2.96?) of wasm-bindgen, I think this can be closed.

@jieyouxu jieyouxu added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status C-bug Category: This is a bug. A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. labels Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants