It surprised me that the existing examples/wasm_threads application compiled with trunk --release. So just to make sure this feature was actually needed, I used the built-from-src trunk on my other project that I'm working on that failed in #866, and indeed it still failed. I even checked the logs and confirmed that the same exact version of wasm-opt was being used, with the same CLI flags.

So evidently there is some difference between my wasm module and the wasm module produced by examples/wasm_threads, and this difference causes different behavior in wasm-opt that ends up making it hit those validation errors from #866.

What's really confusing is that when I take the target/wasm-bindgen/release/ artifacts from my app and the examples/wasm_threads app and run them through wasm2wat, it shows that they both use the threads and bulk-memory features. So I don't really know what's up with my app compared to the example, apart from it's a non-trivial app that has lots of dependencies.

But regardless of all that confusion, trunk --release on my app with this new data-threads attribute does fix the problem I was having in #866. So I believe this feature is still necessary.

Thanks for the PR. I think it's good to have control over those settings. However, I'd prefer that one can enable them independently. So maybe an enum with flags that can be enabled for wasm-opt? As maybe someone wants to enable some of them without "threads" in mind.

At that point, what do you think of just having a data-wasm-opt-flags attribute?

Huh. I also can't see anything obvious. Is that failing code available somewhere?

My own little project with all flags enabled has some 330 dependencies on wasm, among them are currently embassy-rs, which is doing same nasty linker abuses, but it still builds just fine with trunk.

Do you depend on wasm-bindgen? I've seen that cause issues, which I solved by defining a higher-than-default wasm_opt here, It seems to me that trunk takes its wasm-bindgen dependency from there, if defined (@ctron ?). But IIRC these errors looked different.

I cant seem to be able to go back to an old version of wasm_opt, if I switch back to an old version in Trunk.toml it seems to ignore that and go ahead with the already installed 116. So sorry, can't test that.

At that point, what do you think of just having a data-wasm-opt-flags attribute?

I would say thats prefferable over mapping everything. I was also at some point thinking about trying the fast-math flag for wasm-opt, but decided against going through the necessary steps to get there.

Huh. I also can't see anything obvious. Is that failing code available somewhere?

I figured out how to reproduce the problem. Simply add the following to examples/wasm_threads/Cargo.toml:

[profile.release]
strip = true

And if you remove data-threads from index.html, you get the following error:

$ ../../target/debug/trunk build --release
<...>
[wasm-validator error in module] unexpected false: shared memory requires threads [--enable-threads], on
memory
[wasm-validator error in module] unexpected false: nonzero segment flags require bulk memory [--enable-bulk-memory], on
[wasm-validator error in module] unexpected false: nonzero segment flags require bulk memory [--enable-bulk-memory], on
[wasm-validator error in module] unexpected false: nonzero segment flags require bulk memory [--enable-bulk-memory], on
Fatal: error validating input
2024-09-09T22:51:06.121031Z ERROR ❌ error
error from build pipeline

Caused by:
    0: HTML build pipeline failed (1 errors), showing first
    1: error from asset pipeline
    2: running wasm-opt
    3: wasm-opt call to executable '/Users/ben/Library/Caches/dev.trunkrs.trunk/wasm-opt-version_116/bin/wasm-opt' with args: '["--output=/Users/ben/Documents/Fractals/trunk/examples/wasm_threads/target/wasm-opt/release/wasm_threads_bg.wasm", "-Oz", "/Users/ben/Documents/Fractals/trunk/examples/wasm_threads/dist/.stage/wasm_threads-103504893456a948_bg.wasm"]' returned a bad status: exit status: 1
