Skip to content

nightly-2025-02-18 wasm-opt cannot process resulting binary #137315

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

Closed
Ddystopia opened this issue Feb 20, 2025 · 12 comments · Fixed by #137322
Closed

nightly-2025-02-18 wasm-opt cannot process resulting binary #137315

Ddystopia opened this issue Feb 20, 2025 · 12 comments · Fixed by #137322
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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

@Ddystopia
Copy link
Contributor

I bisected by hand and at nightly-2025-02-18 wasm-opt started giving errors like this:

...
[wasm-validator error in function 2590] unexpected false: Bulk memory operations require bulk memory [--enable-bulk-memory], on
(memory.copy
 (local.get $0)
 (i32.add
  (local.get $4)
  (i32.const 8)
 )
 (i32.const 40)
)
[wasm-validator error in function 2621] unexpected false: Bulk memory operations require bulk memory [--enable-bulk-memory], on
(memory.copy
 (i32.add
  (local.get $2)
  (i32.shl
   (local.get $8)
   (i32.const 2)
  )
 )
 (i32.add
  (local.get $2)
  (i32.shl
   (local.get $0)
   (i32.const 2)
  )
 )
 (local.get $5)
)
...

It asks for --enable-bulk-memory, if give it to it, other errors reveal (this is the whole output):

> wasm-opt -Oz project.wasm -o new.wasm --enable-bulk-memory
[wasm-validator error in function 520] unexpected false: all used features should be allowed, on
(i32.trunc_sat_f32_s
 (f32.mul
  (local.get $13)
  (f32.const 20)
 )
)
[wasm-validator error in function 520] unexpected false: all used features should be allowed, on
(i32.trunc_sat_f32_u
 (local.tee $13
  (f32.add
   (local.get $13)
   (f32.const 60)
  )
 )
)
[wasm-validator error in function 1103] unexpected false: all used features should be allowed, on
(i32.trunc_sat_f64_u
 (local.tee $4
  (call $fimport$187
   (i32.load
    (i32.load
     (local.get $1)
    )
   )
   (i32.load offset=12
    (local.get $2)
   )
  )
 )
)
[wasm-validator error in function 1120] unexpected false: all used features should be allowed, on
(i64.trunc_sat_f64_s
 (local.get $4)
)
[wasm-validator error in function 1908] unexpected false: all used features should be allowed, on
(i64.trunc_sat_f64_u
 (local.get $9)
)
Fatal: error validating input

For wasm-opt I tried versions form 116 to 120, all failed.

I am compiling web application with dioxus, it uses wasm-bindgen.

@Ddystopia Ddystopia added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Feb 20, 2025
@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 Feb 20, 2025
@saethlin
Copy link
Member

Can you share a reproducer?

I bisected by hand and at nightly-2025-02-18

Can you bisect using cargo-bisect-rustc to find the relevant PR?

@saethlin saethlin added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example O-wasm Target: WASM (WebAssembly), http://webassembly.org/ and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 20, 2025
@nikic
Copy link
Contributor

nikic commented Feb 20, 2025

This is due to the LLVM 20 upgrade, which enables bulk-memory and nontrapping-fptoint by default. Need to update https://doc.rust-lang.org/rustc/platform-support/wasm32-unknown-unknown.html#enabled-webassembly-features.

cc @alexcrichton

@saethlin saethlin added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. and removed E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress. labels Feb 20, 2025
@alexcrichton
Copy link
Member

I've sent #137322 for an update to the docs. Historically though otherwise this has generally been a "wontfix" issue in the sense that it's expected that new features will be enabled-by-default over time and downstream tooling should be expecting this.

@Ddystopia are you running wasm-opt by hand? If so it might be good to raise an issue over there to have these proposals enabled by default. If not it might be reasonable to file an issue with the tool invoking wasm-opt to pass these --enable-* flags by default.

@saethlin saethlin removed the C-bug Category: This is a bug. label Feb 20, 2025
@alexcrichton
Copy link
Member

