rust: make pkgsCross.wasi32.rustPlatform.buildRustPackage work #323161
Conversation
github-actions
bot
added
6.topic: rust
6.topic: lib
jcaesar
changed the title
| @@ -23,7 +23,9 @@ rec { | |||
|
|
|||
| ccForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc"; | |||
| cxxForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}c++"; | |||
| linkerForHost = if shouldUseLLD stdenv.targetPlatform | |||
| linkerForHost = if stdenv.targetPlatform.isWasi | |||
| then "${pkgsBuildHost.llvmPackages.bintools-unwrapped}/bin/${stdenv.cc.targetPrefix}wasm-ld" | |||
There was a problem hiding this comment.
The problem here is that
- wasm-ld isn't included by bintools-wrapper. And even if add that, rustc seems to pass
-flavor wasmto the linker, even though its path ends inwasm-ld.) ld.lldfrombintools-unwrappedwould also work, but from what I understand, it works because rustc passes-flavor wasm, which is a legacy option.ld.lldfrombintoolschokes on that argument.- Using
"${stdenv.cc}/bin/${stdenv.cc.targetPrefix}clang"as a linker could work (must be "clang", not "cc" as used inccForHost), but I have been unable to figure out how to explain to clang that the linker it should use is not namedwasm-ldbutwasm32-unknown-wasi-wasm-ld(it does have that in its search path already).
So this is the only solution I was able to come up with. Since the resulting binaries are statically linked and work, I don't suppose there's a big problem with not using the wrapped linkers.
There was a problem hiding this comment.
Wouldn't using a wrapped linker be necessary to find dependencies? What linker does Rust use by default?
There was a problem hiding this comment.
What linker does Rust use by default?
Either "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc" if that's clang or ld.lld otherwise, both are wrapped, from what I can see.
Wouldn't using a wrapped linker be necessary to find dependencies?
It's possible that there isn't a single rust package that depends on nixpkgs-built dependencies makes it to linking. Most of them do depend on stuff that doesn't work on wasi, like openssl or gui-related libraries. So I can't answer that question. Why using a wrapped linker breaks certainly needs some investigation… Not sure I have time for that.
In any case: bintools (wrapped): doesn't work. bintools-unwrapped: works. I don't see any other option here.
There was a problem hiding this comment.
What are the observed symptoms of it not working? Not that if you set the NIX_DEBUG variable than it will also print what flags it injects into the LD executable.
There was a problem hiding this comment.
I would at least leave a comment why we are doing this for now, potentially combined with the error message.
There was a problem hiding this comment.
@Mic92 Thank you for the NIX_DEBUG hint. Getting rust to print the linker command line in the unwrapped case was a bit more tricky, but with both, I was able to confirm what's going on.
The wrapper script was passing hardening flags (-z relro -z now) to wasm-ld, which
- cause lld to not recognize the
-flavor wasmarguments - don't make sense on wasm (
warning: unknown -z value: now) anyway
I'm not sure what's best here, I don't think hardening makes a lot of sense when compiling to wasm anyway, but I don't know of a way to disable hardening for an entire target. What I opted for is to instead add hardening_unsupported_flags+=" relro bindnow" in the wrapper script on wasi.
For reference:
The output the linker wrapper is:
error: linking with `/nix/store/6plp6qym01fbp05lwa1rh4ym50vnw1ng-wasm32-unknown-wasi-llvm-binutils-wrapper-17.0.6/bin/wasm32-unknown-wasi-wasm-ld` failed: exit status: 1
|
= note: LC_ALL="C" PATH="/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/x86_64-unknown-linux-gnu/bin:/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/x86_64-unknown-linux-gnu/bin/self-contained:/nix/store/708hwm
= note: HARDENING: disabled flags: pie
HARDENING: Is active (not completely disabled with "all" flag)
HARDENING: enabling relro
HARDENING: enabling bindnow
extra flags before to /nix/store/5gghgw5dkhw3dx862wynrd26sm3k8xbp-llvm-binutils-17.0.6/bin/wasm32-unknown-wasi-wasm-ld:
-z
relro
-z
now
original flags to /nix/store/5gghgw5dkhw3dx862wynrd26sm3k8xbp-llvm-binutils-17.0.6/bin/wasm32-unknown-wasi-wasm-ld:
-flavor
wasm
--rsp-quoting=posix
--export
__main_void
-z
stack-size=1048576
--stack-first
--allow-undefined
--no-demangle
/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/wasm32-wasip1/lib/self-contained/crt1-command.o
/build/source/target/wasm32-wasip1/release/deps/rq-9dc42ad07b27356d.rq.2f192189ed26cc41-cgu.0.rcgu.o
-L
/build/source/target/wasm32-wasip1/release/deps
-L
/build/source/target/release/deps
-L
/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/wasm32-wasip1/lib
-l
c
/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/wasm32-wasip1/lib/libcompiler_builtins-dae8cdcfb0b90f99.rlib
-L
/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/wasm32-wasip1/lib
-L
/nix/store/m9dh8g3r6pvbzsxz6b94136l38wmkvgz-wasm32-unknown-wasi-rustc-1.78.0/lib/rustlib/wasm32-wasip1/lib/self-contained
-o
/build/source/target/wasm32-wasip1/release/deps/rq-9dc42ad07b27356d.wasm
--gc-sections
-O3
extra flags after to /nix/store/5gghgw5dkhw3dx862wynrd26sm3k8xbp-llvm-binutils-17.0.6/bin/wasm32-unknown-wasi-wasm-ld:
-L/nix/store/56c3y4h4ba85fxz0876ld9qd8l3ww2hy-libcxx-static-wasm32-unknown-wasi-17.0.6/lib
-L/nix/store/02cizbm0w4akb46mapwb0in8jcr10srp-wasilibc-static-wasm32-unknown-wasi-21/lib
wasm32-unknown-wasi-wasm-ld: error: unknown argument: -flavor
wasm32-unknown-wasi-wasm-ld: warning: unknown -z value: relro
wasm32-unknown-wasi-wasm-ld: warning: unknown -z value: now
wasm32-unknown-wasi-wasm-ld: error: cannot open wasm: No such file or directory
This is what I was trying with:
diff --git a/pkgs/build-support/bintools-wrapper/default.nix b/pkgs/build-support/bintools-wrapper/default.nix
index e7fcf173c602..1c955fdd8d0d 100644
--- a/pkgs/build-support/bintools-wrapper/default.nix
+++ b/pkgs/build-support/bintools-wrapper/default.nix
@@ -239,6 +239,8 @@ stdenvNoCC.mkDerivation {
basename=$(basename "$variant")
wrap $basename ${./ld-wrapper.sh} $variant
done
+ '' + optionalString targetPlatform.isWasi ''
+ wrap ${targetPrefix}wasm-ld ${./ld-wrapper.sh} $ldPath/${targetPrefix}wasm-ld
'';
strictDeps = true;
diff --git a/pkgs/build-support/rust/lib/default.nix b/pkgs/build-support/rust/lib/default.nix
index b52eecccbb2f..7f66bf01c1e5 100644
--- a/pkgs/build-support/rust/lib/default.nix
+++ b/pkgs/build-support/rust/lib/default.nix
@@ -24,7 +24,7 @@ rec {
ccForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc";
cxxForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}c++";
linkerForHost = if stdenv.targetPlatform.isWasi
- then "${pkgsBuildHost.llvmPackages.bintools-unwrapped}/bin/${stdenv.cc.targetPrefix}wasm-ld"
+ then "${pkgsBuildHost.llvmPackages.bintools}/bin/${stdenv.cc.targetPrefix}wasm-ld"
else if shouldUseLLD stdenv.targetPlatform
&& !stdenv.cc.bintools.isLLVM
then "${pkgsBuildHost.llvmPackages.bintools}/bin/${stdenv.cc.targetPrefix}ld.lld"
diff --git a/pkgs/development/tools/rq/default.nix b/pkgs/development/tools/rq/default.nix
index f8ed521ad2cc..589d347dbac5 100644
--- a/pkgs/development/tools/rq/default.nix
+++ b/pkgs/development/tools/rq/default.nix
@@ -24,6 +24,9 @@ rustPlatform.buildRustPackage rec {
rm build.rs
'';
+ env.NIX_DEBUG = "1";
+ env.RUSTC_LOG = "rustc_codegen_ssa::back::link=info";
+
VERGEN_SEMVER = version;
meta = with lib; {There was a problem hiding this comment.
By "by default", I mean what linker does a rustc you'd get from rustup use in this case?
There was a problem hiding this comment.
Ah, I see. @alyssais For the docker.io/library/rust docker image (which is currently on 1.79 and made using rustup) it defaults to rust-lld -flavor wasm.
There was a problem hiding this comment.
(I'm sorry, I force-pushed over this commit again, I mistakenly thought not changing the linker was going to work. But it does have to be set to lld.)
|
I've re-verified that this is the best solution I can come up with. I've added comments on the parts that aren't entirely starightforward. @Kranzes, if you only removed your reviewer status because this was still a draft, can I ask you to have a look at some point? |
jcaesar
requested review from
zowoq,
winterqt,
figsoda,
Mic92,
alyssais and
Ericson2314
as code owners
| @@ -23,7 +23,9 @@ rec { | |||
|
|
|||
| ccForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc"; | |||
| cxxForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}c++"; | |||
| linkerForHost = if shouldUseLLD stdenv.targetPlatform | |||
| linkerForHost = if stdenv.targetPlatform.isWasi | |||
| then "${pkgsBuildHost.llvmPackages.bintools-unwrapped}/bin/${stdenv.cc.targetPrefix}wasm-ld" | |||
There was a problem hiding this comment.
Wouldn't using a wrapped linker be necessary to find dependencies? What linker does Rust use by default?
|
Thank you for the review. (The reason I wasn't particularly keen on touching wasilibc is that the following takes ages to build. The floorp failure also happens on master, so that's unrelated.) Result of 1 package blacklisted:
3 packages failed to build:
32 packages built:
|
jcaesar
force-pushed
the
pr-9
branch
2 times, most recently
from
cf1e88c
to
90b9b0c
Compare
|
I tried building pkgsCross.wasi32.qrtool, but it failed to build wasm32-unknown-wasi-rustc-1.78.0. |
|
@alyssais Could you tell me exactly what you're trying to build? The following work for me (should be the same thing): (qrtool will fail in installPhase because of #289517 / #308283. Try rq instead if the above does get past the failure you saw in rustc.) [Edit:] Did you try that on master / some other PR? Because that's nearly the same error as in the opening post for this PR. |
|
Looks like I might have mixed up which branch I was building — sorry! |
| @@ -23,8 +23,9 @@ rec { | |||
|
|
|||
| ccForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc"; | |||
| cxxForHost = "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}c++"; | |||
| linkerForHost = if shouldUseLLD stdenv.targetPlatform | |||
| && !stdenv.cc.bintools.isLLVM | |||
| # For wasi: rustc passes lld-flavored args unless the compiler is named "clang". So use lld. | |||
There was a problem hiding this comment.
Do you know which code in rustc does this check?
There was a problem hiding this comment.
After some digging. I think I found the relevant part. So clang or gcc as names would be recognized, but not cc (as set in ccForHost). I've revised the comment accordingly.
(Defaulting to lld without being wrapped in a compiler is here.)
Side notes:
- Since
ccisn't recognized as anything by rust, the current setup forccForHostis maybe not the best in general…? - This doccoment has some helpful insights.
There was a problem hiding this comment.
- Since
ccisn't recognized as anything by rust, the current setup forccForHostis maybe not the best in general…?
Yeah, that's what I was wondering.
There was a problem hiding this comment.
It might also make sense to teach Rust to understand "cc", as it's not exactly a Nixpkgs-specific convention.
There was a problem hiding this comment.
I think I read somewhere that not recognizing cc is intentional. Somebody felt that two letters is too little to go on? I might be mistaken, my memory on this is rather hazy. (In any case, this is a finicky part of rustc. Just looking at the amount of "fix" here. I'd rather not touch it.)
In any case, that's not something I intend to fix here. ^^
There was a problem hiding this comment.
Given that there are other options, I really don't think we should expand the hack of using a custom linker for rustc. Either fixing this in rustc, or fixing ccForHost would be real fixes, not a hack like this.
There was a problem hiding this comment.
There are other options?
In case you're thinking of just doing
linkerForHost = if stdenv.cc.bintools.isLLVM
then "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}clang"
else "${stdenv.cc}/bin/${stdenv.cc.targetPrefix}gcc";
# resp. doing this for all of cc/cxx/linkerForHostthat won't work without further adjustments either:
= note: clang: error: unable to execute command: Executable "wasm-ld" doesn't exist!
clang: error: linker command failed with exit code 1 (use -v to see invocation)
And I really don't now what those adjustments are, or whether they're benign. I've previously looked into whether clang can somehow be taught a different linker command, but couldn't find out anything.
(This would also severely increase the blast radius of this PR…)
There was a problem hiding this comment.
I dug around a bit (for options), here's a few things I found:
- It's unlikely that rustc upstream will accept patches that recognize a
-ccsuffix as a compiler wrapping a linker, reason being that-ccmight be anything, so it's unclear what arguments can be passed to it - cargo-auditable passes linker flags with
-Wl,unconditionally (src) meaning we can't just use a plain linker aslinkerForHost, it has to be invoked through a compiler as long as we want to use cargo-auditable. - 1 and 2 combined mean that some
if foo then bar else blubbis necessary forlinkerFor…. - Using clang as a linker may work, but it's mechanism for finding wasm-ld is not nice: It searches paths passed with
-Bfor<triple>-wasm-ldandwasm-ld(src). However, it doesn't properly normalize the triple, and just uses the-target wasm32-wasiargument it gets from rustc as is. Clang itself doesn't think that's a good state and does have a TODO about this.
(This might also be fixable in rustc, which does seem to have logic to not directly pass its own target name. But I havent looked into how that works.) - The way the linker gets passed from bintools to clang is:
- 4+5 could in theory be worked around like this, but that's a far more terrible hack than what I've tried to do here.
So, the only way I see to get this to work cleanly is to submit a fix to clang for triple normalization, and then submit a PR to nixpkgs staging that switches to if stdenv.cc.bintools.isLLVM then clang else gcc (oh the rebuilds, I don't have hardware to test this), and then finally get back to this. I think that's above the level of difficulty I can manage.
If you've got any advice on how else I could approach this, please do tell.
Rust assumes the linker is lld-flavored (unless it's named clang), and passes arguments accordingly. Two things are required to make that work: * Set the linker to ld.lld * disable linker hardening flags (they'd be passed befor the -flavor arg, breakin that, on top of having no effect)
Description of changes
This PR makes it possible to e.g. build and run rust packages like
nix shell .#pkgsCross.wasi32.charasay -c charasay.wasm say moo(assuming you havewasm32-wasiin your emulated systems).Without this PR, building the cross rustc fails with
I think this is neat, but it admittedly isn't super useful, as a lot of packages fail to compile due to failing C dependencies or some common dependencies assuming that all wasm is for the web. I've tested the 469 rust packages in by-name, and only 60¹ compile. Even those that do compile might fail at runtime, e.g. due to std::thread not being supported and panicking when attempting to create a thread (like alejandra).
I'm only aware of two mildly useful things that work, namely rq and qrtool. But I think it's nice to have this working.
¹ aarch64-esr-decoder, aba, action-validator, adrs, bunbun, cargo-autoinherit, cargo-bloat, cargo-bump, cargo-shear, cargo-swift, catppuccin-whiskers, catppuccinifier-cli, cdwe, circom, clippy-sarif, cmd-wrapped, commitmsgfmt, diesel-cli-ext, dipc, elf-info, emacs-lsp-booster, esbuild-config, french-numbers, galerio, git-upstream, hadolint-sarif, hvm, hyperlink, icnsify, ifrextractor-rs, keedump, kickstart, ldproxy, little_boxes, ludtwig, lutgen, majima, mcumgr-client, mdsh, ndstrim, obsidian-export, okolors, parallel-disk-usage, pdfrip, polylux2pdfpc, preserves-tools, prettypst, protoc-gen-rust-grpc, qrtool, rq, rs-tftpd, rusti-cal, rusty-diceware, sarif-fmt, shellcheck-sarif, tera-cli, thud, toml-cli, typstfmt, unison-fsmonitor, wit-bindgen
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.