Allow compiling the wasm32-wasi std library with atomics

[abrown]

The issue #102157 demonstrates how currently the -Z build-std option will fail when re-compiling the standard library with RUSTFLAGS like RUSTFLAGS="-C target-feature=+atomics,+bulk-memory -C link-args=--shared-memory". This change attempts to resolve those build issues by depending on the the WebAssembly futex module and providing an implementation for env_lock. Fixes #102157.

[rust-highfive]

r? @thomcc

(rust-highfive has picked a reviewer for you, use r? to override)

[abrown]

There are a couple of things I could use help with in this PR:

1. I assume that env_lock here should actually return one of the lock guards available to WebAssembly but I'm not really sure yet about the details of this — can someone point me to an example of how this should be done from other targets?
2. I don't have a great way to test this end-to-end by compiling a Rust program with this version of the stdlib: I think I can build the stdlib now without error: RUSTFLAGS="-C target-feature=+atomics,+bulk-memory" ./x.py build library/std --target wasm32-wasi (actually, is this right?). But I'm not sure how to correctly use this new sysroot to compile a Rust program; a command like the following fails — what is the right way to do this?

$ /.../rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc kernel.rs --target wasm32-wasi --crate-type=cdylib -o kernel.wasm -C link-args="--shared-memory" -C target-feature='+atomics,+atomics' --sysroot /.../rust/build/x86_64-unknown-linux-gnu/stage1

...
error: linker `rust-lld` not found
  |
  = note: No such file or directory (os error 2)

error: aborting due to previous error; 1 warning emitted

[joboet]

1. I assume that env_lock here should actually return one of the lock guards available to WebAssembly but I'm not really sure yet about the details of this — can someone point me to an example of how this should be done from other targets?

UNIX uses an RwLock to ensure correct synchronization between setting and getting the environment variables, as setenv is unsynchronized. The libc used on WASI is based on MUSL and is not thread-safe, too, so it's probably best to copy the UNIX implementation.

[alexcrichton]

But I'm not sure how to correctly use this new sysroot to compile a Rust program; a command like the following fails — what is the right way to do this?

You'll need to set lld = true in config.toml as well to ensure that you're locally compiling wasm-ld. Otherwise though I don't think WASI tests work with the test harness right now and I don't think that compiletest has support for WASI. Notably this has no support for WASI which is used as a node.js-based runner for wasm tests.

I've historically dabbled in getting wasm32-wasi tests up and running using wasmtime-the-CLI but I never ended up finishing the work or proposing it here since it didn't feel like it fit. Overall for testing this I think the best thing to do for now is to compile a toolchain and test it on local programs. Eventually when the whole test suite is up and running more issues can be ironed out.

(FWIW historical test runs turned up no actual failures, just a lot of infrastructural changes to get things working)

[alexcrichton]

If you're interested this is my historical work to get the test suite running with wasmtime-the-CLI. requiring you to specify:

[target.wasm32-wasi]
runtool = 'wasmtime run --disable-cache --'

in config.toml. This doesn't pass the test suite since lots of // ignore-... needs updates for wasm32-wasi, but you should be able to skim the failures and see if there's anything obvious. I don't think that the src/test/ui test suite will be all that useful for testing this change though.

[thomcc]

This is still a draft. Feel free to kick it back to me with at-rustbot ready once it's ready for review.

@rustbot author

[abrown]

As @alexcrichton notes above, the in-tree testing story still has some way to go, so I manually tested that this change works using the following sequence of steps:

1. First, I built the latest HEAD of wasi-libc with threads enabled:

$ git rev-parse HEAD
05b3b876e63cdc919c79f5127a7ae8dbdb7b095f
$ make THREAD_MODEL=posix

2. Then, in this repository, I altered my config.toml to use this new sysroot:

// first, enabled building the linker by setting `lld = true` in the [rust] section; then:
$ echo '[target.wasm32-wasi]' >> config.toml
$ echo 'wasi-root = "/.../wasi-libc/sysroot"' >> config.toml

3. Then, I built the Rust toolchain with all of the changes in this PR along with the flags to activate them:

$ RUSTFLAGS="-C target-feature=+atomics,+bulk-memory" ./x.py build library --target wasm32-wasi
$ rustup toolchain link stage1 build/x86_64-unknown-linux-gnu/stage1

5. Using the same kernel.rs example from Fail to compile with WebAssembly atomics and shared memory #102157, I was able to successfully compile a Rust program that uses shared memory:

$ rustc +stage1 kernel.rs -o kernel.wasm --target wasm32-wasi --crate-type=cdylib  -C link-args="--shared-memory" -C target-feature='+atomics,+atomics'
$ wasm2wat --enable-threads kernel.wasm | grep '(memory'
  (import "env" "memory" (memory (;0;) 17 16384 shared))

[abrown]

I rebased this change on latest master and switched to using RwLock as @joboet pointed out. If someone can point me to the right way to encode a test based on the manual steps I did above, I will add that, but otherwise I think this is ready for review.

@rustbot ready

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[abrown]

Ok, I guess: @rustbot label +T-libs-api -T-libs

[abrown]

Oh, never mind: @rustbot label -T-libs-api +T-libs

[thomcc]

Have you done a check for the places we might cfg(target_arch = "wasm") to see if any of them have similar assumptions? My main hesitation here is I seem to recall that there's a fair bit of code that makes the same assumption (possibly including code std has a dependency on).

[abrown]

Have you done a check for the places we might cfg(target_arch = "wasm") to see if any of them have similar assumptions? My main hesitation here is I seem to recall that there's a fair bit of code that makes the same assumption (possibly including code std has a dependency on).

I think you mean target_family = "wasm"? Once I look for that I see that library/std/src/thread/local.rs has quite a few references to that but it seems to me that they are appropriately paired with not(target_feature = "atomics"). Perhaps that was what you were concerned about? The entire repository has 26 results for target_family = "wasm" but the rest do not seem to me to pertain to your question.

I would say that compiling Rust with atomics and shared memory is still a highly risky venture and that this PR only fixes an issue that I am aware of. There will likely be others once people start experimenting with this.

[thomcc]

I think you mean target_family = "wasm"

Uh, yeah, and just a grep for cfgs with wasm in them I guess (in case they were on target_arch="wasm32" or something).

I would say that compiling Rust with atomics and shared memory is still a highly risky venture and that this PR only fixes an issue that I am aware of. There will likely be others once people start experimenting with this.

This is pretty concerning (assuming you mean that there are likely issues in std; I'm aware the ecosystem is... somwhat ill-prepared for this). Is this target_feature stable? If so I think it might be better to get a handle on it before we expose users to a bunch of stdlib code that may have bugs...

[thomcc]

Oh, it requires -Zbuild-std? Hm...

[abrown]

Yeah, this PR just fixes compilation with features that are already exposed (even if experimentally behind -Z). Is there anything else I should do to get this reviewed and merged?

[bors]

📌 Commit 9530ba0 has been approved by thomcc

It is now in the queue for this repository.

[JohnTitor]

Failed on rollup: #102908 (comment)
@bors r-

[abrown]

@thomcc, 95b0b2d should fix the bors failure.

@rustbot label -S-waiting-on-author +S-waiting-on-review

[thomcc]

@bors r+

[bors]

📌 Commit 95b0b2d has been approved by thomcc

It is now in the queue for this repository.

[bors]

⌛ Testing commit 95b0b2d with merge 2a92176...

[bors]

☀️ Test successful - checks-actions
Approved by: thomcc
Pushing 2a92176 to master...

[rust-timer]

Finished benchmarking commit (2a92176): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	0.8%	[0.7%, 1.0%]	6
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.7%, 1.0%]	6

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	2.6%	[2.6%, 2.6%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.6%	[2.6%, 2.6%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	1.9%	[1.9%, 1.9%]	1
Regressions ❌ (secondary)	3.5%	[3.5%, 3.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.9%	[1.9%, 1.9%]	1

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[lqd]

The regressions are recent diesel noise, and unrelated to this PR and the wasi target.

@rustbot label: +perf-regression-triaged