2024-09-09T22:51:06.121080Z ERROR error from build pipeline
2024-09-09T22:51:06.121086Z  INFO   1: HTML build pipeline failed (1 errors), showing first
2024-09-09T22:51:06.121089Z  INFO   2: error from asset pipeline
2024-09-09T22:51:06.121093Z  INFO   3: running wasm-opt
2024-09-09T22:51:06.121096Z  INFO   4: wasm-opt call to executable '/Users/ben/Library/Caches/dev.trunkrs.trunk/wasm-opt-version_116/bin/wasm-opt' with args: '["--output=/Users/ben/Documents/Fractals/trunk/examples/wasm_threads/target/wasm-opt/release/wasm_threads_bg.wasm", "-Oz", "/Users/ben/Documents/Fractals/trunk/examples/wasm_threads/dist/.stage/wasm_threads-103504893456a948_bg.wasm"]' returned a bad status: exit status: 1

Note, adding strip = "debuginfo" does not cause the problem. Something about stripping debuginfo+symbols is what causes the issue.

Does this perhaps indicate a problem in wasm-opt? Why would the "same" wasm module produce errors only when debuginfo+symbols are removed from it?

I experimented around with a few settings to make my WASM binary smaller in the beginning, IIRC stripping had no impact if I also used wasm-opt. I think wasm-opt is stripping on its own by default, this is also implied by the text for data-keep-debug.

So in that case, unless you observed something different, you could probably also just get rid of that.

My application also works fine if I add strip=true. It must depend on other factors as well. Didn't test the example on my laptop yet.

Have you tried this stuff with wasm-opt 118? And/or wasm-bindgen 0.2.93

Same issue in examples/wasm_threads using wasm-opt 118 and wasm-bindgen 0.2.93:

% TRUNK_TOOLS_WASM_OPT=version_118 TRUNK_TOOLS_WASM_BINDGEN=0.2.93 ../../target/debug/trunk build --release
2024-09-09T23:50:45.444901Z  INFO 🚀 Starting trunk 0.21.0-rc.3
2024-09-09T23:50:45.446317Z  INFO 📦 starting build
   Compiling wasm-bindgen-shared v0.2.93
   Compiling wasm-bindgen v0.2.93
   Compiling once_cell v1.19.0
   Compiling wasm-bindgen-backend v0.2.93
   Compiling wasm-bindgen-macro-support v0.2.93
   Compiling wasm-bindgen-macro v0.2.93
   Compiling js-sys v0.3.69
   Compiling console_error_panic_hook v0.1.7
   Compiling web-sys v0.3.69
   Compiling wasm_thread v0.3.0
   Compiling console_log v1.0.0
   Compiling wasm_threads v0.1.0 (/Users/ben/Documents/Fractals/trunk/examples/wasm_threads)
warning: unstable feature specified for `-Ctarget-feature`: `atomics`
  |
  = note: this feature is not stably supported; its behavior can change in the future

warning: `wasm_threads` (bin "wasm_threads") generated 1 warning
    Finished `release` profile [optimized] target(s) in 6.48s
[wasm-validator error in function 1] unexpected false: Atomic operations require threads [--enable-threads], on

At that point, what do you think of just having a data-wasm-opt-flags attribute?

Sounds like a great idea

My application also works fine if I add strip=true. It must depend on other factors as well. Didn't test the example on my laptop yet.

I have to excuse myself. That was late at night, and I forgot not actually test in release mode. With release mode I get the same error.

Sorry for wasting your time.

Closing this in favor of putting up a PR for custom wasm-opt-flags, wasm-bindgen-flags, and cargo-flags

Closing this in favor of putting up a PR for custom wasm-opt-flags, wasm-bindgen-flags, and cargo-flags

Is that work in progress or waiting for a contribution?

It was going to be WIP by me but I became too busy to get around to it.

@101100 thanks for asking for an update, I forgot to check up on this myself.

Are you working on this right now? Otherwise I might give it a shot myself.

If you did already work some on this, perhaps you could also open a WIP PR so that I can follow that, and see if progress stalls.

@9SMTM6 I didn't get a chance to start on this on the weekend, so you would not be duplicating effort if you started on it.