Oh I can also mention, the link that @nikic posted also has instructions/documentation for turning off on-by-default features if you find that helpful as well @Ddystopia

@jieyouxu
Copy link
Member

I'm going to unmark this as a regression since it's expected that LLVM will continue to enable wasm features that become sufficiently baked.

@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools 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. labels Feb 20, 2025
@Ddystopia
Copy link
Contributor Author

I will open corresponding issues. Should that issue be closed, considering that it is expected?

@jieyouxu
Copy link
Member

jieyouxu commented Feb 20, 2025

I edited #137322's PR description to have that close this issue w/ the doc part once that merges.

workingjubilee pushed a commit to workingjubilee/rustc that referenced this issue Feb 20, 2025
LLVM 20 enabled the `nontrapping-fptoint` and `bulk-memory` features by
default, so this updates the corresponding documentation for the
`wasm32-*` targets (which all point to `wasm32-unknown-unknown`).

cc rust-lang#137315
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Feb 20, 2025
…ieyouxu

Update docs for default features of wasm targets

LLVM 20 enabled the `nontrapping-fptoint` and `bulk-memory` features by default, so this updates the corresponding documentation for the `wasm32-*` targets (which all point to `wasm32-unknown-unknown`).

Closes rust-lang#137315 with a doc update for the doc part.
@eric-seppanen
Copy link

eric-seppanen commented Feb 21, 2025

A lot of downstream tools (e.g. trunk) call out to wasm-opt, and it causes a lot of downstream build breakage when new wasm features get turned on. The error messages are uninformative and it's not always obvious what the fix is.

So far the discussion here hasn't led to a clear way forward. It would be nice if wasm-opt weren't so critical to the rust-webassembly ecosystem, but in the meantime it seems like this should be called out in the release notes so people aren't surprised by the breakage.

@eric-seppanen
Copy link

It would be very helpful if someone with deep webassembly knowledge could weigh in on trunk-rs/trunk#904 .

Would any of these options be the recommended way forward? Should trunk enable all the latest things by default as soon as llvm/rustc does?

@bors bors closed this as completed in 3d5e773 Feb 21, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 21, 2025
Rollup merge of rust-lang#137322 - alexcrichton:update-wasm-docs, r=jieyouxu

Update docs for default features of wasm targets

LLVM 20 enabled the `nontrapping-fptoint` and `bulk-memory` features by default, so this updates the corresponding documentation for the `wasm32-*` targets (which all point to `wasm32-unknown-unknown`).

Closes rust-lang#137315 with a doc update for the doc part.
@alexcrichton
Copy link
Member

@eric-seppanen have you reached out to the wasm-opt/binaryen folks about this? I suspect they'd be accomodating to the suggestion to enable these proposals by default to match LLVM

@eric-seppanen
Copy link

@eric-seppanen have you reached out to the wasm-opt/binaryen folks about this? I suspect they'd be accomodating to the suggestion to enable these proposals by default to match LLVM

@alexcrichton
I don't really understand what request should be made. I'm not terribly familiar with the webassembly features, and I'm not a trunk maintainer. What would it mean to say "enable these proposals by default to match LLVM"? Does that mean that each wasm-opt release has a set of defaults that matches a particular llvm version? Or that it should inspect the incoming binaries and somehow determine which set of features to enable for its output? Is this problem specific to rust, or do I need to understand how other languages handle this issue?

@alexcrichton
Copy link
Member

I'd recommend making a request that the default set of enabled features in binaryen either be (a) proposals that are stage 4+ or (b) proposals that are on-by-default in LLVM. In wasmtime/wasm-tools we do (a) and find that it works well for our use cases, but binaryen might have other use cases as well.

In my mind if binaryen has good reason for not enabling a feature by default then that should be a factor in a decision to enable something in LLVM by default. Often though I find that things aren't on-by-default because it's just forgotten about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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

Successfully merging a pull request may close this issue.

7 participants